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

[Peter Krempa]

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 at redhat.com>
> ---
>  src/qemu/qemu_domain.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)

Once the commit message makes sense:

Reviewed-by: Peter Krempa <pkrempa at redhat.com>

> 
> 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.