[libvirt] [PATCH 0/4] qemu: always use blockdev non-shared storage migration with blockdev (blockdev-add saga)
10:39 a.m.
Once we enable blockdev there's no need to stick to drive mirror for
non-shared-storage migration.
This patchset applies on top of the external snapshot branch:
https://www.redhat.com/archives/libvir-list/2019-September/msg00082.html
Peter Krempa (4):
qemu: migration: Refactor cleanup in
qemuMigrationSrcNBDStorageCopyBlockdev
qemu: migration: Refactor cleanup in
qemuMigrationSrcNBDStorageCopyDriveMirror
qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopy
qemu: migration: Switch to blockdev mode for non-shared storage
migration
src/qemu/qemu_migration.c | 69 ++++++++++++++++-----------------------
1 file changed, 29 insertions(+), 40 deletions(-)
--
2.21.0
10:39 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopyBlockdev
Remove the cleanup label as it's empty.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e387deb497..1a557851bb 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -796,50 +796,49 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL;
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
int mon_ret = 0;
- int ret = -1;
VIR_AUTOUNREF(virStorageSourcePtr) copysrc = NULL;
VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias,
host);
if (!(copysrc = virStorageSourceNew()))
- goto cleanup;
+ return -1;
copysrc->type = VIR_STORAGE_TYPE_NETWORK;
copysrc->protocol = VIR_STORAGE_NET_PROTOCOL_NBD;
copysrc->format = VIR_STORAGE_FILE_RAW;
if (!(copysrc->backingStore = virStorageSourceNew()))
- goto cleanup;
+ return -1;
if (VIR_STRDUP(copysrc->path, diskAlias) < 0)
- goto cleanup;
+ return -1;
if (VIR_ALLOC_N(copysrc->hosts, 1) < 0)
- goto cleanup;
+ return -1;
copysrc->nhosts = 1;
copysrc->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
copysrc->hosts->port = port;
if (VIR_STRDUP(copysrc->hosts->name, host) < 0)
- goto cleanup;
+ return -1;
if (VIR_STRDUP(copysrc->tlsAlias, tlsAlias) < 0)
- goto cleanup;
+ return -1;
if (virAsprintf(©src->nodestorage, "migration-%s-storage",
disk->dst) < 0 ||
virAsprintf(©src->nodeformat, "migration-%s-format",
disk->dst) < 0)
- goto cleanup;
+ return -1;
/* Migration via blockdev-mirror was supported sooner than the auto-read-only
* feature was added to qemu */
if (!(data = qemuBlockStorageSourceAttachPrepareBlockdev(copysrc,
copysrc->backingStore,
false)))
- goto cleanup;
+ return -1;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
- goto cleanup;
+ return -1;
mon_ret = qemuBlockStorageSourceAttachApply(qemuDomainGetMonitor(vm), data);
@@ -852,14 +851,11 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), data);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0)
- goto cleanup;
+ return -1;
VIR_STEAL_PTR(diskPriv->migrSource, copysrc);
- ret = 0;
-
- cleanup:
- return ret;
+ return 0;
}
--
2.21.0
10:39 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopyDriveMirror
Use VIR_AUTOFREE and remove the cleanup label.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1a557851bb..82625b2261 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -868,36 +868,31 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver,
unsigned long long mirror_speed,
bool mirror_shallow)
{
- char *nbd_dest = NULL;
+ VIR_AUTOFREE(char *) nbd_dest = NULL;
int mon_ret;
- int ret = -1;
if (strchr(host, ':')) {
if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s",
host, port, diskAlias) < 0)
- goto cleanup;
+ return -1;
} else {
if (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s",
host, port, diskAlias) < 0)
- goto cleanup;
+ return -1;
}
if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
- goto cleanup;
+ return -1;
mon_ret = qemuMonitorDriveMirror(qemuDomainGetMonitor(vm),
diskAlias, nbd_dest, "raw",
mirror_speed, 0, 0, mirror_shallow, true);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || mon_ret < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- VIR_FREE(nbd_dest);
- return ret;
+ return 0;
}
--
2.21.0
12:01 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopyDriveMirror
On 9/4/19 10:39 AM, Peter Krempa wrote:
Use VIR_AUTOFREE and remove the cleanup label.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1a557851bb..82625b2261 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -868,36 +868,31 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr driver,
unsigned long long mirror_speed,
bool mirror_shallow)
{
- char *nbd_dest = NULL;
+ VIR_AUTOFREE(char *) nbd_dest = NULL;
int mon_ret;
- int ret = -1;
if (strchr(host, ':')) {
if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s",
host, port, diskAlias) < 0)
Should we prefer "nbd://[%s]:%d/%s" here, now that the NBD URI spec is
available (and since qemu supports that for quite some time now)?
https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md
- goto cleanup;
+ return -1;
} else {
if (virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s",
and similar here?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2:28 a.m.
New subject: [libvirt] [PATCH 2/4] qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopyDriveMirror
On Wed, Sep 04, 2019 at 12:01:27 -0500, Eric Blake wrote:
On 9/4/19 10:39 AM, Peter Krempa wrote:
> Use VIR_AUTOFREE and remove the cleanup label.
>
> Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> src/qemu/qemu_migration.c | 17 ++++++-----------
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 1a557851bb..82625b2261 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -868,36 +868,31 @@ qemuMigrationSrcNBDStorageCopyDriveMirror(virQEMUDriverPtr
driver,
> unsigned long long mirror_speed,
> bool mirror_shallow)
> {
> - char *nbd_dest = NULL;
> + VIR_AUTOFREE(char *) nbd_dest = NULL;
> int mon_ret;
> - int ret = -1;
>
> if (strchr(host, ':')) {
> if (virAsprintf(&nbd_dest, "nbd:[%s]:%d:exportname=%s",
> host, port, diskAlias) < 0)
Should we prefer "nbd://[%s]:%d/%s" here, now that the NBD URI spec is
available (and since qemu supports that for quite some time now)?
https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md
That commit is literally 2 days old. Also this code is executed on all
supported qemus (starting from 1.5.3) so you'd need to be more specific
on the 'quite some time now'.
Additionally this patchset actually stops calling this code path when
blockdev is used so I'm not going to invest into investigating whether
the URI format is really supported on ancient qemus if it's not going to
be used in the future anyways.
10:39 a.m.
New subject: [libvirt] [PATCH 3/4] qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopy
Use VIR_AUTOUNREF and remove the cleanup label.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 82625b2261..c13b21a0bb 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -983,13 +983,12 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver,
unsigned int flags)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- int ret = -1;
int port;
size_t i;
unsigned long long mirror_speed = speed;
bool mirror_shallow = *migrate_flags & QEMU_MONITOR_MIGRATE_NON_SHARED_INC;
int rv;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+ VIR_AUTOUNREF(virQEMUDriverConfigPtr) cfg = virQEMUDriverGetConfig(driver);
VIR_DEBUG("Starting drive mirrors for domain %s", vm->def->name);
@@ -997,7 +996,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver,
virReportError(VIR_ERR_OVERFLOW,
_("bandwidth must be less than %llu"),
LLONG_MAX >> 20);
- goto cleanup;
+ return -1;
}
mirror_speed <<= 20;
@@ -1015,34 +1014,34 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver,
if (qemuMigrationSrcNBDStorageCopyOne(driver, vm, disk, host, port,
mirror_speed, mirror_shallow,
tlsAlias, flags) < 0)
- goto cleanup;
+ return -1;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps)
< 0) {
VIR_WARN("Failed to save status on vm %s", vm->def->name);
- goto cleanup;
+ return -1;
}
}
while ((rv = qemuMigrationSrcNBDStorageCopyReady(vm, QEMU_ASYNC_JOB_MIGRATION_OUT))
!= 1) {
if (rv < 0)
- goto cleanup;
+ return -1;
if (priv->job.abortJob) {
priv->job.current->status = QEMU_DOMAIN_JOB_STATUS_CANCELED;
virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
_("canceled by client"));
- goto cleanup;
+ return -1;
}
if (dconn && virConnectIsAlive(dconn) <= 0) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("Lost connection to destination host"));
- goto cleanup;
+ return -1;
}
if (virDomainObjWait(vm) < 0)
- goto cleanup;
+ return -1;
}
qemuMigrationSrcFetchMirrorStats(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
@@ -1051,11 +1050,8 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver,
/* Okay, all disks are ready. Modify migrate_flags */
*migrate_flags &= ~(QEMU_MONITOR_MIGRATE_NON_SHARED_DISK |
QEMU_MONITOR_MIGRATE_NON_SHARED_INC);
- ret = 0;
- cleanup:
- virObjectUnref(cfg);
- return ret;
+ return 0;
}
--
2.21.0
10:39 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: migration: Switch to blockdev mode for non-shared storage migration
When blockdev is used we always should use the blockdev mode for
non-shared storage migration.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c13b21a0bb..6ff956b1f5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -907,6 +907,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver,
const char *tlsAlias,
unsigned int flags)
{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
qemuBlockJobDataPtr job = NULL;
char *diskAlias = NULL;
@@ -921,7 +922,8 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver,
qemuBlockJobSyncBegin(job);
- if (flags & VIR_MIGRATE_TLS) {
+ if (flags & VIR_MIGRATE_TLS ||
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
rc = qemuMigrationSrcNBDStorageCopyBlockdev(driver, vm,
disk, diskAlias,
host, port,
--
2.21.0
12:03 p.m.
On 9/4/19 10:39 AM, Peter Krempa wrote:
Once we enable blockdev there's no need to stick to drive mirror
for
non-shared-storage migration.
This patchset applies on top of the external snapshot branch:
https://www.redhat.com/archives/libvir-list/2019-September/msg00082.html
Peter Krempa (4):
qemu: migration: Refactor cleanup in
qemuMigrationSrcNBDStorageCopyBlockdev
qemu: migration: Refactor cleanup in
qemuMigrationSrcNBDStorageCopyDriveMirror
qemu: migration: Refactor cleanup in qemuMigrationSrcNBDStorageCopy
qemu: migration: Switch to blockdev mode for non-shared storage
migration
In spite of my question on patch 2, series:
Reviewed-by: Eric Blake <eblake(a)redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
1904
days inactive
1905
days old
7 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa