Re: [PATCH] qemu: Don't crash when getting targets for a multipath

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.

> > Fixes: a30078cb832646177defd256e77c632905f1e6d0 > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1814947 > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/qemu/qemu_domain.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) Once the commit message makes sense: Reviewed-by: Peter Krempa <pkrempa(a)redhat.com&gt; > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 0e2252f6cf..ef17829235 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -15862,19 +15862,19 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm, > } > > tmpPath = g_strdup(next->path); > + > + if (virDevMapperGetTargets(next->path, &targetPaths) < 0 && > + errno != ENOSYS && errno != EBADF) { > + virReportSystemError(errno, > + _("Unable to get devmapper targets for %s"), > + next->path); > + return -1; > + } > } > > if (virStringListAdd(&paths, tmpPath) < 0) > return -1; > > - if (virDevMapperGetTargets(next->path, &targetPaths) < 0 && > - errno != ENOSYS && errno != EBADF) { > - virReportSystemError(errno, > - _("Unable to get devmapper targets for %s"), > - next->path); > - return -1; > - } > - > if (virStringListMerge(&paths, &targetPaths) < 0) > return -1; Is this supposed to go after 'tmpPath' in the list? Otherwise you might want to move it toghether with the call to virDevMapperGetTargets.