[libvirt PATCH 0/7] qemu: Add support for zero-copy migration
This series also fixes a few issues around memory locking limit in migration code which I found while implementing the feature. Jiri Denemark (7): qemu: Add qemuDomainSetMaxMemLock helper qemu_migration: Use qemuDomainSetMaxMemLock qemu_migration: Restore original memory locking limit qemu_migration: Don't set unlimited memlock limit for RDMA Add VIR_MIGRATE_ZEROCOPY flag virsh: Add support for VIR_MIGRATE_ZEROCOPY flag qemu_migration: Implement VIR_MIGRATE_ZEROCOPY flag docs/manpages/virsh.rst | 7 +- include/libvirt/libvirt-domain.h | 9 +++ src/qemu/qemu_domain.c | 107 ++++++++++++++++++++----------- src/qemu/qemu_domain.h | 6 ++ src/qemu/qemu_migration.c | 34 ++++++++-- src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 6 ++ src/qemu/qemu_migration_params.h | 1 + tools/virsh-domain.c | 7 ++ 9 files changed, 136 insertions(+), 42 deletions(-) -- 2.35.1
qemuDomainAdjustMaxMemLock combined computing the desired limit with applying it. This patch separates the code to apply a memory locking limit to a new qemuDomainSetMaxMemLock helper for better reusability. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 95 ++++++++++++++++++++++++++---------------- src/qemu/qemu_domain.h | 3 ++ 2 files changed, 61 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9769e3bb92..e363993739 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9459,6 +9459,61 @@ qemuDomainGetMemLockLimitBytes(virDomainDef *def, } +/** + * qemuDomainSetMaxMemLock: + * @vm: domain + * @limit: the desired memory locking limit + * @origPtr: where to store (or load from) the original value of the limit + * + * Set the memory locking limit for @vm unless it's already big enough. If + * @origPtr is non-NULL, the original value of the limit will be store there + * and can be restored by calling this function with @limit == 0. + * + * Returns: 0 on success, -1 otherwise. + */ +int +qemuDomainSetMaxMemLock(virDomainObj *vm, + unsigned long long limit, + unsigned long long *origPtr) +{ + unsigned long long current = 0; + + if (virProcessGetMaxMemLock(vm->pid, ¤t) < 0) + return -1; + + if (limit > 0) { + VIR_DEBUG("Requested memory lock limit: %llu", limit); + /* If the limit is already high enough, we can assume + * that some external process is taking care of managing + * process limits and we shouldn't do anything ourselves: + * we're probably running in a containerized environment + * where we don't have enough privilege anyway */ + if (current >= limit) { + VIR_DEBUG("Current limit %llu is big enough", current); + return 0; + } + + /* If this is the first time adjusting the limit, save the current + * value so that we can restore it once memory locking is no longer + * required */ + if (origPtr && *origPtr == 0) + *origPtr = current; + } else { + /* Once memory locking is no longer required, we can restore the + * original, usually very low, limit. But only if we actually stored + * the original limit before. */ + if (!origPtr || *origPtr == 0) + return 0; + + limit = *origPtr; + *origPtr = 0; + VIR_DEBUG("Resetting memory lock limit back to %llu", limit); + } + + return virProcessSetMaxMemLock(vm->pid, limit); +} + + /** * qemuDomainAdjustMaxMemLock: * @vm: domain @@ -9480,43 +9535,9 @@ int qemuDomainAdjustMaxMemLock(virDomainObj *vm, bool forceVFIO) { - qemuDomainObjPrivate *priv = vm->privateData; - unsigned long long currentMemLock = 0; - unsigned long long desiredMemLock = 0; - - desiredMemLock = qemuDomainGetMemLockLimitBytes(vm->def, forceVFIO); - if (virProcessGetMaxMemLock(vm->pid, ¤tMemLock) < 0) - return -1; - - if (desiredMemLock > 0) { - if (currentMemLock < desiredMemLock) { - /* If this is the first time adjusting the limit, save the current - * value so that we can restore it once memory locking is no longer - * required */ - if (priv->originalMemlock == 0) { - priv->originalMemlock = currentMemLock; - } - } else { - /* If the limit is already high enough, we can assume - * that some external process is taking care of managing - * process limits and we shouldn't do anything ourselves: - * we're probably running in a containerized environment - * where we don't have enough privilege anyway */ - desiredMemLock = 0; - } - } else { - /* Once memory locking is no longer required, we can restore the - * original, usually very low, limit */ - desiredMemLock = priv->originalMemlock; - priv->originalMemlock = 0; - } - - if (desiredMemLock > 0 && - virProcessSetMaxMemLock(vm->pid, desiredMemLock) < 0) { - return -1; - } - - return 0; + return qemuDomainSetMaxMemLock(vm, + qemuDomainGetMemLockLimitBytes(vm->def, forceVFIO), + &QEMU_DOMAIN_PRIVATE(vm)->originalMemlock); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index a87dfff1bb..6d35f61dfd 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -840,6 +840,9 @@ int qemuDomainAdjustMaxMemLock(virDomainObj *vm, bool forceVFIO); int qemuDomainAdjustMaxMemLockHostdev(virDomainObj *vm, virDomainHostdevDef *hostdev); +int qemuDomainSetMaxMemLock(virDomainObj *vm, + unsigned long long limit, + unsigned long long *origPtr); int qemuDomainDefValidateMemoryHotplug(const virDomainDef *def, const virDomainMemoryDef *mem); -- 2.35.1
This helper will not try to set the limit if it is already big enough, which may be useful when libvirt daemon is running in a containerized environment and is not allowed to change memory locking limit. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d93e7e9c74..fe63f45629 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3181,7 +3181,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, if (STREQ_NULLABLE(protocol, "rdma") && vm->def->mem.hard_limit > 0 && - virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) { + qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, NULL) < 0) { goto error; } @@ -4615,7 +4615,7 @@ qemuMigrationSrcStart(virDomainObj *vm, case MIGRATION_DEST_HOST: if (STREQ(spec->dest.host.protocol, "rdma") && vm->def->mem.hard_limit > 0 && - virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) { + qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, NULL) < 0) { return -1; } return qemuMonitorMigrateToHost(priv->mon, migrateFlags, -- 2.35.1
For RDMA migration we update memory locking limit, but never set it back once migration finishes (on the destination host) or aborts (on the source host). Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_migration.c | 11 ++++++++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index e363993739..60ed358871 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2413,6 +2413,11 @@ qemuDomainObjPrivateXMLFormat(virBuffer *buf, priv->originalMemlock); } + if (priv->preMigrationMemlock > 0) { + virBufferAsprintf(buf, "<preMigrationMemlock>%llu</preMigrationMemlock>\n", + priv->preMigrationMemlock); + } + return 0; } @@ -3139,6 +3144,13 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, return -1; } + if (virXPathULongLong("string(./preMigrationMemlock)", ctxt, + &priv->preMigrationMemlock) == -2) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse pre-migration memlock limit")); + return -1; + } + return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 6d35f61dfd..499ad03f91 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -140,6 +140,9 @@ struct _qemuDomainObjPrivate { int nbdPort; /* Port used for migration with NBD */ unsigned short migrationPort; int preMigrationState; + unsigned long long preMigrationMemlock; /* Original RLIMIT_MEMLOCK in case + it was changed for the current + migration job. */ virChrdevs *devs; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fe63f45629..ffae5576d2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3181,7 +3181,8 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, if (STREQ_NULLABLE(protocol, "rdma") && vm->def->mem.hard_limit > 0 && - qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, NULL) < 0) { + qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, + &priv->preMigrationMemlock) < 0) { goto error; } @@ -3945,6 +3946,7 @@ qemuMigrationSrcComplete(virQEMUDriver *driver, VIR_DOMAIN_EVENT_STOPPED_MIGRATED); virObjectEventStateQueue(driver->domainEventState, event); qemuDomainEventEmitJobCompleted(driver, vm); + priv->preMigrationMemlock = 0; } @@ -4035,6 +4037,7 @@ qemuMigrationSrcConfirmPhase(virQEMUDriver *driver, qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, jobPriv->migParams, priv->job.apiFlags); + qemuDomainSetMaxMemLock(vm, 0, &priv->preMigrationMemlock); } qemuDomainSaveStatus(vm); @@ -4615,7 +4618,8 @@ qemuMigrationSrcStart(virDomainObj *vm, case MIGRATION_DEST_HOST: if (STREQ(spec->dest.host.protocol, "rdma") && vm->def->mem.hard_limit > 0 && - qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, NULL) < 0) { + qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, + &priv->preMigrationMemlock) < 0) { return -1; } return qemuMonitorMigrateToHost(priv->mon, migrateFlags, @@ -6155,6 +6159,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, qemuMigrationSrcRestoreDomainState(driver, vm); qemuMigrationParamsReset(driver, vm, VIR_ASYNC_JOB_MIGRATION_OUT, jobPriv->migParams, priv->job.apiFlags); + qemuDomainSetMaxMemLock(vm, 0, &priv->preMigrationMemlock); qemuMigrationJobFinish(vm); } else { if (ret < 0) @@ -6410,7 +6415,7 @@ qemuMigrationDstComplete(virQEMUDriver *driver, job->apiFlags); virPortAllocatorRelease(priv->migrationPort); - priv->migrationPort = 0; + qemuDomainSetMaxMemLock(vm, 0, &priv->preMigrationMemlock); } -- 2.35.1
On a Thursday in 2022, Jiri Denemark wrote:
For RDMA migration we update memory locking limit, but never set it back once migration finishes (on the destination host) or aborts (on the source host).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_migration.c | 11 ++++++++--- 3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fe63f45629..ffae5576d2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6410,7 +6415,7 @@ qemuMigrationDstComplete(virQEMUDriver *driver, job->apiFlags);
virPortAllocatorRelease(priv->migrationPort); - priv->migrationPort = 0;
This removal is suspicious. Jano
+ qemuDomainSetMaxMemLock(vm, 0, &priv->preMigrationMemlock); }
-- 2.35.1
On Thu, Jun 23, 2022 at 16:13:50 +0200, Ján Tomko wrote:
On a Thursday in 2022, Jiri Denemark wrote:
For RDMA migration we update memory locking limit, but never set it back once migration finishes (on the destination host) or aborts (on the source host).
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 12 ++++++++++++ src/qemu/qemu_domain.h | 3 +++ src/qemu/qemu_migration.c | 11 ++++++++--- 3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fe63f45629..ffae5576d2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -6410,7 +6415,7 @@ qemuMigrationDstComplete(virQEMUDriver *driver, job->apiFlags);
virPortAllocatorRelease(priv->migrationPort); - priv->migrationPort = 0;
This removal is suspicious.
Oops, no idea how that happened. Thanks for catching it. Jirka
Our documentation says RDMA migration requires hard_limit to be set so that we know how big memory locking limit should be set for the domain during migration. But since commit v1.2.13-71-gcf521fc8ba (which changed the default hard_limit value from 0 to VIR_DOMAIN_MEMORY_PARAM_UNLIMITED) we were actually setting memlock limit to unlimited if hard_limit was not set. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ffae5576d2..272f1b1b59 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3180,7 +3180,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, } if (STREQ_NULLABLE(protocol, "rdma") && - vm->def->mem.hard_limit > 0 && + virMemoryLimitIsSet(vm->def->mem.hard_limit) && qemuDomainSetMaxMemLock(vm, vm->def->mem.hard_limit << 10, &priv->preMigrationMemlock) < 0) { goto error; -- 2.35.1
The flag can be used to enable zero-copy mechanism for migrating memory pages. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- include/libvirt/libvirt-domain.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 1ea3284e63..05344aaa95 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1089,6 +1089,15 @@ typedef enum { * Since: 8.5.0 */ VIR_MIGRATE_POSTCOPY_RESUME = (1 << 19), + + /* Use zero-copy mechanism for migrating memory pages. For QEMU/KVM this + * means QEMU will be temporarily allowed to lock all guest pages in host's + * memory, although only those that are queued for transfer will be locked + * at the same time. + * + * Since: 8.5.0 + */ + VIR_MIGRATE_ZEROCOPY = (1 << 20), } virDomainMigrateFlags; -- 2.35.1
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- docs/manpages/virsh.rst | 7 ++++++- tools/virsh-domain.c | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 4701791b04..45469f2f35 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3285,7 +3285,7 @@ migrate [--persistent] [--undefinesource] [--suspend] [--copy-storage-all] [--copy-storage-inc] [--change-protection] [--unsafe] [--verbose] [--rdma-pin-all] [--abort-on-error] [--postcopy] - [--postcopy-after-precopy] [--postcopy-resume] + [--postcopy-after-precopy] [--postcopy-resume] [--zerocopy] domain desturi [migrateuri] [graphicsuri] [listen-address] [dname] [--timeout seconds [--timeout-suspend | --timeout-postcopy]] [--xml file] [--migrate-disks disk-list] [--disks-port port] @@ -3362,6 +3362,11 @@ high (and thus allowing the domain to lock most of the host's memory). Doing so may be dangerous to both the domain and the host itself since the host's kernel may run out of memory. +*--zerocopy* requests zero-copy mechanism to be used for migrating memory pages. +For QEMU/KVM this means QEMU will be temporarily allowed to lock all guest +pages in host's memory, although only those that are queued for transfer will +be locked at the same time. + ``Note``: Individual hypervisors usually do not support all possible types of migration. For example, QEMU does not support direct migration. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1d71ac5a67..43034f2f81 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10890,6 +10890,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_BOOL, .help = N_("resume failed post-copy migration") }, + {.name = "zerocopy", + .type = VSH_OT_BOOL, + .help = N_("use zero-copy mechanism for migrating memory pages") + }, {.name = "migrateuri", .type = VSH_OT_STRING, .completer = virshCompleteEmpty, @@ -11296,6 +11300,9 @@ doMigrate(void *opaque) if (vshCommandOptBool(cmd, "postcopy-resume")) flags |= VIR_MIGRATE_POSTCOPY_RESUME; + if (vshCommandOptBool(cmd, "zerocopy")) + flags |= VIR_MIGRATE_ZEROCOPY; + if (vshCommandOptBool(cmd, "tls")) flags |= VIR_MIGRATE_TLS; -- 2.35.1
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/306 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 21 +++++++++++++++++++++ src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 6 ++++++ src/qemu/qemu_migration_params.h | 1 + 4 files changed, 29 insertions(+) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 272f1b1b59..02a465f6cb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2634,6 +2634,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, } } + if (flags & VIR_MIGRATE_ZEROCOPY && !(flags & VIR_MIGRATE_PARALLEL)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("zero-copy is only available for parallel migration")); + return NULL; + } + if (flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { if (flags & VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES && !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { @@ -4798,6 +4804,21 @@ qemuMigrationSrcRun(virQEMUDriver *driver, migParams) < 0) goto error; + if (flags & VIR_MIGRATE_ZEROCOPY) { + /* Zero-copy requires pages in transfer to be locked in host memory. + * Unfortunately, we have no reliable way of computing how many pages + * will need to be locked at the same time. Thus we set the limit to + * the whole guest memory and reset it back once migration is done. */ + unsigned long long limit; + + if (virMemoryLimitIsSet(vm->def->mem.hard_limit)) + limit = vm->def->mem.hard_limit; + else + limit = virDomainDefGetMemoryTotal(vm->def); + + if (qemuDomainSetMaxMemLock(vm, limit << 10, &priv->preMigrationMemlock) < 0) + goto error; + } if (storageMigration) { if (mig->nbd) { diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index fbc0549b34..81cc1e91c0 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -61,6 +61,7 @@ VIR_MIGRATE_PARALLEL | \ VIR_MIGRATE_NON_SHARED_SYNCHRONOUS_WRITES | \ VIR_MIGRATE_POSTCOPY_RESUME | \ + VIR_MIGRATE_ZEROCOPY | \ 0) /* All supported migration parameters and their types. */ diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 4e83d8d8bd..cc66ed8229 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -94,6 +94,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "multifd", "dirty-bitmaps", "return-path", + "zero-copy-send", ); @@ -175,6 +176,11 @@ static const qemuMigrationParamsFlagMapItem qemuMigrationParamsFlagMap[] = { VIR_MIGRATE_TUNNELLED, QEMU_MIGRATION_CAP_RETURN_PATH, QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION}, + + {QEMU_MIGRATION_FLAG_REQUIRED, + VIR_MIGRATE_ZEROCOPY, + QEMU_MIGRATION_CAP_ZERO_COPY_SEND, + QEMU_MIGRATION_SOURCE}, }; /* Translation from VIR_MIGRATE_PARAM_* typed parameters to diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index f2f0090343..d1184acded 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -39,6 +39,7 @@ typedef enum { QEMU_MIGRATION_CAP_MULTIFD, QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS, QEMU_MIGRATION_CAP_RETURN_PATH, + QEMU_MIGRATION_CAP_ZERO_COPY_SEND, QEMU_MIGRATION_CAP_LAST } qemuMigrationCapability; -- 2.35.1
On Thu, Jun 23, 2022 at 03:58:12PM +0200, Jiri Denemark wrote:
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/306
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 21 +++++++++++++++++++++ src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 6 ++++++ src/qemu/qemu_migration_params.h | 1 + 4 files changed, 29 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 272f1b1b59..02a465f6cb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2634,6 +2634,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, } }
+ if (flags & VIR_MIGRATE_ZEROCOPY && !(flags & VIR_MIGRATE_PARALLEL)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("zero-copy is only available for parallel migration")); + return NULL; + }
It is also not compatible with compression, or TLS.
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 4e83d8d8bd..cc66ed8229 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -94,6 +94,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "multifd", "dirty-bitmaps", "return-path", + "zero-copy-send", );
Note the QEMU name was picked in case we later get zero copy receive, as a separately enabled feature. With 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 :|
On Thu, Jun 23, 2022 at 15:08:30 +0100, Daniel P. Berrangé wrote:
On Thu, Jun 23, 2022 at 03:58:12PM +0200, Jiri Denemark wrote:
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/306
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 21 +++++++++++++++++++++ src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 6 ++++++ src/qemu/qemu_migration_params.h | 1 + 4 files changed, 29 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 272f1b1b59..02a465f6cb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2634,6 +2634,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, } }
+ if (flags & VIR_MIGRATE_ZEROCOPY && !(flags & VIR_MIGRATE_PARALLEL)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("zero-copy is only available for parallel migration")); + return NULL; + }
It is also not compatible with compression, or TLS.
Yeah, no zero-copy when you need to transform the data in any way. I think QEMU provides a reasonable error message already. But since it mentions "multifd" while our flag is called "parallel", I decided to explicitly handle this case: internal error: unable to execute QEMU command 'migrate-set-capabilities': Zero copy only available for non-compressed non-TLS multifd migration Do you think I should explicitly check all flags instead?
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 4e83d8d8bd..cc66ed8229 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -94,6 +94,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "multifd", "dirty-bitmaps", "return-path", + "zero-copy-send", );
Note the QEMU name was picked in case we later get zero copy receive, as a separately enabled feature.
The question is whether we need to do the same or if we can have a single zero copy which would enable the right one depending on the side of migration... Jirka
On Thu, Jun 23, 2022 at 04:20:08PM +0200, Jiri Denemark wrote:
On Thu, Jun 23, 2022 at 15:08:30 +0100, Daniel P. Berrangé wrote:
On Thu, Jun 23, 2022 at 03:58:12PM +0200, Jiri Denemark wrote:
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/306
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 21 +++++++++++++++++++++ src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 6 ++++++ src/qemu/qemu_migration_params.h | 1 + 4 files changed, 29 insertions(+)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 272f1b1b59..02a465f6cb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2634,6 +2634,12 @@ qemuMigrationSrcBeginPhase(virQEMUDriver *driver, } }
+ if (flags & VIR_MIGRATE_ZEROCOPY && !(flags & VIR_MIGRATE_PARALLEL)) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("zero-copy is only available for parallel migration")); + return NULL; + }
It is also not compatible with compression, or TLS.
Yeah, no zero-copy when you need to transform the data in any way. I think QEMU provides a reasonable error message already. But since it mentions "multifd" while our flag is called "parallel", I decided to explicitly handle this case:
internal error: unable to execute QEMU command 'migrate-set-capabilities': Zero copy only available for non-compressed non-TLS multifd migration
Do you think I should explicitly check all flags instead?
I'm not fussed, I just wanted to mention it in case it was important.
diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 4e83d8d8bd..cc66ed8229 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -94,6 +94,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "multifd", "dirty-bitmaps", "return-path", + "zero-copy-send", );
Note the QEMU name was picked in case we later get zero copy receive, as a separately enabled feature.
The question is whether we need to do the same or if we can have a single zero copy which would enable the right one depending on the side of migration...
The main reason for needing a flag in QEMU was because mgmt apps need to aware of the mem pinning requirements. QEMU wants separate flags so it doesn't silently chuange behaviour on upgrade and break mgmt apps. Overall it seems like libvirt has enough knowledge to do the right thing without needing two flags. With 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 :|
On a Thursday in 2022, Jiri Denemark wrote:
This series also fixes a few issues around memory locking limit in migration code which I found while implementing the feature.
Jiri Denemark (7): qemu: Add qemuDomainSetMaxMemLock helper qemu_migration: Use qemuDomainSetMaxMemLock qemu_migration: Restore original memory locking limit qemu_migration: Don't set unlimited memlock limit for RDMA Add VIR_MIGRATE_ZEROCOPY flag virsh: Add support for VIR_MIGRATE_ZEROCOPY flag qemu_migration: Implement VIR_MIGRATE_ZEROCOPY flag
docs/manpages/virsh.rst | 7 +- include/libvirt/libvirt-domain.h | 9 +++ src/qemu/qemu_domain.c | 107 ++++++++++++++++++++----------- src/qemu/qemu_domain.h | 6 ++ src/qemu/qemu_migration.c | 34 ++++++++-- src/qemu/qemu_migration.h | 1 + src/qemu/qemu_migration_params.c | 6 ++ src/qemu/qemu_migration_params.h | 1 + tools/virsh-domain.c | 7 ++ 9 files changed, 136 insertions(+), 42 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Daniel P. Berrangé -
Jiri Denemark -
Ján Tomko