On Wed, Mar 05, 2025 at 03:48:10PM -0700, Jim Fehlig via Devel wrote: Are you aware of any common usage of these APIs with a relative path ? Although we've not documented it, my general view is that all paths given to libvirt are expected to be absolute, everywhere - whether API parameters like these, or config file parmeters, or XML elements/attributes. In the perfect world we would canonicalize all relative paths on the client side, but doing that is such an enourmous & complex job it is not likely to be practical. We'd be better off just documenting use of relative paths as "out of scope" and / or "undefined behaviour" 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 :|

Add new function qemuFDPassNewFromMonitor to get an fdset previously passed to qemu, based on the 'prefix' provided when the qemuFDPass object was initially created. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_fd.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_fd.h | 4 ++++ 2 files changed, 50 insertions(+) diff --git a/src/qemu/qemu_fd.c b/src/qemu/qemu_fd.c index e847056573..333f9b128e 100644 --- a/src/qemu/qemu_fd.c +++ b/src/qemu/qemu_fd.c @@ -115,6 +115,52 @@ qemuFDPassNewPassed(unsigned int fdSetID) } +/** + * qemuFDPassNewFromMonitor: + * @prefix: Prefix of an FDset which was already passed to qemu + * @mon: monitor object + * + * Query the monitor for already passed FDsets and return a new qemuFDPass + * object if one is found to contain an fd with associated @prefix. Returns + * NULL on failure or if no matching FDset is found. Similar to + * qemuFDPassNewPassed, this is useful along with qemuFDPassTransferMonitorRollback + * when only knowing the qemuFDPass prefix. + */ +qemuFDPass * +qemuFDPassNewFromMonitor(const char *prefix, qemuMonitor *mon) +{ + g_autoptr(qemuMonitorFdsets) fdsets = NULL; + qemuFDPass *fdpass = NULL; + size_t i; + + VIR_DEBUG("prefix = %s", prefix); + + if (qemuMonitorQueryFdsets(mon, &fdsets) < 0) + return NULL; + + for (i = 0; i < fdsets->nfdsets; i++) { + qemuMonitorFdsetInfo fdset = fdsets->fdsets[i]; + size_t j; + + for (j = 0; j < fdset.nfds; j++) { + qemuMonitorFdsetFdInfo fdinfo = fdset.fds[j]; + + VIR_DEBUG("fdinfo opaque = %s", fdinfo.opaque); + if (STRPREFIX(prefix, fdinfo.opaque)) { + fdpass = g_new0(qemuFDPass, 1); + + fdpass->fdSetID = fdset.id; + fdpass->prefix = g_strdup(prefix); + fdpass->path = g_strdup_printf("/dev/fdset/%u", fdset.id); + fdpass->passed = true; + } + } + } + + return fdpass; +} + + /** * qemuFDPassIsPassed: * @fdpass: The fd passing helper struct diff --git a/src/qemu/qemu_fd.h b/src/qemu/qemu_fd.h index cd0ff2c690..d873e110a8 100644 --- a/src/qemu/qemu_fd.h +++ b/src/qemu/qemu_fd.h @@ -34,6 +34,10 @@ qemuFDPassNew(const char *prefix, qemuFDPass * qemuFDPassNewPassed(unsigned int fdSetID); +qemuFDPass * +qemuFDPassNewFromMonitor(const char *prefix, qemuMonitor *mon) + ATTRIBUTE_NONNULL(1); + bool qemuFDPassIsPassed(qemuFDPass *fdpass, unsigned *id); -- 2.43.0

Add new function qemuMigrationParamsCapEnabled() to check if a capability is set in the caller-provided migration parameters. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/qemu/qemu_migration_params.c | 16 ++++++++++++++++ src/qemu/qemu_migration_params.h | 4 ++++ 2 files changed, 20 insertions(+) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index c10660d6f2..252437e5b2 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -1312,6 +1312,22 @@ qemuMigrationParamsGetULL(qemuMigrationParams *migParams, } +/** + * Returns true if @cap is enabled in @migParams, false otherwise. + */ +bool +qemuMigrationParamsCapEnabled(qemuMigrationParams *migParams, + qemuMigrationCapability cap) +{ + bool enabled = false; + + if (migParams) + ignore_value(virBitmapGetBit(migParams->caps, cap, &enabled)); + + return enabled; +} + + /** * qemuMigrationParamsCheck: * diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 17fc63f527..da62ca5b8f 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -137,6 +137,10 @@ qemuMigrationParamsGetULL(qemuMigrationParams *migParams, qemuMigrationParam param, unsigned long long *value); +bool +qemuMigrationParamsCapEnabled(qemuMigrationParams *migParams, + qemuMigrationCapability cap); + void qemuMigrationParamsSetBlockDirtyBitmapMapping(qemuMigrationParams *migParams, virJSONValue **params); -- 2.43.0

Add the mapped-ram migration capability introduced in QEMU 9.0. Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_migration_params.c | 1 + src/qemu/qemu_migration_params.h | 1 + 2 files changed, 2 insertions(+) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 939d9f01e4..4fc9199f1c 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -106,6 +106,7 @@ VIR_ENUM_IMPL(qemuMigrationCapability, "zero-copy-send", "postcopy-preempt", "switchover-ack", + "mapped-ram", ); diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index fb2cfd9fad..2379950ce9 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -42,6 +42,7 @@ typedef enum { QEMU_MIGRATION_CAP_ZERO_COPY_SEND, QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT, QEMU_MIGRATION_CAP_SWITCHOVER_ACK, + QEMU_MIGRATION_CAP_MAPPED_RAM, QEMU_MIGRATION_CAP_LAST } qemuMigrationCapability; -- 2.43.0

Extend the list of formats to include "sparse", which uses QEMU's mapped-ram stream format [1] to write guest memory blocks at fixed offsets in the save image file. [1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/m... Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu.conf.in | 9 ++++++++- src/qemu/qemu_saveimage.c | 1 + src/qemu/qemu_saveimage.h | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index 31172303dc..44ff658ad6 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -588,11 +588,18 @@ # disk space when saving large memory guests. Various compression formats are # available for specifying a save image compressed by the named algorithm. # Supported compression formats are "zstd", "lzop", "gzip", "bzip2", and "xz". +# The "sparse" format uses QEMU's mapped-ram stream format to write guest memory +# blocks at fixed offsets in the save image file. # save_image_format can be used to select the desired save format. "raw" is # the traditional format used by libvirt and is also the default. The # compression formats can be used to save disk space, although this typically -# results in longer save and restore times. +# results in longer save and restore times. The "sparse" format results in a +# save image file that is roughly the logical size of the guest's memory, +# although on-disk size is a function of guest memory usage. The "sparse" +# format is useful when a predictable maximum save image file size is +# needed. The other formats can result in a save image file much larger +# than guest memory if the guest runs a memory intensive workload. # # save_image_format is used with 'virsh save' or 'virsh managedsave'. It is # an error if the specified save_image_format is not valid, or cannot be diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index d940bfb5c3..29b4e39879 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -45,6 +45,7 @@ VIR_ENUM_IMPL(qemuSaveFormat, "xz", "lzop", "zstd", + "sparse", ); static inline void diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 58f8252c8a..0411f3140a 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -42,6 +42,7 @@ typedef enum { QEMU_SAVE_FORMAT_XZ = 3, QEMU_SAVE_FORMAT_LZOP = 4, QEMU_SAVE_FORMAT_ZSTD = 5, + QEMU_SAVE_FORMAT_SPARSE = 6, /* Note: add new members only at the end. These values are used in the on-disk format. Do not change or re-use numbers. */ -- 2.43.0

Move the code in qemuSaveImageCreate that opens, labels, and wraps the save image fd to a helper function, providing more flexibility for upcoming mapped-ram support. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_saveimage.c | 65 +++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 29b4e39879..e3f6a0ad0f 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -425,6 +425,50 @@ qemuSaveImageDecompressionStop(virCommand *cmd, } +static int +qemuSaveImageCreateFd(virQEMUDriver *driver, + virDomainObj *vm, + const char *path, + virFileWrapperFd *wrapperFd, + bool *needUnlink, + unsigned int flags) +{ + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + int ret = -1; + VIR_AUTOCLOSE fd = -1; + int directFlag = 0; + unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; + + if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) { + wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE; + directFlag = virFileDirectFdFlag(); + if (directFlag < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("bypass cache unsupported by this system")); + return -1; + } + } + + fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, path, + O_WRONLY | O_TRUNC | O_CREAT | directFlag, + needUnlink); + + if (fd < 0) + return -1; + + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) + return -1; + + if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) + return -1; + + ret = fd; + fd = -1; + + return ret; +} + + /* Helper function to execute a migration to file with a correct save header * the caller needs to make sure that the processors are stopped and do all other * actions besides saving memory */ @@ -441,33 +485,14 @@ qemuSaveImageCreate(virQEMUDriver *driver, bool needUnlink = false; int ret = -1; int fd = -1; - int directFlag = 0; virFileWrapperFd *wrapperFd = NULL; - unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; /* Obtain the file handle. */ - if ((flags & VIR_DOMAIN_SAVE_BYPASS_CACHE)) { - wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE; - directFlag = virFileDirectFdFlag(); - if (directFlag < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("bypass cache unsupported by this system")); - goto cleanup; - } - } + fd = qemuSaveImageCreateFd(driver, vm, path, wrapperFd, &needUnlink, flags); - fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, path, - O_WRONLY | O_TRUNC | O_CREAT | directFlag, - &needUnlink); if (fd < 0) goto cleanup; - if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) - goto cleanup; - - if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) - goto cleanup; - if (virQEMUSaveDataWrite(data, fd, path) < 0) goto cleanup; -- 2.43.0

On 3/19/25 06:49, Daniel P. Berrangé wrote: Opps, which makes qemuDomainFileWrapperFDClose a NOP in all cases. I've applied the below diff locally. Any need to respin this series? Regards, Jim diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index e3f6a0ad0f..7ab44edcdc 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -429,7 +429,7 @@ static int qemuSaveImageCreateFd(virQEMUDriver *driver, virDomainObj *vm, const char *path, - virFileWrapperFd *wrapperFd, + virFileWrapperFd **wrapperFd, bool *needUnlink, unsigned int flags) { @@ -459,7 +459,7 @@ qemuSaveImageCreateFd(virQEMUDriver *driver, if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) return -1; - if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) + if (!(*wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) return -1; ret = fd; @@ -488,7 +488,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, virFileWrapperFd *wrapperFd = NULL; /* Obtain the file handle. */ - fd = qemuSaveImageCreateFd(driver, vm, path, wrapperFd, &needUnlink, flags); + fd = qemuSaveImageCreateFd(driver, vm, path, &wrapperFd, &needUnlink, flags); if (fd < 0) goto cleanup;

Introduce support for QEMU's new mapped-ram stream format [1]. mapped-ram can be enabled by setting the 'save_image_format' setting in qemu.conf to 'sparse'. To use mapped-ram with QEMU: - The 'mapped-ram' migration capability must be set to true - The 'multifd' migration capability must be set to true and the 'multifd-channels' migration parameter must set to 1 - QEMU must be provided an fdset containing the migration fd - The 'migrate' qmp command is invoked with a URI referencing the fdset and an offset where to start reading or writing the data stream, e.g. {"execute":"migrate", "arguments":{"detach":true,"resume":false, "uri":"file:/dev/fdset/0,offset=0x11921"}} The mapped-ram stream, in conjunction with direct IO and multifd support provided by subsequent patches, can significantly improve the time required to save VM memory state. The following tables compare mapped-ram with the existing, sequential save stream. In all cases, the save and restore operations are to/from a block device comprised of two NVMe disks in RAID0 configuration with xfs (~8600MiB/s). The values in the 'save time' and 'restore time' columns were scraped from the 'real' time reported by time(1). The 'Size' and 'Blocks' columns were provided by the corresponding outputs of stat(1). VM: 32G RAM, 1 vcpu, idle (shortly after boot) | save | restore | | time | time | Size | Blocks -----------------------+---------+---------+--------------+-------- legacy | 6.193s | 4.399s | 985744812 | 1925288 -----------------------+---------+---------+--------------+-------- mapped-ram | 5.109s | 1.176s | 34368554354 | 1774472 -----------------------+---------+---------+--------------+-------- legacy + direct IO | 5.725s | 4.512s | 985765251 | 1925328 -----------------------+---------+---------+--------------+-------- mapped-ram + direct IO | 4.627s | 1.490s | 34368554354 | 1774304 -----------------------+---------+---------+--------------+-------- mapped-ram + direct IO | | | | + multifd-channels=8 | 4.421s | 0.845s | 34368554318 | 1774312 ------------------------------------------------------------------- VM: 32G RAM, 30G dirty, 1 vcpu in tight loop dirtying memory | save | restore | | time | time | Size | Blocks -----------------------+---------+---------+--------------+--------- legacy | 25.800s | 14.332s | 33154309983 | 64754512 -----------------------+---------+---------+--------------+--------- mapped-ram | 18.742s | 15.027s | 34368559228 | 64617160 -----------------------+---------+---------+--------------+--------- legacy + direct IO | 13.115s | 18.050s | 33154310496 | 64754520 -----------------------+---------+---------+--------------+--------- mapped-ram + direct IO | 13.623s | 15.959s | 34368557392 | 64662040 -----------------------+-------- +---------+--------------+--------- mapped-ram + direct IO | | | | + multifd-channels=8 | 6.994s | 6.470s | 34368554980 | 64665776 -------------------------------------------------------------------- As can be seen from the tables, one caveat of mapped-ram is the logical file size of a saved image is basically equivalent to the VM memory size. Note however that mapped-ram typically uses fewer blocks on disk, hence the name 'sparse' for 'save_image_format'. Also note the mapped-ram stream is incompatible with the existing stream format, hence mapped-ram cannot be used to restore an image saved with the existing format and vice versa. [1] https://gitlab.com/qemu-project/qemu/-/blob/master/docs/devel/migration/m... Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_driver.c | 23 +++++- src/qemu/qemu_migration.c | 149 ++++++++++++++++++++++++++------------ src/qemu/qemu_migration.h | 4 +- src/qemu/qemu_monitor.c | 34 +++++++++ src/qemu/qemu_monitor.h | 4 + src/qemu/qemu_saveimage.c | 31 +++++--- src/qemu/qemu_saveimage.h | 1 + src/qemu/qemu_snapshot.c | 7 +- 8 files changed, 191 insertions(+), 62 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 53bdae402a..2b2dda9d26 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2599,6 +2599,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, qemuDomainObjPrivate *priv = vm->privateData; virQEMUSaveData *data = NULL; g_autoptr(qemuDomainSaveCookie) cookie = NULL; + g_autoptr(qemuMigrationParams) saveParams = NULL; if (virDomainObjBeginAsyncJob(vm, VIR_ASYNC_JOB_SAVE, VIR_DOMAIN_JOB_OPERATION_SAVE, flags) < 0) @@ -2607,6 +2608,14 @@ qemuDomainSaveInternal(virQEMUDriver *driver, if (!qemuMigrationSrcIsAllowed(vm, false, VIR_ASYNC_JOB_SAVE, 0)) goto endjob; + if (format == QEMU_SAVE_FORMAT_SPARSE && + !qemuMigrationCapsGet(vm, QEMU_MIGRATION_CAP_MAPPED_RAM)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("save image format %1$s is not supported by this QEMU binary"), + qemuSaveFormatTypeToString(format)); + goto endjob; + } + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest unexpectedly quit")); @@ -2670,8 +2679,11 @@ qemuDomainSaveInternal(virQEMUDriver *driver, goto endjob; xml = NULL; + if (!(saveParams = qemuMigrationParamsForSave(format == QEMU_SAVE_FORMAT_SPARSE))) + goto endjob; + ret = qemuSaveImageCreate(driver, vm, path, data, compressor, - flags, VIR_ASYNC_JOB_SAVE); + saveParams, flags, VIR_ASYNC_JOB_SAVE); if (ret < 0) goto endjob; @@ -3103,6 +3115,8 @@ doCoreDump(virQEMUDriver *driver, memory_dump_format) < 0) goto cleanup; } else { + g_autoptr(qemuMigrationParams) dump_params = NULL; + if (dumpformat != VIR_DOMAIN_CORE_DUMP_FORMAT_RAW) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("kdump-compressed format is only supported with memory-only dump")); @@ -3112,8 +3126,11 @@ doCoreDump(virQEMUDriver *driver, if (!qemuMigrationSrcIsAllowed(vm, false, VIR_ASYNC_JOB_DUMP, 0)) goto cleanup; - if (qemuMigrationSrcToFile(driver, vm, fd, compressor, - VIR_ASYNC_JOB_DUMP) < 0) + if (!(dump_params = qemuMigrationParamsNew())) + goto cleanup; + + if (qemuMigrationSrcToFile(driver, vm, &fd, compressor, + dump_params, dump_flags, VIR_ASYNC_JOB_DUMP) < 0) goto cleanup; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8753fb5358..f692986056 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -7118,46 +7118,17 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, } -/* Helper function called while vm is active. */ -int -qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, - int fd, - virCommand *compressor, - virDomainAsyncJob asyncJob) +static int +qemuMigrationSrcToLegacyFile(virQEMUDriver *driver, + virDomainObj *vm, + int fd, + virCommand *compressor, + virDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; - int rc; int ret = -1; int pipeFD[2] = { -1, -1 }; - unsigned long saveMigBandwidth = priv->migMaxBandwidth; char *errbuf = NULL; - virErrorPtr orig_err = NULL; - g_autoptr(qemuMigrationParams) migParams = NULL; - - if (qemuMigrationSetDBusVMState(driver, vm) < 0) - return -1; - - /* Increase migration bandwidth to unlimited since target is a file. - * Failure to change migration speed is not fatal. */ - if (!(migParams = qemuMigrationParamsForSave(false))) - return -1; - - if (qemuMigrationParamsSetULL(migParams, - QEMU_MIGRATION_PARAM_MAX_BANDWIDTH, - QEMU_DOMAIN_MIG_BANDWIDTH_MAX * 1024 * 1024) < 0) - return -1; - - if (qemuMigrationParamsApply(vm, asyncJob, migParams, 0) < 0) - return -1; - - priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; - - if (!virDomainObjIsActive(vm)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); - /* nothing to tear down */ - return -1; - } if (compressor && virPipe(pipeFD) < 0) return -1; @@ -7174,7 +7145,7 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, goto cleanup; if (!compressor) { - rc = qemuMonitorMigrateToFd(priv->mon, 0, fd); + ret = qemuMonitorMigrateToFd(priv->mon, 0, fd); } else { virCommandSetInputFD(compressor, pipeFD[0]); virCommandSetOutputFD(compressor, &fd); @@ -7190,12 +7161,98 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, qemuDomainObjExitMonitor(vm); goto cleanup; } - rc = qemuMonitorMigrateToFd(priv->mon, 0, pipeFD[1]); + ret = qemuMonitorMigrateToFd(priv->mon, 0, pipeFD[1]); if (VIR_CLOSE(pipeFD[0]) < 0 || VIR_CLOSE(pipeFD[1]) < 0) VIR_WARN("failed to close intermediate pipe"); } qemuDomainObjExitMonitor(vm); + + cleanup: + VIR_FORCE_CLOSE(pipeFD[0]); + VIR_FORCE_CLOSE(pipeFD[1]); + + if (errbuf) { + VIR_DEBUG("Compression binary stderr: %s", NULLSTR(errbuf)); + VIR_FREE(errbuf); + } + + return ret; +} + + +static int +qemuMigrationSrcToSparseFile(virQEMUDriver *driver, + virDomainObj *vm, + int *fd, + unsigned int flags, + virDomainAsyncJob asyncJob) +{ + int ret; + + /* mapped-ram does not support directIO */ + if ((flags & VIR_DOMAIN_SAVE_BYPASS_CACHE)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("bypass cache unsupported by this system")); + return -1; + } + + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, *fd) < 0) + return -1; + + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) + return -1; + + ret = qemuMonitorMigrateToFdSet(vm, 0, fd); + qemuDomainObjExitMonitor(vm); + return ret; +} + + +/* Helper function called while vm is active. */ +int +qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, + int *fd, + virCommand *compressor, + qemuMigrationParams *migParams, + unsigned int flags, + virDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivate *priv = vm->privateData; + int rc; + int ret = -1; + unsigned long saveMigBandwidth = priv->migMaxBandwidth; + virErrorPtr orig_err = NULL; + + if (qemuMigrationSetDBusVMState(driver, vm) < 0) + return -1; + + /* Increase migration bandwidth to unlimited since target is a file. + * Failure to change migration speed is not fatal. */ + if (migParams && + qemuMigrationParamsSetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_BANDWIDTH, + QEMU_DOMAIN_MIG_BANDWIDTH_MAX * 1024 * 1024) < 0) + return -1; + + if (qemuMigrationParamsApply(vm, asyncJob, migParams, 0) < 0) + return -1; + + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + /* nothing to tear down */ + return -1; + } + + if (migParams && + qemuMigrationParamsCapEnabled(migParams, QEMU_MIGRATION_CAP_MAPPED_RAM)) + rc = qemuMigrationSrcToSparseFile(driver, vm, fd, flags, asyncJob); + else + rc = qemuMigrationSrcToLegacyFile(driver, vm, *fd, compressor, asyncJob); + if (rc < 0) goto cleanup; @@ -7221,8 +7278,17 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, if (ret < 0 && !orig_err) virErrorPreserveLast(&orig_err); - /* Restore max migration bandwidth */ + /* Remove fdset passed to qemu and restore max migration bandwidth */ if (qemuDomainObjIsActive(vm)) { + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) == 0) { + qemuFDPass *fdPass = + qemuFDPassNewFromMonitor("libvirt-outgoing-migrate", priv->mon); + + if (fdPass) + qemuFDPassTransferMonitorRollback(fdPass, priv->mon); + qemuDomainObjExitMonitor(vm); + } + if (qemuMigrationParamsSetULL(migParams, QEMU_MIGRATION_PARAM_MAX_BANDWIDTH, saveMigBandwidth * 1024 * 1024) == 0) @@ -7231,13 +7297,6 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, priv->migMaxBandwidth = saveMigBandwidth; } - VIR_FORCE_CLOSE(pipeFD[0]); - VIR_FORCE_CLOSE(pipeFD[1]); - if (errbuf) { - VIR_DEBUG("Compression binary stderr: %s", NULLSTR(errbuf)); - VIR_FREE(errbuf); - } - virErrorRestore(&orig_err); return ret; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index efe1b9e88a..9fa007b949 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -238,8 +238,10 @@ qemuMigrationSrcIsAllowed(virDomainObj *vm, int qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, - int fd, + int *fd, virCommand *compressor, + qemuMigrationParams *migParams, + unsigned int flags, virDomainAsyncJob asyncJob) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c069d17265..8d244c6f4a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2232,6 +2232,40 @@ qemuMonitorMigrateToFd(qemuMonitor *mon, } +int +qemuMonitorMigrateToFdSet(virDomainObj *vm, + unsigned int flags, + int *fd) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuMonitor *mon = priv->mon; + off_t offset; + g_autoptr(qemuFDPass) fdPassMigrate = NULL; + g_autofree char *uri = NULL; + int ret; + + VIR_DEBUG("fd=%d flags=0x%x", *fd, flags); + + QEMU_CHECK_MONITOR(mon); + + if ((offset = lseek(*fd, 0, SEEK_CUR)) == -1) { + virReportSystemError(errno, + "%s", _("failed to seek on file descriptor")); + return -1; + } + + fdPassMigrate = qemuFDPassNew("libvirt-outgoing-migrate", priv); + qemuFDPassAddFD(fdPassMigrate, fd, "-fd"); + qemuFDPassTransferMonitor(fdPassMigrate, mon); + + uri = g_strdup_printf("file:%s,offset=%#lx", + qemuFDPassGetPath(fdPassMigrate), offset); + ret = qemuMonitorJSONMigrate(mon, flags, uri); + + return ret; +} + + int qemuMonitorMigrateToHost(qemuMonitor *mon, unsigned int flags, diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f7b9263b64..4022bc3c7b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -859,6 +859,10 @@ int qemuMonitorMigrateToFd(qemuMonitor *mon, unsigned int flags, int fd); +int qemuMonitorMigrateToFdSet(virDomainObj *vm, + unsigned int flags, + int *fd); + int qemuMonitorMigrateToHost(qemuMonitor *mon, unsigned int flags, const char *protocol, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index e3f6a0ad0f..cad982f450 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -430,6 +430,7 @@ qemuSaveImageCreateFd(virQEMUDriver *driver, virDomainObj *vm, const char *path, virFileWrapperFd *wrapperFd, + bool sparse, bool *needUnlink, unsigned int flags) { @@ -439,7 +440,7 @@ qemuSaveImageCreateFd(virQEMUDriver *driver, int directFlag = 0; unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; - if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) { + if (!sparse && flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) { wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE; directFlag = virFileDirectFdFlag(); if (directFlag < 0) { @@ -459,7 +460,7 @@ qemuSaveImageCreateFd(virQEMUDriver *driver, if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) return -1; - if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) + if (!sparse && !(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) return -1; ret = fd; @@ -478,6 +479,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, const char *path, virQEMUSaveData *data, virCommand *compressor, + qemuMigrationParams *saveParams, unsigned int flags, virDomainAsyncJob asyncJob) { @@ -486,9 +488,10 @@ qemuSaveImageCreate(virQEMUDriver *driver, int ret = -1; int fd = -1; virFileWrapperFd *wrapperFd = NULL; + bool sparse = data->header.format == QEMU_SAVE_FORMAT_SPARSE; /* Obtain the file handle. */ - fd = qemuSaveImageCreateFd(driver, vm, path, wrapperFd, &needUnlink, flags); + fd = qemuSaveImageCreateFd(driver, vm, path, wrapperFd, sparse, &needUnlink, flags); if (fd < 0) goto cleanup; @@ -497,7 +500,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, goto cleanup; /* Perform the migration */ - if (qemuMigrationSrcToFile(driver, vm, fd, compressor, asyncJob) < 0) + if (qemuMigrationSrcToFile(driver, vm, &fd, compressor, saveParams, flags, asyncJob) < 0) goto cleanup; /* Touch up file header to mark image complete. */ @@ -505,14 +508,18 @@ qemuSaveImageCreate(virQEMUDriver *driver, /* Reopen the file to touch up the header, since we aren't set * up to seek backwards on wrapperFd. The reopened fd will * trigger a single page of file system cache pollution, but - * that's acceptable. */ - if (VIR_CLOSE(fd) < 0) { - virReportSystemError(errno, _("unable to close %1$s"), path); - goto cleanup; - } + * that's acceptable. + * If using mapped-ram, the fd was passed to qemu, so no need + * to close it. */ + if (!sparse) { + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, _("unable to close %1$s"), path); + goto cleanup; + } - if (qemuDomainFileWrapperFDClose(vm, wrapperFd) < 0) - goto cleanup; + if (qemuDomainFileWrapperFDClose(vm, wrapperFd) < 0) + goto cleanup; + } if ((fd = qemuDomainOpenFile(cfg, vm->def, path, O_WRONLY, NULL)) < 0 || virQEMUSaveDataFinish(data, &fd, path) < 0) @@ -552,7 +559,7 @@ qemuSaveImageGetCompressionProgram(int format, *compressor = NULL; - if (format == QEMU_SAVE_FORMAT_RAW) + if (format == QEMU_SAVE_FORMAT_RAW || format == QEMU_SAVE_FORMAT_SPARSE) return 0; if (!(prog = virFindFileInPath(imageFormat))) { diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 0411f3140a..495d773ea5 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -136,6 +136,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, const char *path, virQEMUSaveData *data, virCommand *compressor, + qemuMigrationParams *saveParams, unsigned int flags, virDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 891e67e98b..fa814e051c 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1661,6 +1661,8 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, /* do the memory snapshot if necessary */ if (memory) { + g_autoptr(qemuMigrationParams) snap_params = NULL; + /* check if migration is possible */ if (!qemuMigrationSrcIsAllowed(vm, false, VIR_ASYNC_JOB_SNAPSHOT, 0)) goto cleanup; @@ -1691,8 +1693,11 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, memory_existing = virFileExists(snapdef->memorysnapshotfile); + if (!(snap_params = qemuMigrationParamsNew())) + goto cleanup; + if ((ret = qemuSaveImageCreate(driver, vm, snapdef->memorysnapshotfile, - data, compressor, 0, + data, compressor, snap_params, 0, VIR_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; -- 2.43.0

qemuProcessStartWithMemoryState() is the only caller of qemuProcessStart() that uses the qemuProcessIncomingDef struct. Move creation of the struct to qemuProcessStartWithMemoryState(). Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_process.c | 42 ++++++++++++++++++++--------------------- src/qemu/qemu_process.h | 2 +- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7b7b5c17e3..cf99e4461d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8347,7 +8347,7 @@ qemuProcessStart(virConnectPtr conn, virDomainObj *vm, virCPUDef *updatedCPU, virDomainAsyncJob asyncJob, - const char *migrateFrom, + qemuProcessIncomingDef *incoming, int migrateFd, const char *migratePath, virDomainMomentObj *snapshot, @@ -8355,7 +8355,6 @@ qemuProcessStart(virConnectPtr conn, unsigned int flags) { qemuDomainObjPrivate *priv = vm->privateData; - qemuProcessIncomingDef *incoming = NULL; unsigned int stopFlags; bool relabel = false; bool relabelSavedState = false; @@ -8363,11 +8362,11 @@ qemuProcessStart(virConnectPtr conn, int rv; VIR_DEBUG("conn=%p driver=%p vm=%p name=%s id=%d asyncJob=%s " - "migrateFrom=%s migrateFd=%d migratePath=%s " + "incoming=%p migrateFd=%d migratePath=%s " "snapshot=%p vmop=%d flags=0x%x", conn, driver, vm, vm->def->name, vm->def->id, virDomainAsyncJobTypeToString(asyncJob), - NULLSTR(migrateFrom), migrateFd, NULLSTR(migratePath), + incoming, migrateFd, NULLSTR(migratePath), snapshot, vmop, flags); virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | @@ -8376,20 +8375,13 @@ qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_GEN_VMID | VIR_QEMU_PROCESS_START_RESET_NVRAM, cleanup); - if (!migrateFrom && !snapshot) + if (!incoming && !snapshot) flags |= VIR_QEMU_PROCESS_START_NEW; if (qemuProcessInit(driver, vm, updatedCPU, - asyncJob, !!migrateFrom, flags) < 0) + asyncJob, !!incoming, flags) < 0) goto cleanup; - if (migrateFrom) { - incoming = qemuProcessIncomingDefNew(priv->qemuCaps, NULL, migrateFrom, - migrateFd, migratePath); - if (!incoming) - goto stop; - } - if (qemuProcessPrepareDomain(driver, vm, flags) < 0) goto stop; @@ -8442,14 +8434,13 @@ qemuProcessStart(virConnectPtr conn, qemuSecurityRestoreSavedStateLabel(driver->securityManager, vm->def, migratePath) < 0) VIR_WARN("failed to restore save state label on %s", migratePath); - qemuProcessIncomingDefFree(incoming); return ret; stop: stopFlags = 0; if (!relabel) stopFlags |= VIR_QEMU_PROCESS_STOP_NO_RELABEL; - if (migrateFrom) + if (incoming) stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; if (priv->mon) qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); @@ -8503,8 +8494,9 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, VIR_AUTOCLOSE intermediatefd = -1; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; - const char *migrateFrom = NULL; + qemuProcessIncomingDef *incoming = NULL; int rc = 0; + int ret = -1; if (data) { if (virSaveCookieParseString(data->cookie, (virObject **)&cookie, @@ -8515,10 +8507,15 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, &errbuf, &cmd) < 0) { return -1; } - - migrateFrom = "stdio"; } + /* The fd passed to qemuProcessIncomingDefNew is used to create the migration + * URI, so it must be called after starting the decompression program. + */ + incoming = qemuProcessIncomingDefNew(priv->qemuCaps, NULL, "stdio", *fd, path); + if (!incoming) + return -1; + /* No cookie means libvirt which saved the domain was too old to mess up * the CPU definitions. */ @@ -8529,7 +8526,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, priv->disableSlirp = true; if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, - asyncJob, migrateFrom, *fd, path, snapshot, + asyncJob, incoming, *fd, path, snapshot, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, start_flags) == 0) *started = true; @@ -8541,14 +8538,17 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, virDomainAuditStart(vm, reason, *started); if (!*started || rc < 0) - return -1; + goto cleanup; /* qemuProcessStart doesn't unset the qemu error reporting infrastructure * in case of migration (which is used in this case) so we need to reset it * so that the handle to virtlogd is not held open unnecessarily */ qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL); + ret = 0; - return 0; + cleanup: + qemuProcessIncomingDefFree(incoming); + return ret; } diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index fee00ce53b..a9e0a03a21 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -83,7 +83,7 @@ int qemuProcessStart(virConnectPtr conn, virDomainObj *vm, virCPUDef *updatedCPU, virDomainAsyncJob asyncJob, - const char *migrateFrom, + qemuProcessIncomingDef *incoming, int stdin_fd, const char *stdin_path, virDomainMomentObj *snapshot, -- 2.43.0

Similar to qemuMigrationSrcRun, apply migration parameters in qemuMigrationDstRun. This allows callers to create customized migration parameters, but delegates their application to the function performing the migration. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- src/qemu/qemu_migration.c | 16 ++++++++++------ src/qemu/qemu_migration.h | 5 ++++- src/qemu/qemu_process.c | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f692986056..329b5f61d4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2427,7 +2427,10 @@ qemuMigrationDstGetURI(const char *migrateFrom, int qemuMigrationDstRun(virDomainObj *vm, const char *uri, - virDomainAsyncJob asyncJob) + virDomainAsyncJob asyncJob, + qemuMigrationParams *migParams, + unsigned int flags) + { virTristateBool exitOnError = VIR_TRISTATE_BOOL_ABSENT; qemuDomainObjPrivate *priv = vm->privateData; @@ -2435,6 +2438,10 @@ qemuMigrationDstRun(virDomainObj *vm, VIR_DEBUG("Setting up incoming migration with URI %s", uri); + if (migParams && qemuMigrationParamsApply(vm, asyncJob, + migParams, flags) < 0) + return -1; + /* Ask QEMU not to exit on failure during incoming migration (if supported) * so that we can properly check and report error during Finish phase. */ @@ -3368,10 +3375,6 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, goto error; } - if (qemuMigrationParamsApply(vm, VIR_ASYNC_JOB_MIGRATION_IN, - migParams, flags) < 0) - goto error; - if (mig->nbd && flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC)) { const char *nbdTLSAlias = NULL; @@ -3403,7 +3406,8 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, } if (qemuMigrationDstRun(vm, incoming->uri, - VIR_ASYNC_JOB_MIGRATION_IN) < 0) + VIR_ASYNC_JOB_MIGRATION_IN, + migParams, flags) < 0) goto error; if (qemuProcessFinishStartup(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 9fa007b949..71a9974753 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -283,7 +283,10 @@ qemuMigrationDstGetURI(const char *migrateFrom, int qemuMigrationDstRun(virDomainObj *vm, const char *uri, - virDomainAsyncJob asyncJob); + virDomainAsyncJob asyncJob, + qemuMigrationParams *migParams, + unsigned int flags); + void qemuMigrationSrcPostcopyFailed(virDomainObj *vm); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index cf99e4461d..23b2d4a7b6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8404,7 +8404,7 @@ qemuProcessStart(virConnectPtr conn, relabel = true; if (incoming) { - if (qemuMigrationDstRun(vm, incoming->uri, asyncJob) < 0) + if (qemuMigrationDstRun(vm, incoming->uri, asyncJob, NULL, 0) < 0) goto stop; } else { /* Refresh state of devices from QEMU. During migration this happens -- 2.43.0

On 3/19/25 09:45, Daniel P. Berrangé wrote: Good point. I took another look and agree it should be fine. The interesting scenario is failure to apply the migration params. Prior to this patch the NBD server would not have been started. After the patch it would be running. Shouldn't be a problem though, since the code already handles other misc failures after starting the NBD server. Regards, Jim