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