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
