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