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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/12/20 10:01 AM, Peter Krempa wrote:
On Mon, Oct 12, 2020 at 09:39:33 +0200, Michal Privoznik wrote:
On 10/12/20 1:44 AM, 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@xxxxxxxxxx>
---
   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;

Wee tiny nitpick. If you're touching this might switch it g_auto:

g_autoptr(virConnect) conn = NULL;

and drop the explicit unref.

In this specific case I'd stay with explicit cleanup. If you look at the
cleanup section we have a 'vol', 'pool' and 'conn' pointer. They are
interconnected. While libvirt internally does refcounting, cleaning them
up in random order might seem wrong.


Wouldn't the same 'random' order appear again when we switch to g_auto() for the function anyways? Also, how to order cleanup if we have two objects, both holding a reference to each other (e.g. virDomainObj and qemuMonitor). Isn't refcounting designed exactly for cases like these?

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux