[PATCH] qemu: migration: don't open storage driver too early

[Daniel P. Berrangé]

On Sun, Oct 11, 2020 at 07:44:36PM -0400, Cole Robinson wrote:
> If storage migration is requested, and the destination storage does
> not exist on the remote host, qemu's migration support will call
> into the libvirt storage driver to precreate the destination storage.
> 
> The storage driver virConnectPtr is opened too early though, adding
> an unnecessary dependency on the storage driver for several cases
> that don't require it. This currently requires kubevirt to install
> the storage driver even though they aren't actually using it.
> 
> Push the virGetConnectStorage calls to right before the cases they are
> actually needed.

This pushes the connection open attempts inside a loop body. So if the
VM has multiple disks, then we're going to be repeatedly opening and
closing the connection which is not desirable.

I think we need a global connection across all disks, which is lazy
opened on first access.

> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> ---
>  src/qemu/qemu_migration.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 2000c86640..99a6b41483 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -169,8 +169,7 @@ qemuMigrationSrcRestoreDomainState(virQEMUDriverPtr driver, virDomainObjPtr vm)
>  
>  
>  static int
> -qemuMigrationDstPrecreateDisk(virConnectPtr conn,
> -                              virDomainDiskDefPtr disk,
> +qemuMigrationDstPrecreateDisk(virDomainDiskDefPtr disk,
>                                unsigned long long capacity)
>  {
>      int ret = -1;
> @@ -181,6 +180,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn,
>      g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>      const char *format = NULL;
>      unsigned int flags = 0;
> +    virConnectPtr conn = NULL;
>  
>      VIR_DEBUG("Precreate disk type=%s", virStorageTypeToString(disk->src->type));
>  
> @@ -204,6 +204,9 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn,
>          *volName = '\0';
>          volName++;
>  
> +        if (!(conn = virGetConnectStorage()))
> +            goto cleanup;
> +
>          if (!(pool = virStoragePoolLookupByTargetPath(conn, basePath)))
>              goto cleanup;
>          format = virStorageFileFormatTypeToString(disk->src->format);
> @@ -212,6 +215,9 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn,
>          break;
>  
>      case VIR_STORAGE_TYPE_VOLUME:
> +        if (!(conn = virGetConnectStorage()))
> +            goto cleanup;
> +
>          if (!(pool = virStoragePoolLookupByName(conn, disk->src->srcpool->pool)))
>              goto cleanup;
>          format = virStorageFileFormatTypeToString(disk->src->format);
> @@ -270,6 +276,7 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn,
>      VIR_FREE(volStr);
>      virObjectUnref(vol);
>      virObjectUnref(pool);
> +    virObjectUnref(conn);
>      return ret;
>  }
>  
> @@ -304,13 +311,10 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
>  {
>      int ret = -1;
>      size_t i = 0;
> -    virConnectPtr conn;
>  
>      if (!nbd || !nbd->ndisks)
>          return 0;
>  
> -    if (!(conn = virGetConnectStorage()))
> -        return -1;
>  
>      for (i = 0; i < nbd->ndisks; i++) {
>          virDomainDiskDefPtr disk;
> @@ -349,13 +353,12 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
>  
>          VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath));
>  
> -        if (qemuMigrationDstPrecreateDisk(conn, disk, nbd->disks[i].capacity) < 0)
> +        if (qemuMigrationDstPrecreateDisk(disk, nbd->disks[i].capacity) < 0)
>              goto cleanup;
>      }
>  
>      ret = 0;
>   cleanup:
> -    virObjectUnref(conn);
>      return ret;
>  }

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|