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

Cole Robinson posted 1 patch 3 years, 5 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/626e6103f22c78262f5cf0c1bd421c252a6bffca.1602522367.git.crobinso@redhat.com
src/qemu/qemu_migration.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
[PATCH v2] qemu: migration: don't open storage driver too early
Posted by Cole Robinson 3 years, 5 months ago
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.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
v2:
    Only open the connection once per VM via
    qemuMigrationDstPrecreateStorage
 src/qemu/qemu_migration.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2000c86640..4e959abebf 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -169,7 +169,7 @@ qemuMigrationSrcRestoreDomainState(virQEMUDriverPtr driver, virDomainObjPtr vm)
 
 
 static int
-qemuMigrationDstPrecreateDisk(virConnectPtr conn,
+qemuMigrationDstPrecreateDisk(virConnectPtr *conn,
                               virDomainDiskDefPtr disk,
                               unsigned long long capacity)
 {
@@ -204,7 +204,12 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn,
         *volName = '\0';
         volName++;
 
-        if (!(pool = virStoragePoolLookupByTargetPath(conn, basePath)))
+        if (!*conn) {
+            if (!(*conn = virGetConnectStorage()))
+                goto cleanup;
+        }
+
+        if (!(pool = virStoragePoolLookupByTargetPath(*conn, basePath)))
             goto cleanup;
         format = virStorageFileFormatTypeToString(disk->src->format);
         if (disk->src->format == VIR_STORAGE_FILE_QCOW2)
@@ -212,7 +217,12 @@ qemuMigrationDstPrecreateDisk(virConnectPtr conn,
         break;
 
     case VIR_STORAGE_TYPE_VOLUME:
-        if (!(pool = virStoragePoolLookupByName(conn, disk->src->srcpool->pool)))
+        if (!*conn) {
+            if (!(*conn = virGetConnectStorage()))
+                goto cleanup;
+        }
+
+        if (!(pool = virStoragePoolLookupByName(*conn, disk->src->srcpool->pool)))
             goto cleanup;
         format = virStorageFileFormatTypeToString(disk->src->format);
         volName = disk->src->srcpool->volume;
@@ -304,14 +314,11 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
 {
     int ret = -1;
     size_t i = 0;
-    virConnectPtr conn;
+    virConnectPtr conn = NULL;
 
     if (!nbd || !nbd->ndisks)
         return 0;
 
-    if (!(conn = virGetConnectStorage()))
-        return -1;
-
     for (i = 0; i < nbd->ndisks; i++) {
         virDomainDiskDefPtr disk;
         const char *diskSrcPath;
@@ -349,7 +356,8 @@ qemuMigrationDstPrecreateStorage(virDomainObjPtr vm,
 
         VIR_DEBUG("Proceeding with disk source %s", NULLSTR(diskSrcPath));
 
-        if (qemuMigrationDstPrecreateDisk(conn, disk, nbd->disks[i].capacity) < 0)
+        if (qemuMigrationDstPrecreateDisk(&conn,
+                                          disk, nbd->disks[i].capacity) < 0)
             goto cleanup;
     }
 
-- 
2.28.0

Re: [PATCH v2] qemu: migration: don't open storage driver too early
Posted by Daniel P. Berrangé 3 years, 5 months ago
On Mon, Oct 12, 2020 at 01:08:58PM -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.
> 
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
> v2:
>     Only open the connection once per VM via
>     qemuMigrationDstPrecreateStorage
>  src/qemu/qemu_migration.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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