On Thu, Mar 19, 2020 at 18:39:58 +0100, Michal Privoznik wrote:
On 19. 3. 2020 17:43, Peter Krempa wrote:
> On Thu, Mar 19, 2020 at 17:22:37 +0100, Michal Privoznik wrote:
>> In one of my previous commits I've introduced code that creates
>> all devices for given (possible) multipath target. But I've made
>> a mistake there - the code accesses src->path without checking if
>> the disk source is local. Note that the path is NULL if the
>> source is not local.
>
> Well next->path 'may' be NULL if it's not local, but in this case it
is
> local because NVMe disks are local, but they don't have the path.
Local as in virStorageSourceIsLocalStorage() == true. And for NVMe we
return false exactly because src->path has to be NULL (it can't be set
to any meaningfull path).
Yes. The meaning of virStorageSourceIsLocalStorage shifted after NVMe
disks were added.
How about this formulation:
Note that the path is NULL if the
source is not local as viewed by
virStorageSourceIsLocalStorage().
That is still misleading, because network disks can have non-null path.
I'd prefer:
'next->path' is NULL/doesn't make sense for VIR_STORAGE_TYPE_NVME