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(a)redhat.com&gt; --- 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, &current) < 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, &currentMemLock) < 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

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(a)redhat.com&gt; --- 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 Thu, Jun 23, 2022 at 16:13:50 +0200, Ján Tomko wrote: Oops, no idea how that happened. Thanks for catching it. Jirka

The flag can be used to enable zero-copy mechanism for migrating memory pages. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- 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

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/306 Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- 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 15:08:30 +0100, Daniel P. Berrangé wrote: 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? 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