[libvirt RFC v3 00/19] multifd save restore prototype

This is the multifd save prototype in its first semi-functional state, now with both save and restore minimally functional. Still as mentioned before, likely there are quite a few rough edges, let me know what you think about this possible option. changes from v2 are many, mainly: * added ability to restore the VM from disk using multifd * fixed the multifd-helper to work in both directions, assuming the need to listen for save, and connect for restore. * fixed a large number of bugs, and probably introduced some :-) KNOWN ISSUES: 1) this applies only to virsh save and virsh restore for now (no managed save etc). 2) the .pl scripts to generate the headers for the new APIs do not reliably work for me, for the Restore case. I get: src/remote/remote_daemon_dispatch_stubs.h:10080:9: error: too few arguments to function ‘virDomainRestoreParametersFlags’ if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0) To work around this I had to fixup the header manually to look like: ...(conn, params, nparams, args->flags) < 0) Thanks for your thoughts, Claudio Claudio Fontana (19): iohelper: introduce new struct to carry copy operation parameters iohelper: refactor copy operation as a separate function libvirt: introduce virDomainSaveParametersFlags public API libvirt: introduce virDomainRestoreParametersFlags public API remote: Add RPC support for the virDomainSaveParametersFlags API remote: Add RPC support for the virDomainRestoreParametersFlags API qemu: add a stub for virDomainSaveParametersFlags API qemu: add a stub for virDomainRestoreParametersFlags API qemu: saveimage: introduce virQEMUSaveFd iohelper: move runIO function to a separate module runio: add arguments to extend use beyond just stdin and stdout multifd-helper: new helper for parallel save/restore qemu: wire up saveimage code with the multifd helper qemu: implement qemuMigrationSrcToFilesMultiFd qemu: add parameter to qemuMigrationDstRun to skip waiting qemu: implement qemuSaveImageLoadMultiFd tools: add parallel parameter to virsh save command tools: add parallel parameter to virsh restore command qemu: add migration parameter multifd-compression docs/manpages/virsh.rst | 34 ++- include/libvirt/libvirt-domain.h | 13 + src/driver-hypervisor.h | 14 + src/libvirt-domain.c | 99 +++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 6 + src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_driver.c | 233 +++++++++++---- src/qemu/qemu_migration.c | 155 ++++++---- src/qemu/qemu_migration.h | 16 +- src/qemu/qemu_migration_params.c | 71 +++-- src/qemu/qemu_migration_params.h | 15 + src/qemu/qemu_process.c | 3 +- src/qemu/qemu_process.h | 5 +- src/qemu/qemu_saveimage.c | 480 ++++++++++++++++++++++++------- src/qemu/qemu_saveimage.h | 49 +++- src/qemu/qemu_snapshot.c | 6 +- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 29 +- src/remote_protocol-structs | 17 ++ src/util/iohelper.c | 150 +--------- src/util/meson.build | 15 + src/util/multifd-helper.c | 250 ++++++++++++++++ src/util/runio.c | 214 ++++++++++++++ src/util/runio.h | 38 +++ src/util/virthread.c | 5 + src/util/virthread.h | 1 + tools/virsh-domain.c | 96 +++++-- 29 files changed, 1599 insertions(+), 426 deletions(-) create mode 100644 src/util/multifd-helper.c create mode 100644 src/util/runio.c create mode 100644 src/util/runio.h -- 2.34.1

this is in preparation for a minor refactoring of the copy function itself out of runIO(). Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/iohelper.c | 95 +++++++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 42 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 2c91bf4f93..c13746a547 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -45,6 +45,16 @@ # define O_DIRECT 0 #endif +struct runIOParams { + bool isBlockDev; + bool isDirect; + bool isWrite; + int fdin; + const char *fdinname; + int fdout; + const char *fdoutname; +}; + static int runIO(const char *path, int fd, int oflags) { @@ -53,13 +63,9 @@ runIO(const char *path, int fd, int oflags) size_t buflen = 1024*1024; intptr_t alignMask = 64*1024 - 1; int ret = -1; - int fdin, fdout; - const char *fdinname, *fdoutname; - unsigned long long total = 0; - bool direct = O_DIRECT && ((oflags & O_DIRECT) != 0); - off_t end = 0; + off_t total = 0; struct stat sb; - bool isBlockDev = false; + struct runIOParams p; #if WITH_POSIX_MEMALIGN if (posix_memalign(&base, alignMask + 1, buflen)) @@ -77,34 +83,23 @@ runIO(const char *path, int fd, int oflags) fd, path); goto cleanup; } - isBlockDev = S_ISBLK(sb.st_mode); + p.isBlockDev = S_ISBLK(sb.st_mode); + p.isDirect = O_DIRECT && (oflags & O_DIRECT); switch (oflags & O_ACCMODE) { case O_RDONLY: - fdin = fd; - fdinname = path; - fdout = STDOUT_FILENO; - fdoutname = "stdout"; - /* To make the implementation simpler, we give up on any - * attempt to use O_DIRECT in a non-trivial manner. */ - if (!isBlockDev && direct && ((end = lseek(fd, 0, SEEK_CUR)) != 0)) { - virReportSystemError(end < 0 ? errno : EINVAL, "%s", - _("O_DIRECT read needs entire seekable file")); - goto cleanup; - } + p.isWrite = false; + p.fdin = fd; + p.fdinname = path; + p.fdout = STDOUT_FILENO; + p.fdoutname = "stdout"; break; case O_WRONLY: - fdin = STDIN_FILENO; - fdinname = "stdin"; - fdout = fd; - fdoutname = path; - /* To make the implementation simpler, we give up on any - * attempt to use O_DIRECT in a non-trivial manner. */ - if (!isBlockDev && direct && (end = lseek(fd, 0, SEEK_END)) != 0) { - virReportSystemError(end < 0 ? errno : EINVAL, "%s", - _("O_DIRECT write needs empty seekable file")); - goto cleanup; - } + p.isWrite = true; + p.fdin = STDIN_FILENO; + p.fdinname = "stdin"; + p.fdout = fd; + p.fdoutname = path; break; case O_RDWR: @@ -114,6 +109,22 @@ runIO(const char *path, int fd, int oflags) (oflags & O_ACCMODE)); goto cleanup; } + /* To make the implementation simpler, we give up on any + * attempt to use O_DIRECT in a non-trivial manner. */ + if (!p.isBlockDev && p.isDirect) { + off_t off; + if (p.isWrite) { + if ((off = lseek(fd, 0, SEEK_END)) != 0) { + virReportSystemError(off < 0 ? errno : EINVAL, "%s", + _("O_DIRECT write needs empty seekable file")); + goto cleanup; + } + } else if ((off = lseek(fd, 0, SEEK_CUR)) != 0) { + virReportSystemError(off < 0 ? errno : EINVAL, "%s", + _("O_DIRECT read needs entire seekable file")); + goto cleanup; + } + } while (1) { ssize_t got; @@ -124,16 +135,16 @@ runIO(const char *path, int fd, int oflags) * writes will be aligned. * In other cases using saferead reduces number of syscalls. */ - if (fdin == fd && direct) { - if ((got = read(fdin, buf, buflen)) < 0 && + if (!p.isWrite && p.isDirect) { + if ((got = read(p.fdin, buf, buflen)) < 0 && errno == EINTR) continue; } else { - got = saferead(fdin, buf, buflen); + got = saferead(p.fdin, buf, buflen); } if (got < 0) { - virReportSystemError(errno, _("Unable to read %s"), fdinname); + virReportSystemError(errno, _("Unable to read %s"), p.fdinname); goto cleanup; } if (got == 0) @@ -142,35 +153,35 @@ runIO(const char *path, int fd, int oflags) total += got; /* handle last write size align in direct case */ - if (got < buflen && direct && fdout == fd) { + if (got < buflen && p.isDirect && p.isWrite) { ssize_t aligned_got = (got + alignMask) & ~alignMask; memset(buf + got, 0, aligned_got - got); - if (safewrite(fdout, buf, aligned_got) < 0) { - virReportSystemError(errno, _("Unable to write %s"), fdoutname); + if (safewrite(p.fdout, buf, aligned_got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); goto cleanup; } - if (!isBlockDev && ftruncate(fd, total) < 0) { - virReportSystemError(errno, _("Unable to truncate %s"), fdoutname); + if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) { + virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname); goto cleanup; } break; } - if (safewrite(fdout, buf, got) < 0) { - virReportSystemError(errno, _("Unable to write %s"), fdoutname); + if (safewrite(p.fdout, buf, got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); goto cleanup; } } /* Ensure all data is written */ - if (virFileDataSync(fdout) < 0) { + if (virFileDataSync(p.fdout) < 0) { if (errno != EINVAL && errno != EROFS) { /* fdatasync() may fail on some special FDs, e.g. pipes */ - virReportSystemError(errno, _("unable to fsync %s"), fdoutname); + virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname); goto cleanup; } } -- 2.34.1

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/iohelper.c | 131 +++++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 56 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index c13746a547..1584321839 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -55,17 +55,23 @@ struct runIOParams { const char *fdoutname; }; -static int -runIO(const char *path, int fd, int oflags) +/** + * runIOCopy: execute the IO copy based on the passed parameters + * @p: the IO parameters + * + * Execute the copy based on the passed parameters. + * + * Returns: size transfered, or < 0 on error. + */ + +static off_t +runIOCopy(const struct runIOParams p) { g_autofree void *base = NULL; /* Location to be freed */ char *buf = NULL; /* Aligned location within base */ size_t buflen = 1024*1024; intptr_t alignMask = 64*1024 - 1; - int ret = -1; off_t total = 0; - struct stat sb; - struct runIOParams p; #if WITH_POSIX_MEMALIGN if (posix_memalign(&base, alignMask + 1, buflen)) @@ -77,6 +83,67 @@ runIO(const char *path, int fd, int oflags) buf = (char *) (((intptr_t) base + alignMask) & ~alignMask); #endif + while (1) { + ssize_t got; + + /* If we read with O_DIRECT from file we can't use saferead as + * it can lead to unaligned read after reading last bytes. + * If we write with O_DIRECT use should use saferead so that + * writes will be aligned. + * In other cases using saferead reduces number of syscalls. + */ + if (!p.isWrite && p.isDirect) { + if ((got = read(p.fdin, buf, buflen)) < 0 && + errno == EINTR) + continue; + } else { + got = saferead(p.fdin, buf, buflen); + } + + if (got < 0) { + virReportSystemError(errno, _("Unable to read %s"), p.fdinname); + return -2; + } + if (got == 0) + break; + + total += got; + + /* handle last write size align in direct case */ + if (got < buflen && p.isDirect && p.isWrite) { + ssize_t aligned_got = (got + alignMask) & ~alignMask; + + memset(buf + got, 0, aligned_got - got); + + if (safewrite(p.fdout, buf, aligned_got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); + return -3; + } + + if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) { + virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname); + return -4; + } + + break; + } + + if (safewrite(p.fdout, buf, got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); + return -3; + } + } + return total; +} + +static int +runIO(const char *path, int fd, int oflags) +{ + int ret = -1; + off_t total = 0; + struct stat sb; + struct runIOParams p; + if (fstat(fd, &sb) < 0) { virReportSystemError(errno, _("Unable to access file descriptor %d path %s"), @@ -125,57 +192,9 @@ runIO(const char *path, int fd, int oflags) goto cleanup; } } - - while (1) { - ssize_t got; - - /* If we read with O_DIRECT from file we can't use saferead as - * it can lead to unaligned read after reading last bytes. - * If we write with O_DIRECT use should use saferead so that - * writes will be aligned. - * In other cases using saferead reduces number of syscalls. - */ - if (!p.isWrite && p.isDirect) { - if ((got = read(p.fdin, buf, buflen)) < 0 && - errno == EINTR) - continue; - } else { - got = saferead(p.fdin, buf, buflen); - } - - if (got < 0) { - virReportSystemError(errno, _("Unable to read %s"), p.fdinname); - goto cleanup; - } - if (got == 0) - break; - - total += got; - - /* handle last write size align in direct case */ - if (got < buflen && p.isDirect && p.isWrite) { - ssize_t aligned_got = (got + alignMask) & ~alignMask; - - memset(buf + got, 0, aligned_got - got); - - if (safewrite(p.fdout, buf, aligned_got) < 0) { - virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); - goto cleanup; - } - - if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) { - virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname); - goto cleanup; - } - - break; - } - - if (safewrite(p.fdout, buf, got) < 0) { - virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); - goto cleanup; - } - } + total = runIOCopy(p); + if (total < 0) + goto cleanup; /* Ensure all data is written */ if (virFileDataSync(p.fdout) < 0) { -- 2.34.1

add new API in order to be able to extend parameters to the domain save operation. We will use it to fit the existing arguments of VirDomainSaveFlags, and then add parallel saves functionality. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- include/libvirt/libvirt-domain.h | 9 ++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 51 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 72 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9aa214f3df..4beea34f93 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1481,6 +1481,7 @@ typedef enum { VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from template */ + VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to multiple files */ } virDomainSaveRestoreFlags; int virDomainSave (virDomainPtr domain, @@ -1489,6 +1490,10 @@ int virDomainSaveFlags (virDomainPtr domain, const char *to, const char *dxml, unsigned int flags); +int virDomainSaveParametersFlags (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); int virDomainRestore (virConnectPtr conn, const char *from); int virDomainRestoreFlags (virConnectPtr conn, @@ -1496,6 +1501,10 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *dxml, unsigned int flags); +# define VIR_SAVE_PARAM_FILE "file" +# define VIR_SAVE_PARAM_DXML "dxml" +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections" + /* See below for virDomainSaveImageXMLFlags */ char * virDomainSaveImageGetXMLDesc (virConnectPtr conn, const char *file, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 4423eb0885..a4e1d21e76 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -240,6 +240,12 @@ typedef int const char *dxml, unsigned int flags); +typedef int +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef int (*virDrvDomainRestore)(virConnectPtr conn, const char *from); @@ -1489,6 +1495,7 @@ struct _virHypervisorDriver { virDrvDomainGetControlInfo domainGetControlInfo; virDrvDomainSave domainSave; virDrvDomainSaveFlags domainSaveFlags; + virDrvDomainSaveParametersFlags domainSaveParametersFlags; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cbd7902d2d..9e4fcfd022 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -959,6 +959,57 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, return -1; } +/** + * virDomainSaveParametersFlags: + * @domain: a domain object + * @params: save parameters + * @nparams: number of save parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainSaveFlags by adding parameters to Save. + * + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will + * attempt to trigger a parallel transfer to multiple files, + * where the number of extra files is determined by the parameter + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainSaveParametersFlags(virDomainPtr domain, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error); + + if (conn->driver->domainSaveParametersFlags) { + if (conn->driver->domainSaveParametersFlags(domain, params, nparams, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} + /** * virDomainRestore: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f93692c427..eb3a7afb75 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -916,4 +916,9 @@ LIBVIRT_8.0.0 { virDomainSetLaunchSecurityState; } LIBVIRT_7.8.0; +LIBVIRT_8.3.0 { + global: + virDomainSaveParametersFlags; +} LIBVIRT_8.0.0; + # .... define new API here using predicted next version number .... -- 2.34.1

On 4/26/22 10:47, Claudio Fontana wrote:
add new API in order to be able to extend parameters to the domain save operation. We will use it to fit the existing arguments of VirDomainSaveFlags, and then add parallel saves functionality.
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- include/libvirt/libvirt-domain.h | 9 ++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 51 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 72 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9aa214f3df..4beea34f93 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1481,6 +1481,7 @@ typedef enum { VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from template */ + VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to multiple files */ } virDomainSaveRestoreFlags;
int virDomainSave (virDomainPtr domain, @@ -1489,6 +1490,10 @@ int virDomainSaveFlags (virDomainPtr domain, const char *to, const char *dxml, unsigned int flags); +int virDomainSaveParametersFlags (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); int virDomainRestore (virConnectPtr conn, const char *from); int virDomainRestoreFlags (virConnectPtr conn, @@ -1496,6 +1501,10 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *dxml, unsigned int flags);
+# define VIR_SAVE_PARAM_FILE "file" +# define VIR_SAVE_PARAM_DXML "dxml" +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
The common pattern is to use '_' in multi-word parameter names, i.e. "parallel_connections". Also, this fails to build the API docs [499/508] Generating docs/generate-api with a custom command FAILED: docs/libvirt-api.xml docs/libvirt-lxc-api.xml docs/libvirt-qemu-api.xml docs/libvirt-admin-api.xml /home/jfehlig/virt/gitlab/libvirt/scripts/meson-python.sh /usr/bin/python3 /home/jfehlig/virt/gitlab/libvirt/scripts/apibuild.py /home/jfehlig/virt/gitlab/libvirt/docs /home/jfehlig/virt/gitlab/libvirt/build/docs Misformatted macro comment for VIR_SAVE_PARAM_FILE Expecting '* VIR_SAVE_PARAM_FILE:' got '* virDomainSaveRestoreFlags:' Misformatted macro comment for VIR_SAVE_PARAM_DXML Expecting '* VIR_SAVE_PARAM_DXML:' got '* virDomainSaveRestoreFlags:' Misformatted macro comment for VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Expecting '* VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:' got '* virDomainSaveRestoreFlags:' Missing 'Since' tag for: VIR_SAVE_PARAM_DXML Missing 'Since' tag for: VIR_SAVE_PARAM_FILE Missing 'Since' tag for: VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Missing 'Since' tag for: VIR_DOMAIN_SAVE_PARALLEL Missing 'Since' tag for: virDomainSaveParametersFlags
+ /* See below for virDomainSaveImageXMLFlags */ char * virDomainSaveImageGetXMLDesc (virConnectPtr conn, const char *file, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 4423eb0885..a4e1d21e76 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -240,6 +240,12 @@ typedef int const char *dxml, unsigned int flags);
+typedef int +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef int (*virDrvDomainRestore)(virConnectPtr conn, const char *from); @@ -1489,6 +1495,7 @@ struct _virHypervisorDriver { virDrvDomainGetControlInfo domainGetControlInfo; virDrvDomainSave domainSave; virDrvDomainSaveFlags domainSaveFlags; + virDrvDomainSaveParametersFlags domainSaveParametersFlags; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cbd7902d2d..9e4fcfd022 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -959,6 +959,57 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, return -1; }
+/** + * virDomainSaveParametersFlags: + * @domain: a domain object + * @params: save parameters + * @nparams: number of save parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainSaveFlags by adding parameters to Save. + * + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will + * attempt to trigger a parallel transfer to multiple files, + * where the number of extra files is determined by the parameter + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainSaveParametersFlags(virDomainPtr domain, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error);
Check for the new flag? Jim
+ + if (conn->driver->domainSaveParametersFlags) { + if (conn->driver->domainSaveParametersFlags(domain, params, nparams, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} +
/** * virDomainRestore: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f93692c427..eb3a7afb75 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -916,4 +916,9 @@ LIBVIRT_8.0.0 { virDomainSetLaunchSecurityState; } LIBVIRT_7.8.0;
+LIBVIRT_8.3.0 { + global: + virDomainSaveParametersFlags; +} LIBVIRT_8.0.0; + # .... define new API here using predicted next version number ....

Thanks for looking at this, On 4/27/22 12:42 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
add new API in order to be able to extend parameters to the domain save operation. We will use it to fit the existing arguments of VirDomainSaveFlags, and then add parallel saves functionality.
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- include/libvirt/libvirt-domain.h | 9 ++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 51 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 72 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9aa214f3df..4beea34f93 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1481,6 +1481,7 @@ typedef enum { VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from template */ + VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to multiple files */ } virDomainSaveRestoreFlags;
int virDomainSave (virDomainPtr domain, @@ -1489,6 +1490,10 @@ int virDomainSaveFlags (virDomainPtr domain, const char *to, const char *dxml, unsigned int flags); +int virDomainSaveParametersFlags (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); int virDomainRestore (virConnectPtr conn, const char *from); int virDomainRestoreFlags (virConnectPtr conn, @@ -1496,6 +1501,10 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *dxml, unsigned int flags);
+# define VIR_SAVE_PARAM_FILE "file" +# define VIR_SAVE_PARAM_DXML "dxml" +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
The common pattern is to use '_' in multi-word parameter names, i.e.
I used the same parameter as per: libvirt/libvirt-domain.h:# define VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS "parallel.connections" since it is basically the name parameter and meaning, just now used for save and restore, maybe it make sense to keep it as-is?
"parallel_connections". Also, this fails to build the API docs
[499/508] Generating docs/generate-api with a custom command FAILED: docs/libvirt-api.xml docs/libvirt-lxc-api.xml docs/libvirt-qemu-api.xml docs/libvirt-admin-api.xml /home/jfehlig/virt/gitlab/libvirt/scripts/meson-python.sh /usr/bin/python3 /home/jfehlig/virt/gitlab/libvirt/scripts/apibuild.py /home/jfehlig/virt/gitlab/libvirt/docs /home/jfehlig/virt/gitlab/libvirt/build/docs Misformatted macro comment for VIR_SAVE_PARAM_FILE Expecting '* VIR_SAVE_PARAM_FILE:' got '* virDomainSaveRestoreFlags:' Misformatted macro comment for VIR_SAVE_PARAM_DXML Expecting '* VIR_SAVE_PARAM_DXML:' got '* virDomainSaveRestoreFlags:' Misformatted macro comment for VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Expecting '* VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:' got '* virDomainSaveRestoreFlags:' Missing 'Since' tag for: VIR_SAVE_PARAM_DXML Missing 'Since' tag for: VIR_SAVE_PARAM_FILE Missing 'Since' tag for: VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Missing 'Since' tag for: VIR_DOMAIN_SAVE_PARALLEL Missing 'Since' tag for: virDomainSaveParametersFlags
Thanks, will fix.
+ /* See below for virDomainSaveImageXMLFlags */ char * virDomainSaveImageGetXMLDesc (virConnectPtr conn, const char *file, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 4423eb0885..a4e1d21e76 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -240,6 +240,12 @@ typedef int const char *dxml, unsigned int flags);
+typedef int +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef int (*virDrvDomainRestore)(virConnectPtr conn, const char *from); @@ -1489,6 +1495,7 @@ struct _virHypervisorDriver { virDrvDomainGetControlInfo domainGetControlInfo; virDrvDomainSave domainSave; virDrvDomainSaveFlags domainSaveFlags; + virDrvDomainSaveParametersFlags domainSaveParametersFlags; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cbd7902d2d..9e4fcfd022 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -959,6 +959,57 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, return -1; }
+/** + * virDomainSaveParametersFlags: + * @domain: a domain object + * @params: save parameters + * @nparams: number of save parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainSaveFlags by adding parameters to Save. + * + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will + * attempt to trigger a parallel transfer to multiple files, + * where the number of extra files is determined by the parameter + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainSaveParametersFlags(virDomainPtr domain, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error);
Check for the new flag?
Right, thanks! Claudio
Jim
+ + if (conn->driver->domainSaveParametersFlags) { + if (conn->driver->domainSaveParametersFlags(domain, params, nparams, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} +
/** * virDomainRestore: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f93692c427..eb3a7afb75 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -916,4 +916,9 @@ LIBVIRT_8.0.0 { virDomainSetLaunchSecurityState; } LIBVIRT_7.8.0;
+LIBVIRT_8.3.0 { + global: + virDomainSaveParametersFlags; +} LIBVIRT_8.0.0; + # .... define new API here using predicted next version number ....

On 4/27/22 12:42 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
add new API in order to be able to extend parameters to the domain save operation. We will use it to fit the existing arguments of VirDomainSaveFlags, and then add parallel saves functionality.
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- include/libvirt/libvirt-domain.h | 9 ++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 51 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 72 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 9aa214f3df..4beea34f93 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1481,6 +1481,7 @@ typedef enum { VIR_DOMAIN_SAVE_RUNNING = 1 << 1, /* Favor running over paused */ VIR_DOMAIN_SAVE_PAUSED = 1 << 2, /* Favor paused over running */ VIR_DOMAIN_SAVE_RESET_NVRAM = 1 << 3, /* Re-initialize NVRAM from template */ + VIR_DOMAIN_SAVE_PARALLEL = 1 << 4, /* Parallel Save/Restore to multiple files */ } virDomainSaveRestoreFlags;
int virDomainSave (virDomainPtr domain, @@ -1489,6 +1490,10 @@ int virDomainSaveFlags (virDomainPtr domain, const char *to, const char *dxml, unsigned int flags); +int virDomainSaveParametersFlags (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); int virDomainRestore (virConnectPtr conn, const char *from); int virDomainRestoreFlags (virConnectPtr conn, @@ -1496,6 +1501,10 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *dxml, unsigned int flags);
+# define VIR_SAVE_PARAM_FILE "file" +# define VIR_SAVE_PARAM_DXML "dxml" +# define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections"
The common pattern is to use '_' in multi-word parameter names, i.e. "parallel_connections". Also, this fails to build the API docs
[499/508] Generating docs/generate-api with a custom command FAILED: docs/libvirt-api.xml docs/libvirt-lxc-api.xml docs/libvirt-qemu-api.xml docs/libvirt-admin-api.xml /home/jfehlig/virt/gitlab/libvirt/scripts/meson-python.sh /usr/bin/python3 /home/jfehlig/virt/gitlab/libvirt/scripts/apibuild.py /home/jfehlig/virt/gitlab/libvirt/docs /home/jfehlig/virt/gitlab/libvirt/build/docs Misformatted macro comment for VIR_SAVE_PARAM_FILE Expecting '* VIR_SAVE_PARAM_FILE:' got '* virDomainSaveRestoreFlags:' Misformatted macro comment for VIR_SAVE_PARAM_DXML Expecting '* VIR_SAVE_PARAM_DXML:' got '* virDomainSaveRestoreFlags:' Misformatted macro comment for VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Expecting '* VIR_SAVE_PARAM_PARALLEL_CONNECTIONS:' got '* virDomainSaveRestoreFlags:' Missing 'Since' tag for: VIR_SAVE_PARAM_DXML Missing 'Since' tag for: VIR_SAVE_PARAM_FILE Missing 'Since' tag for: VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Missing 'Since' tag for: VIR_DOMAIN_SAVE_PARALLEL Missing 'Since' tag for: virDomainSaveParametersFlags
+ /* See below for virDomainSaveImageXMLFlags */ char * virDomainSaveImageGetXMLDesc (virConnectPtr conn, const char *file, diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 4423eb0885..a4e1d21e76 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -240,6 +240,12 @@ typedef int const char *dxml, unsigned int flags);
+typedef int +(*virDrvDomainSaveParametersFlags)(virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef int (*virDrvDomainRestore)(virConnectPtr conn, const char *from); @@ -1489,6 +1495,7 @@ struct _virHypervisorDriver { virDrvDomainGetControlInfo domainGetControlInfo; virDrvDomainSave domainSave; virDrvDomainSaveFlags domainSaveFlags; + virDrvDomainSaveParametersFlags domainSaveParametersFlags; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index cbd7902d2d..9e4fcfd022 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -959,6 +959,57 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, return -1; }
+/** + * virDomainSaveParametersFlags: + * @domain: a domain object + * @params: save parameters + * @nparams: number of save parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainSaveFlags by adding parameters to Save. + * + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will + * attempt to trigger a parallel transfer to multiple files, + * where the number of extra files is determined by the parameter + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainSaveParametersFlags(virDomainPtr domain, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "params=%p, nparams=%d, flags=0x%x", + params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error);
Check for the new flag?
Jim
hmm for the others it's done in the actual driver (ie. in src/qemu/) - C
+ + if (conn->driver->domainSaveParametersFlags) { + if (conn->driver->domainSaveParametersFlags(domain, params, nparams, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} +
/** * virDomainRestore: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index f93692c427..eb3a7afb75 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -916,4 +916,9 @@ LIBVIRT_8.0.0 { virDomainSetLaunchSecurityState; } LIBVIRT_7.8.0;
+LIBVIRT_8.3.0 { + global: + virDomainSaveParametersFlags; +} LIBVIRT_8.0.0; + # .... define new API here using predicted next version number ....

add new API in order to be able to extend parameters to the domain restore operation. We will use it to fit the existing arguments of VirDomainRestoreFlags, and then add parallel restore functionality. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- include/libvirt/libvirt-domain.h | 4 +++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 48 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 60 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4beea34f93..3deaf78cd7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1500,6 +1500,10 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *from, const char *dxml, unsigned int flags); +int virDomainRestoreParametersFlags (virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + unsigned int flags); # define VIR_SAVE_PARAM_FILE "file" # define VIR_SAVE_PARAM_DXML "dxml" diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index a4e1d21e76..e62e4c8f74 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -256,6 +256,12 @@ typedef int const char *dxml, unsigned int flags); +typedef int +(*virDrvDomainRestoreParametersFlags)(virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef char * (*virDrvDomainSaveImageGetXMLDesc)(virConnectPtr conn, const char *file, @@ -1498,6 +1504,7 @@ struct _virHypervisorDriver { virDrvDomainSaveParametersFlags domainSaveParametersFlags; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; + virDrvDomainRestoreParametersFlags domainRestoreParametersFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; virDrvDomainSaveImageDefineXML domainSaveImageDefineXML; virDrvDomainCoreDump domainCoreDump; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9e4fcfd022..f967efa473 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1140,6 +1140,54 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, } +/** + * virDomainRestoreParametersFlags: + * @conn: pointer to the hypervisor connection + * @params: restore parameters + * @nparams: number of restore parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainRestoreFlags by adding parameters to Restore. + * + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will + * attempt to restore from multiple files in parallel, + * where the number of extra files is determined by the parameter + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainRestoreParametersFlags(virConnectPtr conn, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, params=%p, nparams=%d, flags=0x%x", + conn, params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error); + + if (conn->driver->domainRestoreParametersFlags) { + if (conn->driver->domainRestoreParametersFlags(conn, params, nparams, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} + + /** * virDomainSaveImageGetXMLDesc: * @conn: pointer to the hypervisor connection diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index eb3a7afb75..74c1464b38 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -919,6 +919,7 @@ LIBVIRT_8.0.0 { LIBVIRT_8.3.0 { global: virDomainSaveParametersFlags; + virDomainRestoreParametersFlags; } LIBVIRT_8.0.0; # .... define new API here using predicted next version number .... -- 2.34.1

On 4/26/22 10:47, Claudio Fontana wrote:
add new API in order to be able to extend parameters to the domain restore operation. We will use it to fit the existing arguments of VirDomainRestoreFlags, and then add parallel restore functionality.
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- include/libvirt/libvirt-domain.h | 4 +++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 48 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + 4 files changed, 60 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4beea34f93..3deaf78cd7 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1500,6 +1500,10 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *from, const char *dxml, unsigned int flags); +int virDomainRestoreParametersFlags (virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + unsigned int flags);
# define VIR_SAVE_PARAM_FILE "file" # define VIR_SAVE_PARAM_DXML "dxml" diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index a4e1d21e76..e62e4c8f74 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -256,6 +256,12 @@ typedef int const char *dxml, unsigned int flags);
+typedef int +(*virDrvDomainRestoreParametersFlags)(virConnectPtr conn, + virTypedParameterPtr params, + int nparams, + unsigned int flags); + typedef char * (*virDrvDomainSaveImageGetXMLDesc)(virConnectPtr conn, const char *file, @@ -1498,6 +1504,7 @@ struct _virHypervisorDriver { virDrvDomainSaveParametersFlags domainSaveParametersFlags; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; + virDrvDomainRestoreParametersFlags domainRestoreParametersFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; virDrvDomainSaveImageDefineXML domainSaveImageDefineXML; virDrvDomainCoreDump domainCoreDump; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 9e4fcfd022..f967efa473 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -1140,6 +1140,54 @@ virDomainRestoreFlags(virConnectPtr conn, const char *from, const char *dxml, }
+/** + * virDomainRestoreParametersFlags: + * @conn: pointer to the hypervisor connection + * @params: restore parameters + * @nparams: number of restore parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainRestoreFlags by adding parameters to Restore. + * + * If @flags includes VIR_DOMAIN_SAVE_PARALLEL, then libvirt will + * attempt to restore from multiple files in parallel, + * where the number of extra files is determined by the parameter + * VIR_SAVE_PARAM_PARALLEL_CONNECTIONS. + * + * Returns 0 in case of success and -1 in case of failure. + */
Same API doc build failure here due to missing 'Since:' tag. Jim
+int +virDomainRestoreParametersFlags(virConnectPtr conn, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + VIR_DEBUG("conn=%p, params=%p, nparams=%d, flags=0x%x", + conn, params, nparams, flags); + VIR_TYPED_PARAMS_DEBUG(params, nparams); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_SAVE_RUNNING, + VIR_DOMAIN_SAVE_PAUSED, + error); + + if (conn->driver->domainRestoreParametersFlags) { + if (conn->driver->domainRestoreParametersFlags(conn, params, nparams, flags) < 0) + goto error; + return 0; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return -1; +} + + /** * virDomainSaveImageGetXMLDesc: * @conn: pointer to the hypervisor connection diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index eb3a7afb75..74c1464b38 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -919,6 +919,7 @@ LIBVIRT_8.0.0 { LIBVIRT_8.3.0 { global: virDomainSaveParametersFlags; + virDomainRestoreParametersFlags; } LIBVIRT_8.0.0;
# .... define new API here using predicted next version number ....

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 17 ++++++++++++++++- src/remote_protocol-structs | 9 +++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7e7a21fcab..1fc5d41971 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8446,6 +8446,7 @@ static virHypervisorDriver hypervisor_driver = { .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */ .domainSave = remoteDomainSave, /* 0.3.0 */ .domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */ + .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4f13cef662..c2ae5c5748 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -230,6 +230,9 @@ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64; /* Upper limit on migrate parameters */ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64; +/* Upper limit on save/restore parameters */ +const REMOTE_DOMAIN_SAVE_PARAMS_MAX = 64; + /* Upper limit on number of job stats */ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; @@ -3227,6 +3230,12 @@ struct remote_domain_migrate_confirm3_params_args { int cancelled; }; +struct remote_domain_save_parameters_flags_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6920,5 +6929,11 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439 + REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, + + /** + * @generate: both + * @acl: domain:hibernate + */ + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index d88176781d..89eadeb644 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -563,6 +563,14 @@ struct remote_domain_save_flags_args { remote_string dxml; u_int flags; }; +struct remote_domain_save_parameters_flags_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_restore_args { remote_nonnull_string from; }; @@ -3689,4 +3697,5 @@ enum remote_procedure { REMOTE_PROC_NETWORK_CREATE_XML_FLAGS = 437, REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, }; -- 2.34.1

On 4/26/22 10:47, Claudio Fontana wrote: > Signed-off-by: Claudio Fontana <cfontana@suse.de> > --- > src/remote/remote_driver.c | 1 + > src/remote/remote_protocol.x | 17 ++++++++++++++++- > src/remote_protocol-structs | 9 +++++++++ > 3 files changed, 26 insertions(+), 1 deletion(-) > > diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c > index 7e7a21fcab..1fc5d41971 100644 > --- a/src/remote/remote_driver.c > +++ b/src/remote/remote_driver.c > @@ -8446,6 +8446,7 @@ static virHypervisorDriver hypervisor_driver = { > .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */ > .domainSave = remoteDomainSave, /* 0.3.0 */ > .domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */ > + .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ > .domainRestore = remoteDomainRestore, /* 0.3.0 */ > .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ > .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ > diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x > index 4f13cef662..c2ae5c5748 100644 > --- a/src/remote/remote_protocol.x > +++ b/src/remote/remote_protocol.x > @@ -230,6 +230,9 @@ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64; > /* Upper limit on migrate parameters */ > const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64; > > +/* Upper limit on save/restore parameters */ > +const REMOTE_DOMAIN_SAVE_PARAMS_MAX = 64; > + > /* Upper limit on number of job stats */ > const REMOTE_DOMAIN_JOB_STATS_MAX = 64; > > @@ -3227,6 +3230,12 @@ struct remote_domain_migrate_confirm3_params_args { > int cancelled; > }; > > +struct remote_domain_save_parameters_flags_args { > + remote_nonnull_domain dom; > + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; > + unsigned int flags; > +}; > + > /* The device removed event is the last event where we have to support > * dual forms for back-compat to older clients; all future events can > * use just the modern form with callbackID. */ > @@ -6920,5 +6929,11 @@ enum remote_procedure { > * @generate: both > * @acl: domain:write > */ > - REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439 > + REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, > + > + /** > + * @generate: both > + * @acl: domain:hibernate > + */ > + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 > }; > diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs > index d88176781d..89eadeb644 100644 > --- a/src/remote_protocol-structs > +++ b/src/remote_protocol-structs > @@ -563,6 +563,14 @@ struct remote_domain_save_flags_args { > remote_string dxml; > u_int flags; > }; > +struct remote_domain_save_parameters_flags_args { > + remote_nonnull_domain dom; > + struct { > + u_int params_len; > + remote_typed_param * params_val; > + } params; > + u_int flags; > +}; 'ninja test' fails here, wanting this addition moved later in the file 41/314 libvirt / check-remote_protocol FAIL 0.19s exit status 1 >>> MALLOC_PERTURB_=253 LANG=C LC_ALL='' /usr/bin/python3 /home/jfehlig/virt/gitlab/libvirt/scripts/check-remote-protocol.py remote_protocol virt_remote_driver /home/jfehlig/virt/gitlab/libvirt/build/src/remote/libvirt_remote_driver.a /usr/bin/pdwtags /home/jfehlig/virt/gitlab/libvirt/build/../src/remote_protocol-structs --- /home/jfehlig/virt/gitlab/libvirt/build/../src/remote_protocol-structs 2022-04-26 15:57:29.515818322 -0600 +++ - 2022-04-26 15:58:12.933537465 -0600 @@ -563,14 +563,6 @@ remote_string dxml; u_int flags; }; -struct remote_domain_save_parameters_flags_args { - remote_nonnull_domain dom; - struct { - u_int params_len; - remote_typed_param * params_val; - } params; - u_int flags; -}; struct remote_domain_restore_args { remote_nonnull_string from; }; @@ -2609,6 +2601,14 @@ u_int flags; int cancelled; }; +struct remote_domain_save_parameters_flags_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_event_device_removed_msg { remote_nonnull_domain dom; remote_nonnull_string devAlias; 41/314 libvirt / check-remote_protocol FAIL 0.19s exit status 1 Jim > struct remote_domain_restore_args { > remote_nonnull_string from; > }; > @@ -3689,4 +3697,5 @@ enum remote_procedure { > REMOTE_PROC_NETWORK_CREATE_XML_FLAGS = 437, > REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, > REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, > + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, > };

On 4/27/22 12:47 AM, Jim Fehlig wrote: > On 4/26/22 10:47, Claudio Fontana wrote: >> Signed-off-by: Claudio Fontana <cfontana@suse.de> >> --- >> src/remote/remote_driver.c | 1 + >> src/remote/remote_protocol.x | 17 ++++++++++++++++- >> src/remote_protocol-structs | 9 +++++++++ >> 3 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c >> index 7e7a21fcab..1fc5d41971 100644 >> --- a/src/remote/remote_driver.c >> +++ b/src/remote/remote_driver.c >> @@ -8446,6 +8446,7 @@ static virHypervisorDriver hypervisor_driver = { >> .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */ >> .domainSave = remoteDomainSave, /* 0.3.0 */ >> .domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */ >> + .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ >> .domainRestore = remoteDomainRestore, /* 0.3.0 */ >> .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ >> .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ >> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x >> index 4f13cef662..c2ae5c5748 100644 >> --- a/src/remote/remote_protocol.x >> +++ b/src/remote/remote_protocol.x >> @@ -230,6 +230,9 @@ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64; >> /* Upper limit on migrate parameters */ >> const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64; >> >> +/* Upper limit on save/restore parameters */ >> +const REMOTE_DOMAIN_SAVE_PARAMS_MAX = 64; >> + >> /* Upper limit on number of job stats */ >> const REMOTE_DOMAIN_JOB_STATS_MAX = 64; >> >> @@ -3227,6 +3230,12 @@ struct remote_domain_migrate_confirm3_params_args { >> int cancelled; >> }; >> >> +struct remote_domain_save_parameters_flags_args { >> + remote_nonnull_domain dom; >> + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; >> + unsigned int flags; >> +}; >> + >> /* The device removed event is the last event where we have to support >> * dual forms for back-compat to older clients; all future events can >> * use just the modern form with callbackID. */ >> @@ -6920,5 +6929,11 @@ enum remote_procedure { >> * @generate: both >> * @acl: domain:write >> */ >> - REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439 >> + REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, >> + >> + /** >> + * @generate: both >> + * @acl: domain:hibernate >> + */ >> + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 >> }; >> diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs >> index d88176781d..89eadeb644 100644 >> --- a/src/remote_protocol-structs >> +++ b/src/remote_protocol-structs >> @@ -563,6 +563,14 @@ struct remote_domain_save_flags_args { >> remote_string dxml; >> u_int flags; >> }; >> +struct remote_domain_save_parameters_flags_args { >> + remote_nonnull_domain dom; >> + struct { >> + u_int params_len; >> + remote_typed_param * params_val; >> + } params; >> + u_int flags; >> +}; > > 'ninja test' fails here, wanting this addition moved later in the file > > 41/314 libvirt / check-remote_protocol > FAIL 0.19s exit status 1 > >>> MALLOC_PERTURB_=253 LANG=C LC_ALL='' /usr/bin/python3 > /home/jfehlig/virt/gitlab/libvirt/scripts/check-remote-protocol.py > remote_protocol virt_remote_driver > /home/jfehlig/virt/gitlab/libvirt/build/src/remote/libvirt_remote_driver.a > /usr/bin/pdwtags > /home/jfehlig/virt/gitlab/libvirt/build/../src/remote_protocol-structs > --- /home/jfehlig/virt/gitlab/libvirt/build/../src/remote_protocol-structs > 2022-04-26 15:57:29.515818322 -0600 > +++ - 2022-04-26 15:58:12.933537465 -0600 > @@ -563,14 +563,6 @@ > remote_string dxml; > u_int flags; > }; > -struct remote_domain_save_parameters_flags_args { > - remote_nonnull_domain dom; > - struct { > - u_int params_len; > - remote_typed_param * params_val; > - } params; > - u_int flags; > -}; > struct remote_domain_restore_args { > remote_nonnull_string from; > }; > @@ -2609,6 +2601,14 @@ > u_int flags; > int cancelled; > }; > +struct remote_domain_save_parameters_flags_args { > + remote_nonnull_domain dom; > + struct { > + u_int params_len; > + remote_typed_param * params_val; > + } params; > + u_int flags; > +}; > struct remote_domain_event_device_removed_msg { > remote_nonnull_domain dom; > remote_nonnull_string devAlias; > 41/314 libvirt / check-remote_protocol > FAIL 0.19s exit status 1 > > Jim >> struct remote_domain_restore_args { >> remote_nonnull_string from; >> }; >> @@ -3689,4 +3697,5 @@ enum remote_procedure { >> REMOTE_PROC_NETWORK_CREATE_XML_FLAGS = 437, >> REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, >> REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, >> + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, >> }; > Hah strange I don't get this...

On Wed, Apr 27, 2022 at 10:55:34PM +0200, Claudio Fontana wrote:
On 4/27/22 12:47 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 17 ++++++++++++++++- src/remote_protocol-structs | 9 +++++++++ 3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 7e7a21fcab..1fc5d41971 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8446,6 +8446,7 @@ static virHypervisorDriver hypervisor_driver = { .domainGetControlInfo = remoteDomainGetControlInfo, /* 0.9.3 */ .domainSave = remoteDomainSave, /* 0.3.0 */ .domainSaveFlags = remoteDomainSaveFlags, /* 0.9.4 */ + .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4f13cef662..c2ae5c5748 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -230,6 +230,9 @@ const REMOTE_NODE_MEMORY_PARAMETERS_MAX = 64; /* Upper limit on migrate parameters */ const REMOTE_DOMAIN_MIGRATE_PARAM_LIST_MAX = 64;
+/* Upper limit on save/restore parameters */ +const REMOTE_DOMAIN_SAVE_PARAMS_MAX = 64; + /* Upper limit on number of job stats */ const REMOTE_DOMAIN_JOB_STATS_MAX = 64;
@@ -3227,6 +3230,12 @@ struct remote_domain_migrate_confirm3_params_args { int cancelled; };
+struct remote_domain_save_parameters_flags_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6920,5 +6929,11 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439 + REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, + + /** + * @generate: both + * @acl: domain:hibernate + */ + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index d88176781d..89eadeb644 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -563,6 +563,14 @@ struct remote_domain_save_flags_args { remote_string dxml; u_int flags; }; +struct remote_domain_save_parameters_flags_args { + remote_nonnull_domain dom; + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +};
'ninja test' fails here, wanting this addition moved later in the file
snip
Hah strange I don't get this...
You'll be missing the 'dwarves' package, which is needed to parse the XDR code to generate this test file. 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 :|

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1fc5d41971..c5b644ce49 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c2ae5c5748..7b919ef375 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args { unsigned int flags; }; +struct remote_domain_restore_parameters_flags_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 89eadeb644..72e92184ca 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args { remote_string dxml; u_int flags; }; +struct remote_domain_restore_parameters_flags_args { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_save_image_get_xml_desc_args { remote_nonnull_string file; u_int flags; @@ -3698,4 +3705,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441, }; -- 2.34.1

On 4/26/22 10:47, Claudio Fontana wrote:
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1fc5d41971..c5b644ce49 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c2ae5c5748..7b919ef375 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args { unsigned int flags; };
+struct remote_domain_restore_parameters_flags_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
I've stared at this for quite a while but can't figure out why the dispatch stub does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting the following build failure In file included from ../src/remote/remote_daemon_dispatch.c:133: src/remote/remote_daemon_dispatch_stubs.h: In function ‘remoteDispatchDomainRestoreParametersFlags’: src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1 of ‘virDomainRestoreParametersFlags’ from incompatible pointer type [-Werror=incompatible-pointer-types] 10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0) | ^~~~~~ | | | virTypedParameterPtr {aka struct _virTypedParameter *} In file included from ../include/libvirt/libvirt.h:36, from ../src/internal.h:65, from ../src/util/virerror.h:24, from ../src/remote/remote_daemon_dispatch.c:23: ../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’ {aka ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka ‘struct _virTypedParameter *’} 1576 | int virDomainRestoreParametersFlags (virConnectPtr conn, Perhaps a bug in gendispatch.pl. I'm not familiar with the script or debugging it, but others here can likely provide help. Jim
}; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 89eadeb644..72e92184ca 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args { remote_string dxml; u_int flags; }; +struct remote_domain_restore_parameters_flags_args { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_save_image_get_xml_desc_args { remote_nonnull_string file; u_int flags; @@ -3698,4 +3705,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441, };

On 4/27/22 1:00 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1fc5d41971..c5b644ce49 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c2ae5c5748..7b919ef375 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args { unsigned int flags; };
+struct remote_domain_restore_parameters_flags_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
I've stared at this for quite a while but can't figure out why the dispatch stub does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting the following build failure
In file included from ../src/remote/remote_daemon_dispatch.c:133:
src/remote/remote_daemon_dispatch_stubs.h: In function ‘remoteDispatchDomainRestoreParametersFlags’:
src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1 of ‘virDomainRestoreParametersFlags’ from incompatible pointer type [-Werror=incompatible-pointer-types]
10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0)
| ^~~~~~
| |
| virTypedParameterPtr {aka struct _virTypedParameter *}
In file included from ../include/libvirt/libvirt.h:36,
from ../src/internal.h:65,
from ../src/util/virerror.h:24,
from ../src/remote/remote_daemon_dispatch.c:23:
../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’ {aka ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka ‘struct _virTypedParameter *’}
1576 | int virDomainRestoreParametersFlags (virConnectPtr conn,
Perhaps a bug in gendispatch.pl. I'm not familiar with the script or debugging it, but others here can likely provide help.
Indeed, same issue I get. Thanks again, Claudio
Jim
}; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 89eadeb644..72e92184ca 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args { remote_string dxml; u_int flags; }; +struct remote_domain_restore_parameters_flags_args { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_save_image_get_xml_desc_args { remote_nonnull_string file; u_int flags; @@ -3698,4 +3705,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441, };

On 4/27/22 1:00 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1fc5d41971..c5b644ce49 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c2ae5c5748..7b919ef375 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args { unsigned int flags; };
+struct remote_domain_restore_parameters_flags_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
I've stared at this for quite a while but can't figure out why the dispatch stub does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting the following build failure
In file included from ../src/remote/remote_daemon_dispatch.c:133:
src/remote/remote_daemon_dispatch_stubs.h: In function ‘remoteDispatchDomainRestoreParametersFlags’:
src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1 of ‘virDomainRestoreParametersFlags’ from incompatible pointer type [-Werror=incompatible-pointer-types]
10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0)
| ^~~~~~
| |
| virTypedParameterPtr {aka struct _virTypedParameter *}
In file included from ../include/libvirt/libvirt.h:36,
from ../src/internal.h:65,
from ../src/util/virerror.h:24,
from ../src/remote/remote_daemon_dispatch.c:23:
../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’ {aka ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka ‘struct _virTypedParameter *’}
1576 | int virDomainRestoreParametersFlags (virConnectPtr conn,
Perhaps a bug in gendispatch.pl. I'm not familiar with the script or debugging it, but others here can likely provide help.
Jim
Still fighting this one, could not defeat the beast yet..
}; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 89eadeb644..72e92184ca 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -579,6 +579,13 @@ struct remote_domain_restore_flags_args { remote_string dxml; u_int flags; }; +struct remote_domain_restore_parameters_flags_args { + struct { + u_int params_len; + remote_typed_param * params_val; + } params; + u_int flags; +}; struct remote_domain_save_image_get_xml_desc_args { remote_nonnull_string file; u_int flags; @@ -3698,4 +3705,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_MEMORY_DEVICE_SIZE_CHANGE = 438, REMOTE_PROC_DOMAIN_SET_LAUNCH_SECURITY_STATE = 439, REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441, };

On Wed, Apr 27, 2022 at 10:56:22PM +0200, Claudio Fontana wrote:
On 4/27/22 1:00 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1fc5d41971..c5b644ce49 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c2ae5c5748..7b919ef375 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args { unsigned int flags; };
+struct remote_domain_restore_parameters_flags_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
I've stared at this for quite a while but can't figure out why the dispatch stub does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting the following build failure
In file included from ../src/remote/remote_daemon_dispatch.c:133:
src/remote/remote_daemon_dispatch_stubs.h: In function ‘remoteDispatchDomainRestoreParametersFlags’:
src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1 of ‘virDomainRestoreParametersFlags’ from incompatible pointer type [-Werror=incompatible-pointer-types]
10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0)
| ^~~~~~
| |
| virTypedParameterPtr {aka struct _virTypedParameter *}
In file included from ../include/libvirt/libvirt.h:36,
from ../src/internal.h:65,
from ../src/util/virerror.h:24,
from ../src/remote/remote_daemon_dispatch.c:23:
../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’ {aka ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka ‘struct _virTypedParameter *’}
1576 | int virDomainRestoreParametersFlags (virConnectPtr conn,
Perhaps a bug in gendispatch.pl. I'm not familiar with the script or debugging it, but others here can likely provide help.
Jim
Still fighting this one, could not defeat the beast yet..
Don't bother. gendispatch.pl is not capable of correctly handling all API designs we have and it isn't worth trying to fix it unless the problem is obvious or you enjoy reading & writing obtuse perl code ;-P In the .xdr file, switch 'generate: both' to exclude either the client or server code generation, or both, depending on which bit is broken. Then just write the code by hand. You can start with the broken code and just fix it up and add to remote_daemon_dispatch.c directly. You will see several examples not using 'generate: both' you can follow 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 4/28/22 11:34 AM, Daniel P. Berrangé wrote:
On Wed, Apr 27, 2022 at 10:56:22PM +0200, Claudio Fontana wrote:
On 4/27/22 1:00 AM, Jim Fehlig wrote:
On 4/26/22 10:47, Claudio Fontana wrote:
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 14 +++++++++++++- src/remote_protocol-structs | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 1fc5d41971..c5b644ce49 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParametersFlags = remoteDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = remoteDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index c2ae5c5748..7b919ef375 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3236,6 +3236,11 @@ struct remote_domain_save_parameters_flags_args { unsigned int flags; };
+struct remote_domain_restore_parameters_flags_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + /* The device removed event is the last event where we have to support * dual forms for back-compat to older clients; all future events can * use just the modern form with callbackID. */ @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMETERS_FLAGS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMETERS_FLAGS = 441
I've stared at this for quite a while but can't figure out why the dispatch stub does not pass virConnectPtr to virDomainRestoreParametersFlags. I'm hitting the following build failure
In file included from ../src/remote/remote_daemon_dispatch.c:133:
src/remote/remote_daemon_dispatch_stubs.h: In function ‘remoteDispatchDomainRestoreParametersFlags’:
src/remote/remote_daemon_dispatch_stubs.h:10080:41: error: passing argument 1 of ‘virDomainRestoreParametersFlags’ from incompatible pointer type [-Werror=incompatible-pointer-types]
10080 | if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0)
| ^~~~~~
| |
| virTypedParameterPtr {aka struct _virTypedParameter *}
In file included from ../include/libvirt/libvirt.h:36,
from ../src/internal.h:65,
from ../src/util/virerror.h:24,
from ../src/remote/remote_daemon_dispatch.c:23:
../include/libvirt/libvirt-domain.h:1576:72: note: expected ‘virConnectPtr’ {aka ‘struct _virConnect *’} but argument is of type ‘virTypedParameterPtr’ {aka ‘struct _virTypedParameter *’}
1576 | int virDomainRestoreParametersFlags (virConnectPtr conn,
Perhaps a bug in gendispatch.pl. I'm not familiar with the script or debugging it, but others here can likely provide help.
Jim
Still fighting this one, could not defeat the beast yet..
Don't bother. gendispatch.pl is not capable of correctly handling all API designs we have and it isn't worth trying to fix it unless the problem is obvious or you enjoy reading & writing obtuse perl code ;-P
In the .xdr file, switch 'generate: both' to exclude either the client or server code generation, or both, depending on which bit is broken. Then just write the code by hand. You can start with the broken code and just fix it up and add to remote_daemon_dispatch.c directly. You will see several examples not using 'generate: both' you can follow
With regards, Daniel
I actually think I found the problem. It needs to be put explicitly into a list of methods that require conn, like NodeSetMemoryParameters. Here is the change: commit 192d8e56e88c00ede94768f6f73c6be64f31c789 (HEAD -> api_draft) Author: Claudio Fontana <cfontana@suse.de> Date: Thu Apr 28 08:16:27 2022 -0600 gendispatch: add DomainRestoreParametersFlags as requiring conn argument Signed-off-by: Claudio Fontana <cfontana@suse.de> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 9f5bf0e316..bce88cfc52 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -637,7 +637,10 @@ elsif ($mode eq "server") { } elsif ($args_member =~ m/^remote_typed_param (\S+)<(\S+)>;/) { push(@vars_list, "virTypedParameterPtr $1 = NULL"); push(@vars_list, "int n$1 = 0"); - if ($call->{ProcName} eq "NodeSetMemoryParameters") { + + # NB: if your new API starts with remote_typed_params, enter it here if you need + # the conn arg to be passed first! + if ($call->{ProcName} eq "NodeSetMemoryParameters" || $call->{ProcName} eq "DomainRestoreParametersFlags") { push(@args_list, $conn_var); } push(@args_list, "$1");

On Thu, Apr 28, 2022 at 04:20:13PM +0200, Claudio Fontana wrote:
I actually think I found the problem. It needs to be put explicitly into a list of methods that require conn, like NodeSetMemoryParameters.
Ah, lucky someone already hit the same problem scenario
Here is the change:
commit 192d8e56e88c00ede94768f6f73c6be64f31c789 (HEAD -> api_draft) Author: Claudio Fontana <cfontana@suse.de> Date: Thu Apr 28 08:16:27 2022 -0600
gendispatch: add DomainRestoreParametersFlags as requiring conn argument
Signed-off-by: Claudio Fontana <cfontana@suse.de>
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 9f5bf0e316..bce88cfc52 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -637,7 +637,10 @@ elsif ($mode eq "server") { } elsif ($args_member =~ m/^remote_typed_param (\S+)<(\S+)>;/) { push(@vars_list, "virTypedParameterPtr $1 = NULL"); push(@vars_list, "int n$1 = 0"); - if ($call->{ProcName} eq "NodeSetMemoryParameters") { + + # NB: if your new API starts with remote_typed_params, enter it here if you need + # the conn arg to be passed first! + if ($call->{ProcName} eq "NodeSetMemoryParameters" || $call->{ProcName} eq "DomainRestoreParametersFlags") { push(@args_list, $conn_var); } push(@args_list, "$1");
Looks ok to me. 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 :|

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++++++++--- src/qemu/qemu_saveimage.c | 2 ++ src/qemu/qemu_saveimage.h | 1 + src/qemu/qemu_snapshot.c | 2 +- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ee0963c30d..c702376a4a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2641,7 +2641,7 @@ static int qemuDomainSaveInternal(virQEMUDriver *driver, virDomainObj *vm, const char *path, int compressed, virCommand *compressor, - const char *xmlin, unsigned int flags) + const char *xmlin, int nconn, unsigned int flags) { g_autofree char *xml = NULL; bool was_running = false; @@ -2722,7 +2722,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, xml = NULL; ret = qemuSaveImageCreate(driver, vm, path, data, compressor, - flags, VIR_ASYNC_JOB_SAVE); + nconn, flags, VIR_ASYNC_JOB_SAVE); if (ret < 0) goto endjob; @@ -2791,7 +2791,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, goto cleanup; ret = qemuDomainSaveInternal(driver, vm, path, compressed, - compressor, dxml, flags); + compressor, dxml, -1, flags); cleanup: virDomainObjEndAPI(&vm); @@ -2804,6 +2804,63 @@ qemuDomainSave(virDomainPtr dom, const char *path) return qemuDomainSaveFlags(dom, path, NULL, 0); } +static int +qemuDomainSaveParametersFlags(virDomainPtr dom, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + const char *to = NULL; + const char *dxml = NULL; + virQEMUDriver *driver = dom->conn->privateData; + int compressed; + g_autoptr(virCommand) compressor = NULL; + int ret = -1; + int nconn = 0; + virDomainObj *vm = NULL; + g_autoptr(virQEMUDriverConfig) cfg = NULL; + + virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | + VIR_DOMAIN_SAVE_RUNNING | + VIR_DOMAIN_SAVE_PAUSED | + VIR_DOMAIN_SAVE_PARALLEL, -1); + + if (virTypedParamsValidate(params, nparams, + VIR_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING, + VIR_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING, + VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT, + NULL) < 0) + return -1; + + if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_FILE, &to) < 0) + return -1; + if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_DXML, &dxml) < 0) + return -1; + if (virTypedParamsGetInt(params, nparams, VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, &nconn) < 0) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + if ((compressed = qemuSaveImageGetCompressionProgram(cfg->saveImageFormat, + &compressor, + "save", false)) < 0) + goto cleanup; + + if (!(vm = qemuDomainObjFromDomain(dom))) + goto cleanup; + + if (virDomainSaveFlagsEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto cleanup; + + ret = qemuDomainSaveInternal(driver, vm, to, compressed, + compressor, dxml, nconn, flags); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + static char * qemuDomainManagedSavePath(virQEMUDriver *driver, virDomainObj *vm) { @@ -2854,7 +2911,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags) VIR_INFO("Saving state of domain '%s' to '%s'", vm->def->name, name); ret = qemuDomainSaveInternal(driver, vm, name, compressed, - compressor, NULL, flags); + compressor, NULL, -1, flags); if (ret == 0) vm->hasManagedSave = true; @@ -20824,6 +20881,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainGetControlInfo = qemuDomainGetControlInfo, /* 0.9.3 */ .domainSave = qemuDomainSave, /* 0.2.0 */ .domainSaveFlags = qemuDomainSaveFlags, /* 0.9.4 */ + .domainSaveParametersFlags = qemuDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = qemuDomainRestore, /* 0.2.0 */ .domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */ .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */ diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 4fd4c5cfcd..6e7f067be2 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -258,6 +258,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, const char *path, virQEMUSaveData *data, virCommand *compressor, + int nconn, unsigned int flags, virDomainAsyncJob asyncJob) { @@ -269,6 +270,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, virFileWrapperFd *wrapperFd = NULL; unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; + nconn = nconn; /* unused */ /* Obtain the file handle. */ if ((flags & VIR_DOMAIN_SAVE_BYPASS_CACHE)) { wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE; diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 391cd55ed0..b3d5c02fd6 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -96,6 +96,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, const char *path, virQEMUSaveData *data, virCommand *compressor, + int nconn, unsigned int flags, virDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index b62fab7bb3..2e445e8296 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -1457,7 +1457,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, memory_existing = virFileExists(snapdef->memorysnapshotfile); if ((ret = qemuSaveImageCreate(driver, vm, snapdef->memorysnapshotfile, - data, compressor, 0, + data, compressor, -1, 0, VIR_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; -- 2.34.1

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_driver.c | 53 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c702376a4a..9f5ae687e8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5811,12 +5811,12 @@ static int qemuNodeGetSecurityModel(virConnectPtr conn, return 0; } - static int -qemuDomainRestoreFlags(virConnectPtr conn, - const char *path, - const char *dxml, - unsigned int flags) +qemuDomainRestoreInternal(virConnectPtr conn, + const char *path, + const char *dxml, + int nchannels, + unsigned int flags) { virQEMUDriver *driver = conn->privateData; qemuDomainObjPrivate *priv = NULL; @@ -5834,7 +5834,8 @@ qemuDomainRestoreFlags(virConnectPtr conn, virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED | - VIR_DOMAIN_SAVE_RESET_NVRAM, -1); + VIR_DOMAIN_SAVE_RESET_NVRAM | + VIR_DOMAIN_SAVE_PARALLEL, -1); if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM) reset_nvram = true; @@ -5913,11 +5914,48 @@ qemuDomainRestoreFlags(virConnectPtr conn, return ret; } +static int +qemuDomainRestoreFlags(virConnectPtr conn, + const char *path, + const char *dxml, + unsigned int flags) +{ + return qemuDomainRestoreInternal(conn, path, dxml, -1, flags); +} + static int qemuDomainRestore(virConnectPtr conn, const char *path) { - return qemuDomainRestoreFlags(conn, path, NULL, 0); + return qemuDomainRestoreInternal(conn, path, NULL, -1, 0); +} + +static int +qemuDomainRestoreParametersFlags(virConnectPtr conn, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + const char *path = NULL; + const char *dxml = NULL; + int ret = -1; + int nchannels = 2; + + if (virTypedParamsValidate(params, nparams, + VIR_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING, + VIR_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING, + VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, VIR_TYPED_PARAM_INT, + NULL) < 0) + return -1; + + if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_FILE, &path) < 0) + return -1; + if (virTypedParamsGetString(params, nparams, VIR_SAVE_PARAM_DXML, &dxml) < 0) + return -1; + if (virTypedParamsGetInt(params, nparams, VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, &nchannels) < 0) + return -1; + + ret = qemuDomainRestoreInternal(conn, path, dxml, nchannels, flags); + return ret; } static char * @@ -20884,6 +20922,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSaveParametersFlags = qemuDomainSaveParametersFlags, /* 8.3.0 */ .domainRestore = qemuDomainRestore, /* 0.2.0 */ .domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParametersFlags = qemuDomainRestoreParametersFlags, /* 8.3.0 */ .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = qemuDomainCoreDump, /* 0.7.0 */ -- 2.34.1

use this data type to encapsulate the pathname, file descriptor, wrapper, and need to unlink. Adapt qemuSaveImageCreate and qemuSaveImageOpen to use a virQEMUSaveFd instead of a plain file descriptor. This makes management of wrapper, need_unlink etc much easier. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_driver.c | 100 +++++++++------ src/qemu/qemu_saveimage.c | 259 ++++++++++++++++++++++---------------- src/qemu/qemu_saveimage.h | 27 +++- 3 files changed, 235 insertions(+), 151 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9f5ae687e8..914be2e703 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5824,12 +5824,13 @@ qemuDomainRestoreInternal(virConnectPtr conn, virDomainObj *vm = NULL; g_autofree char *xmlout = NULL; const char *newxml = dxml; - int fd = -1; int ret = -1; virQEMUSaveData *data = NULL; - virFileWrapperFd *wrapperFd = NULL; + virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; bool hook_taint = false; bool reset_nvram = false; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + int oflags = O_RDONLY; virCheckFlags(VIR_DOMAIN_SAVE_BYPASS_CACHE | VIR_DOMAIN_SAVE_RUNNING | @@ -5840,10 +5841,18 @@ qemuDomainRestoreInternal(virConnectPtr conn, if (flags & VIR_DOMAIN_SAVE_RESET_NVRAM) reset_nvram = true; - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) != 0, - &wrapperFd, false, false); - if (fd < 0) + if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) { + if (virFileDirectFdFlag() < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("bypass cache unsupported by this system")); + return -1; + } + oflags |= O_DIRECT; + } + if (virQEMUSaveFdInit(&saveFd, path, 0, oflags, cfg) < 0) + return -1; + + if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0) goto cleanup; if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) @@ -5897,16 +5906,13 @@ qemuDomainRestoreInternal(virConnectPtr conn, flags) < 0) goto cleanup; - ret = qemuSaveImageStartVM(conn, driver, vm, &fd, data, path, + ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path, false, reset_nvram, VIR_ASYNC_JOB_START); qemuProcessEndJob(vm); cleanup: - VIR_FORCE_CLOSE(fd); - if (virFileWrapperFdClose(wrapperFd) < 0) - ret = -1; - virFileWrapperFdFree(wrapperFd); + ret = virQEMUSaveFdFini(&saveFd, vm, ret); virQEMUSaveDataFree(data); if (vm && ret < 0) qemuDomainRemoveInactive(driver, vm); @@ -5965,15 +5971,16 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, virQEMUDriver *driver = conn->privateData; char *ret = NULL; g_autoptr(virDomainDef) def = NULL; - int fd = -1; virQEMUSaveData *data = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL); - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - false, NULL, false, false); + if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDONLY, cfg) < 0) + return NULL; - if (fd < 0) + if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0) goto cleanup; if (virDomainSaveImageGetXMLDescEnsureACL(conn, def) < 0) @@ -5983,7 +5990,8 @@ qemuDomainSaveImageGetXMLDesc(virConnectPtr conn, const char *path, cleanup: virQEMUSaveDataFree(data); - VIR_FORCE_CLOSE(fd); + if (virQEMUSaveFdFini(&saveFd, NULL, ret ? 0 : -1) < 0) + ret = NULL; return ret; } @@ -5995,22 +6003,23 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, int ret = -1; g_autoptr(virDomainDef) def = NULL; g_autoptr(virDomainDef) newdef = NULL; - int fd = -1; virQEMUSaveData *data = NULL; + virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); int state = -1; virCheckFlags(VIR_DOMAIN_SAVE_RUNNING | VIR_DOMAIN_SAVE_PAUSED, -1); + if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDWR, cfg) < 0) + return -1; + if (flags & VIR_DOMAIN_SAVE_RUNNING) state = 1; else if (flags & VIR_DOMAIN_SAVE_PAUSED) state = 0; - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - false, NULL, true, false); - - if (fd < 0) + if (qemuSaveImageOpen(driver, NULL, &def, &data, false, &saveFd) < 0) goto cleanup; if (virDomainSaveImageDefineXMLEnsureACL(conn, def) < 0) @@ -6037,15 +6046,15 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, VIR_DOMAIN_XML_MIGRATABLE))) goto cleanup; - if (lseek(fd, 0, SEEK_SET) != 0) { + if (lseek(saveFd.fd, 0, SEEK_SET) != 0) { virReportSystemError(errno, _("cannot seek in '%s'"), path); goto cleanup; } - if (virQEMUSaveDataWrite(data, fd, path) < 0) + if (virQEMUSaveDataWrite(data, saveFd.fd, path) < 0) goto cleanup; - if (VIR_CLOSE(fd) < 0) { + if (virQEMUSaveFdClose(&saveFd, NULL) < 0) { virReportSystemError(errno, _("failed to write header data to '%s'"), path); goto cleanup; } @@ -6053,8 +6062,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, ret = 0; cleanup: - VIR_FORCE_CLOSE(fd); virQEMUSaveDataFree(data); + ret = virQEMUSaveFdFini(&saveFd, NULL, ret); return ret; } @@ -6066,8 +6075,9 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) g_autofree char *path = NULL; char *ret = NULL; g_autoptr(virDomainDef) def = NULL; - int fd = -1; virQEMUSaveData *data = NULL; + virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivate *priv; virCheckFlags(VIR_DOMAIN_SAVE_IMAGE_XML_SECURE, NULL); @@ -6089,15 +6099,19 @@ qemuDomainManagedSaveGetXMLDesc(virDomainPtr dom, unsigned int flags) goto cleanup; } - if ((fd = qemuSaveImageOpen(driver, priv->qemuCaps, path, &def, &data, - false, NULL, false, false)) < 0) + if (virQEMUSaveFdInit(&saveFd, path, 0, O_RDONLY, cfg) < 0) + goto cleanup; + + if (qemuSaveImageOpen(driver, priv->qemuCaps, &def, &data, false, + &saveFd) < 0) goto cleanup; ret = qemuDomainDefFormatXML(driver, priv->qemuCaps, def, flags); cleanup: virQEMUSaveDataFree(data); - VIR_FORCE_CLOSE(fd); + if (virQEMUSaveFdFini(&saveFd, vm, ret ? 0 : -1) < 0) + ret = NULL; virDomainObjEndAPI(&vm); return ret; } @@ -6148,16 +6162,25 @@ qemuDomainObjRestore(virConnectPtr conn, { g_autoptr(virDomainDef) def = NULL; qemuDomainObjPrivate *priv = vm->privateData; - int fd = -1; int ret = -1; g_autofree char *xmlout = NULL; virQEMUSaveData *data = NULL; - virFileWrapperFd *wrapperFd = NULL; + virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; + int oflags = O_RDONLY; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - fd = qemuSaveImageOpen(driver, NULL, path, &def, &data, - bypass_cache, &wrapperFd, false, true); - if (fd < 0) { - if (fd == -3) + if (bypass_cache) { + if (virFileDirectFdFlag() < 0) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("bypass cache unsupported by this system")); + return -1; + } + oflags |= O_DIRECT; + } + + ret = qemuSaveImageOpen(driver, NULL, &def, &data, true, &saveFd); + if (ret < 0) { + if (ret == -3) ret = 1; goto cleanup; } @@ -6201,15 +6224,12 @@ qemuDomainObjRestore(virConnectPtr conn, virDomainObjAssignDef(vm, &def, true, NULL); - ret = qemuSaveImageStartVM(conn, driver, vm, &fd, data, path, + ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path, start_paused, reset_nvram, asyncJob); cleanup: virQEMUSaveDataFree(data); - VIR_FORCE_CLOSE(fd); - if (virFileWrapperFdClose(wrapperFd) < 0) - ret = -1; - virFileWrapperFdFree(wrapperFd); + ret = virQEMUSaveFdFini(&saveFd, vm, ret); return ret; } diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 6e7f067be2..6af256aabe 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -248,6 +248,123 @@ qemuSaveImageGetCompressionCommand(virQEMUSaveFormat compression) return ret; } +/* + * virQEMUSaveFdInit: initialize a virQEMUSaveFd + * + * @saveFd: the structure to initialize + * @base: the main file name + * @idx: 0 for the main file, >0 for multifd channels. + * @oflags the file descriptor open flags + * @cfg: the driver config + * + * Returns -1 on error, 0 on success, + * and in both cases virQEMUSaveFdFini must be called to free resources. + */ +int virQEMUSaveFdInit(virQEMUSaveFd *saveFd, const char *base, int idx, + int oflags, virQEMUDriverConfig *cfg) +{ + unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; + bool isCreat = oflags & O_CREAT; + bool isDirect = O_DIRECT && (oflags & O_DIRECT); + + if (isDirect) + wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE; + if (idx > 0) { + saveFd->path = g_strdup_printf("%s.%d", base, idx); + } else { + saveFd->path = g_strdup(base); + } + saveFd->wrapper = NULL; + if (isCreat) { + saveFd->fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, saveFd->path, + oflags, &saveFd->need_unlink); + } else { + saveFd->fd = qemuDomainOpenFile(cfg, NULL, saveFd->path, oflags, NULL); + } + if (saveFd->fd < 0) + return -1; + /* + * no wrapper required for the multifd channels. + * For O_CREAT, we always add the wrapper for the main file. + * For !O_CREAT, we only add the wrapper if using O_DIRECT. + */ + if (idx == 0 && (isDirect || isCreat)) { + saveFd->wrapper = virFileWrapperFdNew(&saveFd->fd, saveFd->path, wrapperFlags); + if (!saveFd->wrapper) + return -1; + } + return 0; +} + +/* + * virQEMUSaveFdClose: close a virQEMUSaveFd descriptor with normal close. + * + * @saveFd: the saveFd structure with the file descriptors to close. + * @vm: the virDomainObj (necessary to release lock), or NULL. + * + * If saveFd is NULL, the function will return success. + * + * Returns -1 on error, 0 on success. + */ +int virQEMUSaveFdClose(virQEMUSaveFd *saveFd, virDomainObj *vm) +{ + if (!saveFd) + return 0; + + if (VIR_CLOSE(saveFd->fd) < 0) { + virReportSystemError(errno, _("unable to close %s"), saveFd->path); + return -1; + } + if (vm) { + if (qemuDomainFileWrapperFDClose(vm, saveFd->wrapper) < 0) + return -1; + } else { + if (virFileWrapperFdClose(saveFd->wrapper) < 0) + return -1; + } + return 0; +} + +/* + * virQEMUSaveFdFini: finalize a virQEMUSaveFd + * + * @saveFd: the saveFd structure containing the resources to free. + * @vm: the virDomainObj (necessary to release lock for long close ops), or NULL. + * @ret: the current operation result (< 0 is failure) + * + * If saveFd is NULL, the return value will be unchanged. + * + * Returns ret, or -1 if an error is detected. + */ +int virQEMUSaveFdFini(virQEMUSaveFd *saveFd, virDomainObj *vm, int ret) +{ + if (!saveFd) + return ret; + VIR_FORCE_CLOSE(saveFd->fd); + if (vm) { + if (qemuDomainFileWrapperFDClose(vm, saveFd->wrapper) < 0) + ret = -1; + } else { + if (virFileWrapperFdClose(saveFd->wrapper) < 0) + ret = -1; + } + + if (ret < 0 && saveFd->need_unlink && saveFd->path) { + if (unlink(saveFd->path) < 0) { + virReportSystemError(errno, _("cannot remove file: %s"), + saveFd->path); + } + } + if (saveFd->wrapper) { + virFileWrapperFdFree(saveFd->wrapper); + saveFd->wrapper = NULL; + } + + g_free(saveFd->path); + saveFd->path = NULL; + 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 @@ -263,42 +380,34 @@ qemuSaveImageCreate(virQEMUDriver *driver, virDomainAsyncJob asyncJob) { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - bool needUnlink = false; + virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; + unsigned int oflags = O_WRONLY | O_TRUNC | O_CREAT; int ret = -1; - int fd = -1; - int directFlag = 0; - virFileWrapperFd *wrapperFd = NULL; - unsigned int wrapperFlags = VIR_FILE_WRAPPER_NON_BLOCKING; - nconn = nconn; /* unused */ - /* Obtain the file handle. */ - if ((flags & VIR_DOMAIN_SAVE_BYPASS_CACHE)) { - wrapperFlags |= VIR_FILE_WRAPPER_BYPASS_CACHE; - directFlag = virFileDirectFdFlag(); - if (directFlag < 0) { + + if (flags & VIR_DOMAIN_SAVE_BYPASS_CACHE) { + if (virFileDirectFdFlag() < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("bypass cache unsupported by this system")); - goto cleanup; + return -1; } + oflags |= O_DIRECT; } - fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, path, - O_WRONLY | O_TRUNC | O_CREAT | directFlag, - &needUnlink); - if (fd < 0) + if (virQEMUSaveFdInit(&saveFd, path, 0, oflags, cfg) < 0) { goto cleanup; - - if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, fd) < 0) + } + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, saveFd.fd) < 0) goto cleanup; - if (!(wrapperFd = virFileWrapperFdNew(&fd, path, wrapperFlags))) + if (virQEMUSaveDataWrite(data, saveFd.fd, saveFd.path) < 0) goto cleanup; - if (virQEMUSaveDataWrite(data, fd, path) < 0) - goto cleanup; /* Perform the migration */ - if (qemuMigrationSrcToFile(driver, vm, fd, compressor, asyncJob) < 0) + if (qemuMigrationSrcToFile(driver, vm, saveFd.fd, compressor, asyncJob) < 0) + goto cleanup; + if (virQEMUSaveFdClose(&saveFd, vm) < 0) goto cleanup; /* Touch up file header to mark image complete. */ @@ -307,29 +416,17 @@ qemuSaveImageCreate(virQEMUDriver *driver, * 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 %s"), path); - 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) + if ((saveFd.fd = qemuDomainOpenFile(cfg, vm->def, saveFd.path, O_WRONLY, NULL)) < 0 || + virQEMUSaveDataFinish(data, &saveFd.fd, saveFd.path) < 0) goto cleanup; ret = 0; - cleanup: - VIR_FORCE_CLOSE(fd); - if (qemuDomainFileWrapperFDClose(vm, wrapperFd) < 0) - ret = -1; - virFileWrapperFdFree(wrapperFd); - if (ret < 0 && needUnlink) - unlink(path); + cleanup: + ret = virQEMUSaveFdFini(&saveFd, vm, ret); return ret; } @@ -422,94 +519,49 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, * @path: path of the save image * @ret_def: returns domain definition created from the XML stored in the image * @ret_data: returns structure filled with data from the image header - * @bypass_cache: bypass cache when opening the file - * @wrapperFd: returns the file wrapper structure - * @open_write: open the file for writing (for updates) - * @unlink_corrupt: remove the image file if it is corrupted + * @unlink_corrupt: mark the image file for removal if it is corrupted + * @saveFd: the save file * - * Returns the opened fd of the save image file and fills the appropriate fields - * on success. On error returns -1 on most failures, -3 if corrupt image was - * unlinked (no error raised). + * Returns 0 on success or -1 on failure. + * On success, the appropriate fields are filled. */ int qemuSaveImageOpen(virQEMUDriver *driver, virQEMUCaps *qemuCaps, - const char *path, virDomainDef **ret_def, virQEMUSaveData **ret_data, - bool bypass_cache, - virFileWrapperFd **wrapperFd, - bool open_write, - bool unlink_corrupt) + bool unlink_corrupt, + virQEMUSaveFd *saveFd) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - VIR_AUTOCLOSE fd = -1; - int ret = -1; g_autoptr(virQEMUSaveData) data = NULL; virQEMUSaveHeader *header; g_autoptr(virDomainDef) def = NULL; - int oflags = open_write ? O_RDWR : O_RDONLY; size_t xml_len; size_t cookie_len; - if (bypass_cache) { - int directFlag = virFileDirectFdFlag(); - if (directFlag < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("bypass cache unsupported by this system")); - return -1; - } - oflags |= directFlag; - } - - if ((fd = qemuDomainOpenFile(cfg, NULL, path, oflags, NULL)) < 0) - return -1; - - if (bypass_cache && - !(*wrapperFd = virFileWrapperFdNew(&fd, path, - VIR_FILE_WRAPPER_BYPASS_CACHE))) - return -1; data = g_new0(virQEMUSaveData, 1); header = &data->header; - if (saferead(fd, header, sizeof(*header)) != sizeof(*header)) { - if (unlink_corrupt) { - if (unlink(path) < 0) { - virReportSystemError(errno, - _("cannot remove corrupt file: %s"), - path); - return -1; - } else { - return -3; - } - } - + if (saferead(saveFd->fd, header, sizeof(*header)) != sizeof(*header)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read qemu header")); + if (unlink_corrupt) { + saveFd->need_unlink = true; + } return -1; } if (memcmp(header->magic, QEMU_SAVE_MAGIC, sizeof(header->magic)) != 0) { - if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0) { - if (unlink_corrupt) { - if (unlink(path) < 0) { - virReportSystemError(errno, - _("cannot remove corrupt file: %s"), - path); - return -1; - } else { - return -3; - } - } - + if (memcmp(header->magic, QEMU_SAVE_PARTIAL, sizeof(header->magic)) == 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("save image is incomplete")); - return -1; + else + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("image magic is incorrect")); + if (unlink_corrupt) { + saveFd->need_unlink = true; } - - virReportError(VIR_ERR_OPERATION_FAILED, "%s", - _("image magic is incorrect")); return -1; } @@ -540,7 +592,7 @@ qemuSaveImageOpen(virQEMUDriver *driver, data->xml = g_new0(char, xml_len); - if (saferead(fd, data->xml, xml_len) != xml_len) { + if (saferead(saveFd->fd, data->xml, xml_len) != xml_len) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read domain XML")); return -1; @@ -549,7 +601,7 @@ qemuSaveImageOpen(virQEMUDriver *driver, if (cookie_len > 0) { data->cookie = g_new0(char, cookie_len); - if (saferead(fd, data->cookie, cookie_len) != cookie_len) { + if (saferead(saveFd->fd, data->cookie, cookie_len) != cookie_len) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("failed to read cookie")); return -1; @@ -565,10 +617,7 @@ qemuSaveImageOpen(virQEMUDriver *driver, *ret_def = g_steal_pointer(&def); *ret_data = g_steal_pointer(&data); - ret = fd; - fd = -1; - - return ret; + return 0; } int diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index b3d5c02fd6..5dc63f3661 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -54,6 +54,24 @@ struct _virQEMUSaveData { }; +typedef struct _virQEMUSaveFd virQEMUSaveFd; +struct _virQEMUSaveFd { + char *path; + int fd; + bool need_unlink; + virFileWrapperFd *wrapper; +}; + +#define QEMU_SAVEFD_INVALID (virQEMUSaveFd) { .path = NULL, .fd = -1, .need_unlink = false, .wrapper = NULL } + +int virQEMUSaveFdInit(virQEMUSaveFd *saveFd, const char *base, int idx, + int oflags, virQEMUDriverConfig *cfg) + ATTRIBUTE_NONNULL(5); + +int virQEMUSaveFdClose(virQEMUSaveFd *saveFd, virDomainObj *vm); + +int virQEMUSaveFdFini(virQEMUSaveFd *saveFd, virDomainObj *vm, int ret); + virDomainDef * qemuSaveImageUpdateDef(virQEMUDriver *driver, virDomainDef *def, @@ -74,14 +92,11 @@ qemuSaveImageStartVM(virConnectPtr conn, int qemuSaveImageOpen(virQEMUDriver *driver, virQEMUCaps *qemuCaps, - const char *path, virDomainDef **ret_def, virQEMUSaveData **ret_data, - bool bypass_cache, - virFileWrapperFd **wrapperFd, - bool open_write, - bool unlink_corrupt) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); + bool unlink_corrupt, + virQEMUSaveFd *saveFd) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6); int qemuSaveImageGetCompressionProgram(const char *imageFormat, -- 2.34.1

where it can be reused by other helpers. No changes other than the move. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/iohelper.c | 178 +---------------------------------- src/util/meson.build | 2 + src/util/runio.c | 214 +++++++++++++++++++++++++++++++++++++++++++ src/util/runio.h | 23 +++++ 4 files changed, 240 insertions(+), 177 deletions(-) create mode 100644 src/util/runio.c create mode 100644 src/util/runio.h diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 1584321839..5a0098542e 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -38,183 +38,7 @@ #include "virrandom.h" #include "virstring.h" #include "virgettext.h" - -#define VIR_FROM_THIS VIR_FROM_STORAGE - -#ifndef O_DIRECT -# define O_DIRECT 0 -#endif - -struct runIOParams { - bool isBlockDev; - bool isDirect; - bool isWrite; - int fdin; - const char *fdinname; - int fdout; - const char *fdoutname; -}; - -/** - * runIOCopy: execute the IO copy based on the passed parameters - * @p: the IO parameters - * - * Execute the copy based on the passed parameters. - * - * Returns: size transfered, or < 0 on error. - */ - -static off_t -runIOCopy(const struct runIOParams p) -{ - g_autofree void *base = NULL; /* Location to be freed */ - char *buf = NULL; /* Aligned location within base */ - size_t buflen = 1024*1024; - intptr_t alignMask = 64*1024 - 1; - off_t total = 0; - -#if WITH_POSIX_MEMALIGN - if (posix_memalign(&base, alignMask + 1, buflen)) - abort(); - buf = base; -#else - buf = g_new0(char, buflen + alignMask); - base = buf; - buf = (char *) (((intptr_t) base + alignMask) & ~alignMask); -#endif - - while (1) { - ssize_t got; - - /* If we read with O_DIRECT from file we can't use saferead as - * it can lead to unaligned read after reading last bytes. - * If we write with O_DIRECT use should use saferead so that - * writes will be aligned. - * In other cases using saferead reduces number of syscalls. - */ - if (!p.isWrite && p.isDirect) { - if ((got = read(p.fdin, buf, buflen)) < 0 && - errno == EINTR) - continue; - } else { - got = saferead(p.fdin, buf, buflen); - } - - if (got < 0) { - virReportSystemError(errno, _("Unable to read %s"), p.fdinname); - return -2; - } - if (got == 0) - break; - - total += got; - - /* handle last write size align in direct case */ - if (got < buflen && p.isDirect && p.isWrite) { - ssize_t aligned_got = (got + alignMask) & ~alignMask; - - memset(buf + got, 0, aligned_got - got); - - if (safewrite(p.fdout, buf, aligned_got) < 0) { - virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); - return -3; - } - - if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) { - virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname); - return -4; - } - - break; - } - - if (safewrite(p.fdout, buf, got) < 0) { - virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); - return -3; - } - } - return total; -} - -static int -runIO(const char *path, int fd, int oflags) -{ - int ret = -1; - off_t total = 0; - struct stat sb; - struct runIOParams p; - - if (fstat(fd, &sb) < 0) { - virReportSystemError(errno, - _("Unable to access file descriptor %d path %s"), - fd, path); - goto cleanup; - } - p.isBlockDev = S_ISBLK(sb.st_mode); - p.isDirect = O_DIRECT && (oflags & O_DIRECT); - - switch (oflags & O_ACCMODE) { - case O_RDONLY: - p.isWrite = false; - p.fdin = fd; - p.fdinname = path; - p.fdout = STDOUT_FILENO; - p.fdoutname = "stdout"; - break; - case O_WRONLY: - p.isWrite = true; - p.fdin = STDIN_FILENO; - p.fdinname = "stdin"; - p.fdout = fd; - p.fdoutname = path; - break; - - case O_RDWR: - default: - virReportSystemError(EINVAL, - _("Unable to process file with flags %d"), - (oflags & O_ACCMODE)); - goto cleanup; - } - /* To make the implementation simpler, we give up on any - * attempt to use O_DIRECT in a non-trivial manner. */ - if (!p.isBlockDev && p.isDirect) { - off_t off; - if (p.isWrite) { - if ((off = lseek(fd, 0, SEEK_END)) != 0) { - virReportSystemError(off < 0 ? errno : EINVAL, "%s", - _("O_DIRECT write needs empty seekable file")); - goto cleanup; - } - } else if ((off = lseek(fd, 0, SEEK_CUR)) != 0) { - virReportSystemError(off < 0 ? errno : EINVAL, "%s", - _("O_DIRECT read needs entire seekable file")); - goto cleanup; - } - } - total = runIOCopy(p); - if (total < 0) - goto cleanup; - - /* Ensure all data is written */ - if (virFileDataSync(p.fdout) < 0) { - if (errno != EINVAL && errno != EROFS) { - /* fdatasync() may fail on some special FDs, e.g. pipes */ - virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname); - goto cleanup; - } - } - - ret = 0; - - cleanup: - if (VIR_CLOSE(fd) < 0 && - ret == 0) { - virReportSystemError(errno, _("Unable to close %s"), path); - ret = -1; - } - return ret; -} +#include "runio.h" static const char *program_name; diff --git a/src/util/meson.build b/src/util/meson.build index 24350a3e67..58001a1699 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -175,6 +175,8 @@ keycode_dep = declare_dependency( io_helper_sources = [ 'iohelper.c', + 'runio.c', + 'runio.h', ] virt_util_lib = static_library( diff --git a/src/util/runio.c b/src/util/runio.c new file mode 100644 index 0000000000..a7b902af7e --- /dev/null +++ b/src/util/runio.c @@ -0,0 +1,214 @@ +/* + * runio.c: I/O copy function + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include <unistd.h> +#include <fcntl.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> + +#include "virthread.h" +#include "virfile.h" +#include "viralloc.h" +#include "virerror.h" +#include "virrandom.h" +#include "virstring.h" +#include "virgettext.h" +#include "runio.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +#ifndef O_DIRECT +# define O_DIRECT 0 +#endif + +struct runIOParams { + bool isBlockDev; + bool isDirect; + bool isWrite; + int fdin; + const char *fdinname; + int fdout; + const char *fdoutname; +}; + +/** + * runIOCopy: execute the IO copy based on the passed parameters + * @p: the IO parameters + * + * Execute the copy based on the passed parameters. + * + * Returns: size transfered, or < 0 on error. + */ + +static off_t +runIOCopy(const struct runIOParams p) +{ + g_autofree void *base = NULL; /* Location to be freed */ + char *buf = NULL; /* Aligned location within base */ + size_t buflen = 1024*1024; + intptr_t alignMask = 64*1024 - 1; + off_t total = 0; + +#if WITH_POSIX_MEMALIGN + if (posix_memalign(&base, alignMask + 1, buflen)) + abort(); + buf = base; +#else + buf = g_new0(char, buflen + alignMask); + base = buf; + buf = (char *) (((intptr_t) base + alignMask) & ~alignMask); +#endif + + while (1) { + ssize_t got; + + /* If we read with O_DIRECT from file we can't use saferead as + * it can lead to unaligned read after reading last bytes. + * If we write with O_DIRECT use should use saferead so that + * writes will be aligned. + * In other cases using saferead reduces number of syscalls. + */ + if (!p.isWrite && p.isDirect) { + if ((got = read(p.fdin, buf, buflen)) < 0 && + errno == EINTR) + continue; + } else { + got = saferead(p.fdin, buf, buflen); + } + + if (got < 0) { + virReportSystemError(errno, _("Unable to read %s"), p.fdinname); + return -2; + } + if (got == 0) + break; + + total += got; + + /* handle last write size align in direct case */ + if (got < buflen && p.isDirect && p.isWrite) { + ssize_t aligned_got = (got + alignMask) & ~alignMask; + + memset(buf + got, 0, aligned_got - got); + + if (safewrite(p.fdout, buf, aligned_got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); + return -3; + } + + if (!p.isBlockDev && ftruncate(p.fdout, total) < 0) { + virReportSystemError(errno, _("Unable to truncate %s"), p.fdoutname); + return -4; + } + + break; + } + + if (safewrite(p.fdout, buf, got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), p.fdoutname); + return -3; + } + } + return total; +} + + +off_t +runIO(const char *path, int fd, int oflags) +{ + int ret = -1; + off_t total = 0; + struct stat sb; + struct runIOParams p; + + if (fstat(fd, &sb) < 0) { + virReportSystemError(errno, + _("Unable to access file descriptor %d path %s"), + fd, path); + goto cleanup; + } + p.isBlockDev = S_ISBLK(sb.st_mode); + p.isDirect = O_DIRECT && (oflags & O_DIRECT); + + switch (oflags & O_ACCMODE) { + case O_RDONLY: + p.isWrite = false; + p.fdin = fd; + p.fdinname = path; + p.fdout = STDOUT_FILENO; + p.fdoutname = "stdout"; + break; + case O_WRONLY: + p.isWrite = true; + p.fdin = STDIN_FILENO; + p.fdinname = "stdin"; + p.fdout = fd; + p.fdoutname = path; + break; + + case O_RDWR: + default: + virReportSystemError(EINVAL, + _("Unable to process file with flags %d"), + (oflags & O_ACCMODE)); + goto cleanup; + } + /* To make the implementation simpler, we give up on any + * attempt to use O_DIRECT in a non-trivial manner. */ + if (!p.isBlockDev && p.isDirect) { + off_t off; + if (p.isWrite) { + if ((off = lseek(fd, 0, SEEK_END)) != 0) { + virReportSystemError(off < 0 ? errno : EINVAL, "%s", + _("O_DIRECT write needs empty seekable file")); + goto cleanup; + } + } else if ((off = lseek(fd, 0, SEEK_CUR)) != 0) { + virReportSystemError(off < 0 ? errno : EINVAL, "%s", + _("O_DIRECT read needs entire seekable file")); + goto cleanup; + } + } + total = runIOCopy(p); + if (total < 0) + goto cleanup; + + /* Ensure all data is written */ + if (virFileDataSync(p.fdout) < 0) { + if (errno != EINVAL && errno != EROFS) { + /* fdatasync() may fail on some special FDs, e.g. pipes */ + virReportSystemError(errno, _("unable to fsync %s"), p.fdoutname); + goto cleanup; + } + } + + ret = 0; + + cleanup: + if (VIR_CLOSE(fd) < 0 && + ret == 0) { + virReportSystemError(errno, _("Unable to close %s"), path); + ret = -1; + } + return ret; +} diff --git a/src/util/runio.h b/src/util/runio.h new file mode 100644 index 0000000000..beb58606c9 --- /dev/null +++ b/src/util/runio.h @@ -0,0 +1,23 @@ +/* + * runio.h: I/O copy function + * + * Copyright (C) 2011-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +off_t runIO(const char *path, int fd, int oflags); -- 2.34.1

add arguments to runio to allow read/write from/to arbitrary file descriptors, as opposed to just stdin and stdout. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/util/iohelper.c | 2 +- src/util/runio.c | 10 +++++----- src/util/runio.h | 17 ++++++++++++++++- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 5a0098542e..93674c1e2f 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -96,7 +96,7 @@ main(int argc, char **argv) usage(EXIT_FAILURE); } - if (fd < 0 || runIO(path, fd, oflags) < 0) + if (fd < 0 || runIO(path, fd, oflags, STDIN_FILENO, STDOUT_FILENO) < 0) goto error; return 0; diff --git a/src/util/runio.c b/src/util/runio.c index a7b902af7e..f42acddae9 100644 --- a/src/util/runio.c +++ b/src/util/runio.c @@ -134,7 +134,7 @@ runIOCopy(const struct runIOParams p) off_t -runIO(const char *path, int fd, int oflags) +runIO(const char *path, int fd, int oflags, int in_fd, int out_fd) { int ret = -1; off_t total = 0; @@ -155,13 +155,13 @@ runIO(const char *path, int fd, int oflags) p.isWrite = false; p.fdin = fd; p.fdinname = path; - p.fdout = STDOUT_FILENO; - p.fdoutname = "stdout"; + p.fdout = out_fd; + p.fdoutname = "output"; break; case O_WRONLY: p.isWrite = true; - p.fdin = STDIN_FILENO; - p.fdinname = "stdin"; + p.fdin = in_fd; + p.fdinname = "input"; p.fdout = fd; p.fdoutname = path; break; diff --git a/src/util/runio.h b/src/util/runio.h index beb58606c9..66df588881 100644 --- a/src/util/runio.h +++ b/src/util/runio.h @@ -20,4 +20,19 @@ #pragma once -off_t runIO(const char *path, int fd, int oflags); +/* + * runIO: copy unidirectionally all data to/from a file descriptor. + * + * @path: the pathname corresponding to FD. + * @fd: the file descriptor to read from or write to. + * @oflags: the file status flags of FD. + * + * If the oflags indicate O_RDONLY, then the direction will be from FD, + * and @out_fd indicates the file to write to. + * + * If the oflags indicate O_WRONLY, then the direction will be to FD, + * and @in_fd indicates the file to read from. + * + * Returns the number of bytes transferred, or < 0 on error. + */ +off_t runIO(const char *path, int fd, int oflags, int in_fd, int out_fd); -- 2.34.1

For the save direction, this helper listens on a unix socket which QEMU connects to for multifd migration to files. For the restore direction, this helper connects to a unix socket QEMU listens at for multifd migration from files. The file descriptors are passed as command line parameters. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/libvirt_private.syms | 1 + src/util/meson.build | 13 ++ src/util/multifd-helper.c | 250 ++++++++++++++++++++++++++++++++++++++ src/util/virthread.c | 5 + src/util/virthread.h | 1 + 5 files changed, 270 insertions(+) create mode 100644 src/util/multifd-helper.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 97bfca906b..5f2bee985e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3427,6 +3427,7 @@ virThreadCreateFull; virThreadID; virThreadIsSelf; virThreadJoin; +virThreadJoinRet; virThreadMaxName; virThreadSelf; virThreadSelfID; diff --git a/src/util/meson.build b/src/util/meson.build index 58001a1699..8ea74ff9e8 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -179,6 +179,12 @@ io_helper_sources = [ 'runio.h', ] +multifd_helper_sources = [ + 'multifd-helper.c', + 'runio.c', + 'runio.h', +] + virt_util_lib = static_library( 'virt_util', [ @@ -216,6 +222,13 @@ if conf.has('WITH_LIBVIRTD') dtrace_gen_headers, ], } + virt_helpers += { + 'name': 'libvirt_multifd_helper', + 'sources': [ + files(multifd_helper_sources), + dtrace_gen_headers, + ], + } endif util_inc_dir = include_directories('.') diff --git a/src/util/multifd-helper.c b/src/util/multifd-helper.c new file mode 100644 index 0000000000..37e61a3a4d --- /dev/null +++ b/src/util/multifd-helper.c @@ -0,0 +1,250 @@ +/* + * multifd-helper.c: listens on Unix socket to perform I/O to multiple files + * + * Copyright (C) 2022 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * This has been written to support QEMU multifd migration to file, + * allowing better use of cpu resources to speed up the save/restore. + */ + +#include <config.h> + +#include <unistd.h> +#include <fcntl.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/socket.h> +#include <sys/un.h> + +#include "virthread.h" +#include "virfile.h" +#include "virerror.h" +#include "virstring.h" +#include "virgettext.h" +#include "runio.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +typedef struct _multiFdConnData multiFdConnData; +struct _multiFdConnData { + int clientfd; + int filefd; + int oflags; + const char *path; + virThread tid; + + off_t total; +}; + +typedef struct _multiFdThreadArgs multiFdThreadArgs; +struct _multiFdThreadArgs { + int nchannels; + multiFdConnData *conn; /* contains main fd + nchannels */ + const char *sun_path; /* unix socket name to use for the server */ + struct sockaddr_un serv_addr; + + off_t total; +}; + +static void clientThreadFunc(void *a) +{ + multiFdConnData *c = a; + c->total = runIO(c->path, c->filefd, c->oflags, c->clientfd, c->clientfd); +} + +static off_t waitClientThreads(multiFdConnData *conn, int n) +{ + int i; + off_t total = 0; + for (i = 0; i < n; i++) { + multiFdConnData *c = &conn[i]; + if (virThreadJoinRet(&c->tid) < 0) { + total = -1; + } else if (total >= 0) { + total += c->total; + } + if (VIR_CLOSE(c->clientfd) < 0) { + total = -1; + } + } + return total; +} + +static void loadThreadFunc(void *a) +{ + multiFdThreadArgs *args = a; + int i; + args->total = -1; + + for (i = 0; i < args->nchannels + 1; i++) { + /* Perform outgoing connections */ + multiFdConnData *c = &args->conn[i]; + c->clientfd = socket(AF_UNIX, SOCK_STREAM, 0); + if (c->clientfd < 0) { + virReportSystemError(errno, "%s", _("loadThread: socket() failed")); + goto cleanup; + } + if (connect(c->clientfd, (const struct sockaddr *)&args->serv_addr, + sizeof(struct sockaddr_un)) < 0) { + virReportSystemError(errno, "%s", _("loadThread: connect() failed")); + goto cleanup; + } + if (virThreadCreate(&c->tid, true, &clientThreadFunc, c) < 0) { + virReportSystemError(errno, "%s", _("loadThread: client thread creation failed")); + goto cleanup; + } + } + args->total = waitClientThreads(args->conn, args->nchannels + 1); + + cleanup: + for (i = 0; i < args->nchannels + 1; i++) { + multiFdConnData *c = &args->conn[i]; + VIR_FORCE_CLOSE(c->clientfd); + } +} + +static void saveThreadFunc(void *a) +{ + multiFdThreadArgs *args = a; + int i; + const char buf[1] = {'R'}; + int sockfd; + + if ((sockfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, "%s", _("saveThread: socket() failed")); + return; + } + unlink(args->sun_path); + if (bind(sockfd, (struct sockaddr *)&args->serv_addr, sizeof(args->serv_addr)) < 0) { + virReportSystemError(errno, "%s", _("saveThread: bind() failed")); + goto cleanup; + } + if (listen(sockfd, args->nchannels + 1) < 0) { + virReportSystemError(errno, "%s", _("saveThread: listen() failed")); + goto cleanup; + } + + /* signal that the server is ready */ + if (safewrite(STDOUT_FILENO, &buf, 1) != 1) { + virReportSystemError(errno, "%s", _("saveThread: safewrite failed")); + goto cleanup; + } + + for (i = 0; i < args->nchannels + 1; i++) { + /* Wait for incoming connection. */ + multiFdConnData *c = &args->conn[i]; + if ((c->clientfd = accept(sockfd, NULL, NULL)) < 0) { + virReportSystemError(errno, "%s", _("saveThread: accept() failed")); + goto cleanup; + } + if (virThreadCreate(&c->tid, true, &clientThreadFunc, c) < 0) { + virReportSystemError(errno, "%s", _("saveThread: client thread creation failed")); + goto cleanup; + } + } + + args->total = waitClientThreads(args->conn, args->nchannels + 1); + + cleanup: + for (i = 0; i < args->nchannels + 1; i++) { + multiFdConnData *c = &args->conn[i]; + VIR_FORCE_CLOSE(c->clientfd); + } + if (VIR_CLOSE(sockfd) < 0) + args->total = -1; +} + +static const char *program_name; + +G_GNUC_NORETURN static void +usage(int status) +{ + if (status) { + fprintf(stderr, _("%s: try --help for more details"), program_name); + } else { + fprintf(stderr, _("Usage: %s UNIX_SOCNAME N MAINFD FD0 FD1 ... FDn"), program_name); + } + exit(status); +} + +int +main(int argc, char **argv) +{ + virThread tid; + virThreadFunc func = saveThreadFunc; + multiFdThreadArgs args = { 0 }; + int i; + + sleep(10); + + program_name = argv[0]; + + if (virGettextInitialize() < 0 || + virErrorInitialize() < 0) { + fprintf(stderr, _("%s: initialization failed"), program_name); + exit(EXIT_FAILURE); + } + + if (argc > 1 && STREQ(argv[1], "--help")) + usage(EXIT_SUCCESS); + if (argc < 4) + usage(EXIT_FAILURE); + + args.sun_path = argv[1]; + if (virStrToLong_i(argv[2], NULL, 10, &args.nchannels) < 0) + fprintf(stderr, _("%s: malformed number of channels N %s"), program_name, argv[2]); + + if (argc < 4 + args.nchannels) + usage(EXIT_FAILURE); + + args.conn = g_new0(multiFdConnData, args.nchannels + 1); + + for (i = 3; i < 3 + args.nchannels + 1; i++) { + multiFdConnData *c = &args.conn[i - 3]; + + if (virStrToLong_i(argv[i], NULL, 10, &c->filefd) < 0) { + fprintf(stderr, _("%s: malformed FD %s"), program_name, argv[i]); + usage(EXIT_FAILURE); + } +#ifndef F_GETFL +#error "multifd-helper requires F_GETFL parameter of fcntl" +#endif + c->oflags = fcntl(c->filefd, F_GETFL); + if ((c->oflags & O_ACCMODE) == O_RDONLY) { + func = loadThreadFunc; + } + } + + /* initialize server address structure */ + memset(&args.serv_addr, 0, sizeof(args.serv_addr)); + args.serv_addr.sun_family = AF_UNIX; + strncpy(args.serv_addr.sun_path, args.sun_path, sizeof(args.serv_addr.sun_path) - 1); + + if (virThreadCreate(&tid, true, func, &args) < 0) { + virReportSystemError(errno, _("%s: failed to create server thread"), program_name); + exit(EXIT_FAILURE); + } + + if (virThreadJoinRet(&tid) < 0) + exit(EXIT_FAILURE); + + if (args.total < 0) + exit(EXIT_FAILURE); + + exit(EXIT_SUCCESS); +} diff --git a/src/util/virthread.c b/src/util/virthread.c index 5422bb74fd..0f6c6a68fa 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -348,6 +348,11 @@ void virThreadJoin(virThread *thread) pthread_join(thread->thread, NULL); } +int virThreadJoinRet(virThread *thread) +{ + return pthread_join(thread->thread, NULL); +} + void virThreadCancel(virThread *thread) { pthread_cancel(thread->thread); diff --git a/src/util/virthread.h b/src/util/virthread.h index 23abe0b6c9..5cecb9bd8a 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -89,6 +89,7 @@ int virThreadCreateFull(virThread *thread, void virThreadSelf(virThread *thread); bool virThreadIsSelf(virThread *thread); void virThreadJoin(virThread *thread); +int virThreadJoinRet(virThread *thread); size_t virThreadMaxName(void); -- 2.34.1

On 4/26/22 6:47 PM, Claudio Fontana wrote:
For the save direction, this helper listens on a unix socket which QEMU connects to for multifd migration to files.
For the restore direction, this helper connects to a unix socket QEMU listens at for multifd migration from files.
The file descriptors are passed as command line parameters.
Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/libvirt_private.syms | 1 + src/util/meson.build | 13 ++ src/util/multifd-helper.c | 250 ++++++++++++++++++++++++++++++++++++++ src/util/virthread.c | 5 + src/util/virthread.h | 1 + 5 files changed, 270 insertions(+) create mode 100644 src/util/multifd-helper.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 97bfca906b..5f2bee985e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3427,6 +3427,7 @@ virThreadCreateFull; virThreadID; virThreadIsSelf; virThreadJoin; +virThreadJoinRet; virThreadMaxName; virThreadSelf; virThreadSelfID; diff --git a/src/util/meson.build b/src/util/meson.build index 58001a1699..8ea74ff9e8 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -179,6 +179,12 @@ io_helper_sources = [ 'runio.h', ]
+multifd_helper_sources = [ + 'multifd-helper.c', + 'runio.c', + 'runio.h', +] + virt_util_lib = static_library( 'virt_util', [ @@ -216,6 +222,13 @@ if conf.has('WITH_LIBVIRTD') dtrace_gen_headers, ], } + virt_helpers += { + 'name': 'libvirt_multifd_helper', + 'sources': [ + files(multifd_helper_sources), + dtrace_gen_headers, + ], + } endif
util_inc_dir = include_directories('.') diff --git a/src/util/multifd-helper.c b/src/util/multifd-helper.c new file mode 100644 index 0000000000..37e61a3a4d --- /dev/null +++ b/src/util/multifd-helper.c @@ -0,0 +1,250 @@ +/* + * multifd-helper.c: listens on Unix socket to perform I/O to multiple files + * + * Copyright (C) 2022 SUSE LLC + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * This has been written to support QEMU multifd migration to file, + * allowing better use of cpu resources to speed up the save/restore. + */ + +#include <config.h> + +#include <unistd.h> +#include <fcntl.h> +#include <stdlib.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/socket.h> +#include <sys/un.h> + +#include "virthread.h" +#include "virfile.h" +#include "virerror.h" +#include "virstring.h" +#include "virgettext.h" +#include "runio.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +typedef struct _multiFdConnData multiFdConnData; +struct _multiFdConnData { + int clientfd; + int filefd; + int oflags; + const char *path; + virThread tid; + + off_t total; +}; + +typedef struct _multiFdThreadArgs multiFdThreadArgs; +struct _multiFdThreadArgs { + int nchannels; + multiFdConnData *conn; /* contains main fd + nchannels */ + const char *sun_path; /* unix socket name to use for the server */ + struct sockaddr_un serv_addr; + + off_t total; +}; + +static void clientThreadFunc(void *a) +{ + multiFdConnData *c = a; + c->total = runIO(c->path, c->filefd, c->oflags, c->clientfd, c->clientfd); +} + +static off_t waitClientThreads(multiFdConnData *conn, int n) +{ + int i; + off_t total = 0; + for (i = 0; i < n; i++) { + multiFdConnData *c = &conn[i]; + if (virThreadJoinRet(&c->tid) < 0) { + total = -1; + } else if (total >= 0) { + total += c->total; + } + if (VIR_CLOSE(c->clientfd) < 0) { + total = -1; + } + } + return total; +} + +static void loadThreadFunc(void *a) +{ + multiFdThreadArgs *args = a; + int i; + args->total = -1; + + for (i = 0; i < args->nchannels + 1; i++) { + /* Perform outgoing connections */ + multiFdConnData *c = &args->conn[i]; + c->clientfd = socket(AF_UNIX, SOCK_STREAM, 0); + if (c->clientfd < 0) { + virReportSystemError(errno, "%s", _("loadThread: socket() failed")); + goto cleanup; + } + if (connect(c->clientfd, (const struct sockaddr *)&args->serv_addr, + sizeof(struct sockaddr_un)) < 0) { + virReportSystemError(errno, "%s", _("loadThread: connect() failed")); + goto cleanup; + } + if (virThreadCreate(&c->tid, true, &clientThreadFunc, c) < 0) { + virReportSystemError(errno, "%s", _("loadThread: client thread creation failed")); + goto cleanup; + } + } + args->total = waitClientThreads(args->conn, args->nchannels + 1); + + cleanup: + for (i = 0; i < args->nchannels + 1; i++) { + multiFdConnData *c = &args->conn[i]; + VIR_FORCE_CLOSE(c->clientfd); + } +} + +static void saveThreadFunc(void *a) +{ + multiFdThreadArgs *args = a; + int i; + const char buf[1] = {'R'}; + int sockfd; + + if ((sockfd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + virReportSystemError(errno, "%s", _("saveThread: socket() failed")); + return; + } + unlink(args->sun_path); + if (bind(sockfd, (struct sockaddr *)&args->serv_addr, sizeof(args->serv_addr)) < 0) { + virReportSystemError(errno, "%s", _("saveThread: bind() failed")); + goto cleanup; + } + if (listen(sockfd, args->nchannels + 1) < 0) { + virReportSystemError(errno, "%s", _("saveThread: listen() failed")); + goto cleanup; + } + + /* signal that the server is ready */ + if (safewrite(STDOUT_FILENO, &buf, 1) != 1) { + virReportSystemError(errno, "%s", _("saveThread: safewrite failed")); + goto cleanup; + } + + for (i = 0; i < args->nchannels + 1; i++) { + /* Wait for incoming connection. */ + multiFdConnData *c = &args->conn[i]; + if ((c->clientfd = accept(sockfd, NULL, NULL)) < 0) { + virReportSystemError(errno, "%s", _("saveThread: accept() failed")); + goto cleanup; + } + if (virThreadCreate(&c->tid, true, &clientThreadFunc, c) < 0) { + virReportSystemError(errno, "%s", _("saveThread: client thread creation failed")); + goto cleanup; + } + } + + args->total = waitClientThreads(args->conn, args->nchannels + 1); + + cleanup: + for (i = 0; i < args->nchannels + 1; i++) { + multiFdConnData *c = &args->conn[i]; + VIR_FORCE_CLOSE(c->clientfd); + } + if (VIR_CLOSE(sockfd) < 0) + args->total = -1; +} + +static const char *program_name; + +G_GNUC_NORETURN static void +usage(int status) +{ + if (status) { + fprintf(stderr, _("%s: try --help for more details"), program_name); + } else { + fprintf(stderr, _("Usage: %s UNIX_SOCNAME N MAINFD FD0 FD1 ... FDn"), program_name); + } + exit(status); +} + +int +main(int argc, char **argv) +{ + virThread tid; + virThreadFunc func = saveThreadFunc; + multiFdThreadArgs args = { 0 }; + int i; + + sleep(10);
This of course should not be there, something handy during debugging only.
+ + program_name = argv[0]; + + if (virGettextInitialize() < 0 || + virErrorInitialize() < 0) { + fprintf(stderr, _("%s: initialization failed"), program_name); + exit(EXIT_FAILURE); + } + + if (argc > 1 && STREQ(argv[1], "--help")) + usage(EXIT_SUCCESS); + if (argc < 4) + usage(EXIT_FAILURE); + + args.sun_path = argv[1]; + if (virStrToLong_i(argv[2], NULL, 10, &args.nchannels) < 0) + fprintf(stderr, _("%s: malformed number of channels N %s"), program_name, argv[2]); + + if (argc < 4 + args.nchannels) + usage(EXIT_FAILURE); + + args.conn = g_new0(multiFdConnData, args.nchannels + 1); + + for (i = 3; i < 3 + args.nchannels + 1; i++) { + multiFdConnData *c = &args.conn[i - 3]; + + if (virStrToLong_i(argv[i], NULL, 10, &c->filefd) < 0) { + fprintf(stderr, _("%s: malformed FD %s"), program_name, argv[i]); + usage(EXIT_FAILURE); + } +#ifndef F_GETFL +#error "multifd-helper requires F_GETFL parameter of fcntl" +#endif + c->oflags = fcntl(c->filefd, F_GETFL); + if ((c->oflags & O_ACCMODE) == O_RDONLY) { + func = loadThreadFunc; + } + } + + /* initialize server address structure */ + memset(&args.serv_addr, 0, sizeof(args.serv_addr)); + args.serv_addr.sun_family = AF_UNIX; + strncpy(args.serv_addr.sun_path, args.sun_path, sizeof(args.serv_addr.sun_path) - 1); + + if (virThreadCreate(&tid, true, func, &args) < 0) { + virReportSystemError(errno, _("%s: failed to create server thread"), program_name); + exit(EXIT_FAILURE); + } + + if (virThreadJoinRet(&tid) < 0) + exit(EXIT_FAILURE); + + if (args.total < 0) + exit(EXIT_FAILURE); + + exit(EXIT_SUCCESS); +} diff --git a/src/util/virthread.c b/src/util/virthread.c index 5422bb74fd..0f6c6a68fa 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -348,6 +348,11 @@ void virThreadJoin(virThread *thread) pthread_join(thread->thread, NULL); }
+int virThreadJoinRet(virThread *thread) +{ + return pthread_join(thread->thread, NULL); +} + void virThreadCancel(virThread *thread) { pthread_cancel(thread->thread); diff --git a/src/util/virthread.h b/src/util/virthread.h index 23abe0b6c9..5cecb9bd8a 100644 --- a/src/util/virthread.h +++ b/src/util/virthread.h @@ -89,6 +89,7 @@ int virThreadCreateFull(virThread *thread, void virThreadSelf(virThread *thread); bool virThreadIsSelf(virThread *thread); void virThreadJoin(virThread *thread); +int virThreadJoinRet(virThread *thread);
size_t virThreadMaxName(void);

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_saveimage.c | 130 ++++++++++++++++++++++++++++++++++++-- src/qemu/qemu_saveimage.h | 11 ++++ 2 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 6af256aabe..fbeb355272 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -17,6 +17,7 @@ */ #include <config.h> +#include <configmake.h> #include "qemu_saveimage.h" #include "qemu_domain.h" @@ -365,6 +366,93 @@ int virQEMUSaveFdFini(virQEMUSaveFd *saveFd, virDomainObj *vm, int ret) return ret; } +/* + * qemuSaveImageFreeMultiFd: free all multifd virQEMUSaveFds. + * @multiFd: the array of saveFds + * @vm: the virDomainObj, to release lock + * @nconn: number of multifd channels + * @ret: the current operation result (< 0 is failure) + * + * If multiFd is NULL, the return value will be unchanged. + * + * Returns ret, or -1 if an error is detected. + */ +int qemuSaveImageFreeMultiFd(virQEMUSaveFd *multiFd, virDomainObj *vm, int nconn, int ret) +{ + int i; + + if (!multiFd) + return ret; + + for (i = 0; i < nconn; i++) { + ret = virQEMUSaveFdFini(&multiFd[i], vm, ret); + } + /* + * do it again to unlink all in the error case, + * if error happened in the middle of previous loop. + */ + for (i = 0; i < nconn; i++) { + ret = virQEMUSaveFdFini(&multiFd[i], vm, ret); + } + g_free(multiFd); + return ret; +} + +/* + * qemuSaveImageCloseMultiFd: perform normal close on all multifd virQEMUSaveFds. + * If multiFd is NULL, the function will return success. + * + * Returns -1 on error, 0 on success. + */ + +int qemuSaveImageCloseMultiFd(virQEMUSaveFd *multiFd, int nconn, virDomainObj *vm) +{ + int i; + + if (!multiFd) + return 0; + + for (i = 0; i < nconn; i++) { + if (virQEMUSaveFdClose(&multiFd[i], vm) < 0) { + return -1; + } + } + return 0; +} + +/* + * qemuSaveImageCreateMultiFd: allocate and initialize all multifd virQEMUSaveFds. + * + * Returns the new array of virQEMUSaveFds, or NULL on error. + */ + +virQEMUSaveFd * +qemuSaveImageCreateMultiFd(virQEMUDriver *driver, virDomainObj *vm, + virCommand *cmd, const char *path, + int oflags, virQEMUDriverConfig *cfg, + int nconn) +{ + virQEMUSaveFd *multiFd = g_new0(virQEMUSaveFd, nconn); + int i; + + for (i = 0; i < nconn; i++) { + virQEMUSaveFd *m = &multiFd[i]; + if (virQEMUSaveFdInit(m, path, i + 1, oflags, cfg) < 0 || + qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, m->fd) < 0) { + + virQEMUSaveFdFini(m, vm, -1); + goto error; + } + virCommandAddArgFormat(cmd, "%d", m->fd); + virCommandPassFD(cmd, m->fd, 0); + } + return multiFd; + + error: + qemuSaveImageFreeMultiFd(multiFd, vm, nconn, -1); + return NULL; +} + /* 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 @@ -381,6 +469,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virQEMUSaveFd saveFd = QEMU_SAVEFD_INVALID; + virQEMUSaveFd *multiFd = NULL; unsigned int oflags = O_WRONLY | O_TRUNC | O_CREAT; int ret = -1; nconn = nconn; /* unused */ @@ -403,10 +492,43 @@ qemuSaveImageCreate(virQEMUDriver *driver, if (virQEMUSaveDataWrite(data, saveFd.fd, saveFd.path) < 0) goto cleanup; + if (flags & VIR_DOMAIN_SAVE_PARALLEL) { + g_autoptr(virCommand) cmd = NULL; + g_autofree char *helper_path = NULL; + qemuDomainObjPrivate *priv = vm->privateData; + g_autofree char *sun_path = g_strdup_printf("%s/save-multifd.sock", priv->libDir); + char buf[1]; + int helper_out = -1; + if (!(helper_path = virFileFindResource("libvirt_multifd_helper", + abs_top_builddir "/src", + LIBEXECDIR))) + goto cleanup; + cmd = virCommandNewArgList(helper_path, sun_path, NULL); + virCommandAddArgFormat(cmd, "%d", nconn); + virCommandAddArgFormat(cmd, "%d", saveFd.fd); + virCommandPassFD(cmd, saveFd.fd, 0); + virCommandSetOutputFD(cmd, &helper_out); /* should create pipe automagically */ + + /* Perform parallel multifd migration to files (main fd + channels) */ + if (!(multiFd = qemuSaveImageCreateMultiFd(driver, vm, cmd, saveFd.path, oflags, cfg, nconn))) + goto cleanup; + if (virCommandRunAsync(cmd, NULL) < 0) + goto cleanup; + if (saferead(helper_out, &buf, 1) != 1 || buf[0] != 'R') + goto cleanup; + if (chown(sun_path, cfg->user, cfg->group) < 0) + goto cleanup; + /* still using single fd migration for now */ + if (qemuMigrationSrcToFile(driver, vm, saveFd.fd, compressor, asyncJob) < 0) + goto cleanup; + if (qemuSaveImageCloseMultiFd(multiFd, nconn, vm) < 0) + goto cleanup; + } else { + /* Perform non-parallel migration to file */ + if (qemuMigrationSrcToFile(driver, vm, saveFd.fd, compressor, asyncJob) < 0) + goto cleanup; + } - /* Perform the migration */ - if (qemuMigrationSrcToFile(driver, vm, saveFd.fd, compressor, asyncJob) < 0) - goto cleanup; if (virQEMUSaveFdClose(&saveFd, vm) < 0) goto cleanup; @@ -425,7 +547,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, cleanup: - + ret = qemuSaveImageFreeMultiFd(multiFd, vm, nconn, ret); ret = virQEMUSaveFdFini(&saveFd, vm, ret); return ret; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 5dc63f3661..b775c5eb08 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -72,6 +72,17 @@ int virQEMUSaveFdClose(virQEMUSaveFd *saveFd, virDomainObj *vm); int virQEMUSaveFdFini(virQEMUSaveFd *saveFd, virDomainObj *vm, int ret); +virQEMUSaveFd * +qemuSaveImageCreateMultiFd(virQEMUDriver *driver, virDomainObj *vm, + virCommand *cmd, const char *path, + int oflags, virQEMUDriverConfig *cfg, + int nconn) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(6); + +int qemuSaveImageCloseMultiFd(virQEMUSaveFd *multiFd, int nconn, virDomainObj *vm); + +int qemuSaveImageFreeMultiFd(virQEMUSaveFd *multiFd, virDomainObj *vm, int nconn, int ret); + virDomainDef * qemuSaveImageUpdateDef(virQEMUDriver *driver, virDomainDef *def, -- 2.34.1

implement a function similar to qemuMigrationSrcToFile that migrates to multiple files using QEMU multifd. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_migration.c | 129 ++++++++++++++++++++----------- src/qemu/qemu_migration.h | 7 ++ src/qemu/qemu_migration_params.c | 22 ++++++ src/qemu/qemu_migration_params.h | 9 +++ src/qemu/qemu_saveimage.c | 3 +- 6 files changed, 124 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b91db851bb..c5afe1439e 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1230,6 +1230,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { { "rdma-pin-all", QEMU_CAPS_MIGRATE_RDMA }, + { "multifd", QEMU_MIGRATION_CAP_MULTIFD }, }; /* Use virQEMUCapsQMPSchemaQueries for querying parameters of events */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b735bdb391..b73cfcedc5 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5896,13 +5896,14 @@ qemuMigrationDstFinish(virQEMUDriver *driver, return dom; } - /* Helper function called while vm is active. */ -int -qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, - int fd, - virCommand *compressor, - virDomainAsyncJob asyncJob) +static int +qemuMigrationSrcToFileAux(virQEMUDriver *driver, virDomainObj *vm, + int fd, + virCommand *compressor, + virDomainAsyncJob asyncJob, + const char *sun_path, + int nchannels) { qemuDomainObjPrivate *priv = vm->privateData; bool bwParam = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH); @@ -5913,24 +5914,24 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, char *errbuf = NULL; virErrorPtr orig_err = NULL; g_autoptr(qemuMigrationParams) migParams = NULL; + bool needParams = (bwParam || sun_path); if (qemuMigrationSetDBusVMState(driver, vm) < 0) return -1; + if (sun_path && !virQEMUCapsGet(priv->qemuCaps, QEMU_MIGRATION_CAP_MULTIFD)) + return -1; + /* Increase migration bandwidth to unlimited since target is a file. * Failure to change migration speed is not fatal. */ - if (bwParam) { - if (!(migParams = qemuMigrationParamsNew())) - return -1; + if (needParams && !((migParams = qemuMigrationParamsNew()))) + return -1; + if (bwParam) { if (qemuMigrationParamsSetULL(migParams, QEMU_MIGRATION_PARAM_MAX_BANDWIDTH, QEMU_DOMAIN_MIG_BANDWIDTH_MAX * 1024 * 1024) < 0) return -1; - - if (qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0) - return -1; - priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; } else { if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { @@ -5941,6 +5942,17 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, } } + if (sun_path) { + qemuMigrationParamsSetCap(migParams, QEMU_MIGRATION_CAP_MULTIFD); + if (qemuMigrationParamsSetInt(migParams, + QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, + nchannels) < 0) + return -1; + } + + if (needParams && qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0) + return -1; + if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest unexpectedly quit")); @@ -5948,45 +5960,53 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, return -1; } - if (compressor && virPipe(pipeFD) < 0) + if (!sun_path && compressor && virPipe(pipeFD) < 0) return -1; - /* All right! We can use fd migration, which means that qemu - * doesn't have to open() the file, so while we still have to - * grant SELinux access, we can do it on fd and avoid cleanup - * later, as well as skip futzing with cgroup. */ - if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, - compressor ? pipeFD[1] : fd) < 0) - goto cleanup; - if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; - if (!compressor) { - rc = qemuMonitorMigrateToFd(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - fd); + if (sun_path) { + rc = qemuMonitorMigrateToSocket(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + sun_path); } else { - virCommandSetInputFD(compressor, pipeFD[0]); - virCommandSetOutputFD(compressor, &fd); - virCommandSetErrorBuffer(compressor, &errbuf); - virCommandDoAsyncIO(compressor); - if (virSetCloseExec(pipeFD[1]) < 0) { - virReportSystemError(errno, "%s", - _("Unable to set cloexec flag")); - qemuDomainObjExitMonitor(vm); - goto cleanup; - } - if (virCommandRunAsync(compressor, NULL) < 0) { - qemuDomainObjExitMonitor(vm); + /* + * All right! We can use fd migration, which means that qemu + * doesn't have to open() the file, so while we still have to + * grant SELinux access, we can do it on fd and avoid cleanup + * later, as well as skip futzing with cgroup. + */ + if (qemuSecuritySetImageFDLabel(driver->securityManager, vm->def, + compressor ? pipeFD[1] : fd) < 0) goto cleanup; + + if (!compressor) { + rc = qemuMonitorMigrateToFd(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + fd); + } else { + virCommandSetInputFD(compressor, pipeFD[0]); + virCommandSetOutputFD(compressor, &fd); + virCommandSetErrorBuffer(compressor, &errbuf); + virCommandDoAsyncIO(compressor); + if (virSetCloseExec(pipeFD[1]) < 0) { + virReportSystemError(errno, "%s", + _("Unable to set cloexec flag")); + qemuDomainObjExitMonitor(vm); + goto cleanup; + } + if (virCommandRunAsync(compressor, NULL) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } + rc = qemuMonitorMigrateToFd(priv->mon, + QEMU_MONITOR_MIGRATE_BACKGROUND, + pipeFD[1]); + if (VIR_CLOSE(pipeFD[0]) < 0 || + VIR_CLOSE(pipeFD[1]) < 0) + VIR_WARN("failed to close intermediate pipe"); } - rc = qemuMonitorMigrateToFd(priv->mon, - QEMU_MONITOR_MIGRATE_BACKGROUND, - pipeFD[1]); - if (VIR_CLOSE(pipeFD[0]) < 0 || - VIR_CLOSE(pipeFD[1]) < 0) - VIR_WARN("failed to close intermediate pipe"); } qemuDomainObjExitMonitor(vm); if (rc < 0) @@ -6007,7 +6027,7 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, goto cleanup; } - if (compressor && virCommandWait(compressor, NULL) < 0) + if (!sun_path && compressor && virCommandWait(compressor, NULL) < 0) goto cleanup; qemuDomainEventEmitJobCompleted(driver, vm); @@ -6046,6 +6066,25 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, return ret; } +int +qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainObj *vm, + int fd, + virCommand *compressor, + virDomainAsyncJob asyncJob) +{ + return qemuMigrationSrcToFileAux(driver, vm, fd, compressor, + asyncJob, NULL, -1); +} + +int +qemuMigrationSrcToFilesMultiFd(virQEMUDriver *driver, virDomainObj *vm, + virDomainAsyncJob asyncJob, + const char *sun_path, + int nchannels) +{ + return qemuMigrationSrcToFileAux(driver, vm, -1, NULL, + asyncJob, sun_path, nchannels); +} int qemuMigrationSrcCancel(virQEMUDriver *driver, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index a8afa66119..ddc8e65489 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -213,6 +213,13 @@ qemuMigrationSrcToFile(virQEMUDriver *driver, virDomainAsyncJob asyncJob) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +int +qemuMigrationSrcToFilesMultiFd(virQEMUDriver *driver, virDomainObj *vm, + virDomainAsyncJob asyncJob, + const char *sun_path, + int nchannels) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + int qemuMigrationSrcCancel(virQEMUDriver *driver, virDomainObj *vm); diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index df2384b213..36174a66d8 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -1109,6 +1109,28 @@ qemuMigrationParamsFetch(virQEMUDriver *driver, } +void +qemuMigrationParamsSetCap(qemuMigrationParams *migParams, + virQEMUCapsFlags flag) +{ + ignore_value(virBitmapSetBit(migParams->caps, flag)); +} + + +int +qemuMigrationParamsSetInt(qemuMigrationParams *migParams, + qemuMigrationParam param, + int value) +{ + if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_INT) < 0) + return -1; + + migParams->params[param].value.i = value; + migParams->params[param].set = true; + return 0; +} + + int qemuMigrationParamsSetULL(qemuMigrationParams *migParams, qemuMigrationParam param, diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 4a8815e776..99af73b4a4 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -123,6 +123,15 @@ qemuMigrationParamsFetch(virQEMUDriver *driver, int asyncJob, qemuMigrationParams **migParams); +void +qemuMigrationParamsSetCap(qemuMigrationParams *migParams, + virQEMUCapsFlags flag); + +int +qemuMigrationParamsSetInt(qemuMigrationParams *migParams, + qemuMigrationParam param, + int value); + int qemuMigrationParamsSetULL(qemuMigrationParams *migParams, qemuMigrationParam param, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index fbeb355272..7d59e0ea36 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -518,8 +518,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, goto cleanup; if (chown(sun_path, cfg->user, cfg->group) < 0) goto cleanup; - /* still using single fd migration for now */ - if (qemuMigrationSrcToFile(driver, vm, saveFd.fd, compressor, asyncJob) < 0) + if (qemuMigrationSrcToFilesMultiFd(driver, vm, asyncJob, sun_path, nconn) < 0) goto cleanup; if (qemuSaveImageCloseMultiFd(multiFd, nconn, vm) < 0) goto cleanup; -- 2.34.1

The distinction on whether to wait for the migration completion or not was made on the async job type, but with the future addition of multifd migration from files, we need a way to avoid waiting, so we can prepare multifd migration parameters before starting the transfers. Adapt all callers. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_driver.c | 8 ++++---- src/qemu/qemu_migration.c | 18 ++++++++++-------- src/qemu/qemu_migration.h | 3 ++- src/qemu/qemu_process.c | 3 ++- src/qemu/qemu_process.h | 5 +++-- src/qemu/qemu_saveimage.c | 4 +++- src/qemu/qemu_saveimage.h | 1 + src/qemu/qemu_snapshot.c | 4 ++-- 8 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 914be2e703..8c1c05c904 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1630,7 +1630,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, } if (qemuProcessStart(conn, driver, vm, NULL, VIR_ASYNC_JOB_START, - NULL, -1, NULL, NULL, + NULL, -1, NULL, false, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); @@ -5907,7 +5907,7 @@ qemuDomainRestoreInternal(virConnectPtr conn, goto cleanup; ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path, - false, reset_nvram, VIR_ASYNC_JOB_START); + false, reset_nvram, true, VIR_ASYNC_JOB_START); qemuProcessEndJob(vm); @@ -6225,7 +6225,7 @@ qemuDomainObjRestore(virConnectPtr conn, virDomainObjAssignDef(vm, &def, true, NULL); ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path, - start_paused, reset_nvram, asyncJob); + start_paused, reset_nvram, true, asyncJob); cleanup: virQEMUSaveDataFree(data); @@ -6488,7 +6488,7 @@ qemuDomainObjStart(virConnectPtr conn, } ret = qemuProcessStart(conn, driver, vm, NULL, asyncJob, - NULL, -1, NULL, NULL, + NULL, -1, NULL, false, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "booted", ret >= 0); if (ret >= 0) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index b73cfcedc5..2ea3669de1 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2139,7 +2139,8 @@ int qemuMigrationDstRun(virQEMUDriver *driver, virDomainObj *vm, const char *uri, - virDomainAsyncJob asyncJob) + virDomainAsyncJob asyncJob, + bool wait) { qemuDomainObjPrivate *priv = vm->privateData; int rv; @@ -2160,14 +2161,15 @@ qemuMigrationDstRun(virQEMUDriver *driver, if (rv < 0) return -1; - if (asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) { - /* qemuMigrationDstWaitForCompletion is called from the Finish phase */ - return 0; + if (wait) { + /* + * the Migration Finish phase, as well as the multifd load from files, + * need to call qemuMigrationDstWaitForCompletion separately, not here. + */ + if (qemuMigrationDstWaitForCompletion(driver, vm, asyncJob, false) < 0) + return -1; } - if (qemuMigrationDstWaitForCompletion(driver, vm, asyncJob, false) < 0) - return -1; - return 0; } @@ -3041,7 +3043,7 @@ qemuMigrationDstPrepareAny(virQEMUDriver *driver, } if (qemuMigrationDstRun(driver, vm, incoming->uri, - VIR_ASYNC_JOB_MIGRATION_IN) < 0) + VIR_ASYNC_JOB_MIGRATION_IN, false) < 0) goto stopjob; if (qemuProcessFinishStartup(driver, vm, VIR_ASYNC_JOB_MIGRATION_IN, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index ddc8e65489..c3c48c19c0 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -255,7 +255,8 @@ int qemuMigrationDstRun(virQEMUDriver *driver, virDomainObj *vm, const char *uri, - virDomainAsyncJob asyncJob); + virDomainAsyncJob asyncJob, + bool wait); void qemuMigrationAnyPostcopyFailed(virQEMUDriver *driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b0b00eb0a2..0a1e7985fb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7788,6 +7788,7 @@ qemuProcessStart(virConnectPtr conn, const char *migrateFrom, int migrateFd, const char *migratePath, + bool wait_incoming, virDomainMomentObj *snapshot, virNetDevVPortProfileOp vmop, unsigned int flags) @@ -7850,7 +7851,7 @@ qemuProcessStart(virConnectPtr conn, relabel = true; if (incoming) { - if (qemuMigrationDstRun(driver, vm, incoming->uri, asyncJob) < 0) + if (qemuMigrationDstRun(driver, vm, incoming->uri, asyncJob, wait_incoming) < 0) goto stop; } else { /* Refresh state of devices from QEMU. During migration this happens diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index f81bfd930a..5a1d005cb0 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -86,8 +86,9 @@ int qemuProcessStart(virConnectPtr conn, virCPUDef *updatedCPU, virDomainAsyncJob asyncJob, const char *migrateFrom, - int stdin_fd, - const char *stdin_path, + int fd, + const char *migratePath, + bool wait_incoming, virDomainMomentObj *snapshot, virNetDevVPortProfileOp vmop, unsigned int flags); diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 7d59e0ea36..98ce99a8b5 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -750,6 +750,7 @@ qemuSaveImageStartVM(virConnectPtr conn, const char *path, bool start_paused, bool reset_nvram, + bool wait_incoming, virDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; @@ -804,7 +805,8 @@ qemuSaveImageStartVM(virConnectPtr conn, priv->disableSlirp = true; if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, - asyncJob, "stdio", *fd, path, NULL, + asyncJob, "stdio", *fd, path, wait_incoming, + NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, start_flags) == 0) started = true; diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index b775c5eb08..7be0892dde 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -97,6 +97,7 @@ qemuSaveImageStartVM(virConnectPtr conn, const char *path, bool start_paused, bool reset_nvram, + bool wait_incoming, virDomainAsyncJob asyncJob) ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 2e445e8296..626a5a14b9 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2092,7 +2092,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, rc = qemuProcessStart(snapshot->domain->conn, driver, vm, cookie ? cookie->cpu : NULL, - VIR_ASYNC_JOB_START, NULL, -1, NULL, snap, + VIR_ASYNC_JOB_START, NULL, -1, NULL, false, snap, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); @@ -2215,7 +2215,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, start_flags |= paused ? VIR_QEMU_PROCESS_START_PAUSED : 0; rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL, - VIR_ASYNC_JOB_START, NULL, -1, NULL, NULL, + VIR_ASYNC_JOB_START, NULL, -1, NULL, false, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); -- 2.34.1

use multifd to restore parallel saves. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_driver.c | 10 ++++- src/qemu/qemu_migration.c | 2 +- src/qemu/qemu_migration.h | 6 +++ src/qemu/qemu_saveimage.c | 94 ++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_saveimage.h | 9 +++- 5 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8c1c05c904..73b13e40e1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5906,8 +5906,14 @@ qemuDomainRestoreInternal(virConnectPtr conn, flags) < 0) goto cleanup; - ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path, - false, reset_nvram, true, VIR_ASYNC_JOB_START); + if (flags & VIR_DOMAIN_SAVE_PARALLEL) { + ret = qemuSaveImageLoadMultiFd(conn, vm, oflags, data, reset_nvram, + &saveFd, nchannels, VIR_ASYNC_JOB_START); + + } else { + ret = qemuSaveImageStartVM(conn, driver, vm, &saveFd.fd, data, path, + false, reset_nvram, true, VIR_ASYNC_JOB_START); + } qemuProcessEndJob(vm); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 2ea3669de1..7aec2dc377 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1933,7 +1933,7 @@ qemuMigrationSrcWaitForCompletion(virQEMUDriver *driver, } -static int +int qemuMigrationDstWaitForCompletion(virQEMUDriver *driver, virDomainObj *vm, virDomainAsyncJob asyncJob, diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index c3c48c19c0..38f4877cf0 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -191,6 +191,12 @@ qemuMigrationDstFinish(virQEMUDriver *driver, int retcode, bool v3proto); +int +qemuMigrationDstWaitForCompletion(virQEMUDriver *driver, + virDomainObj *vm, + virDomainAsyncJob asyncJob, + bool postcopy); + int qemuMigrationSrcConfirm(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 98ce99a8b5..ecdab98168 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -552,6 +552,89 @@ qemuSaveImageCreate(virQEMUDriver *driver, } +int qemuSaveImageLoadMultiFd(virConnectPtr conn, virDomainObj *vm, int oflags, + virQEMUSaveData *data, bool reset_nvram, + virQEMUSaveFd *saveFd, int nchannels, + virDomainAsyncJob asyncJob) +{ + virQEMUDriver *driver = conn->privateData; + qemuDomainObjPrivate *priv = vm->privateData; + virQEMUSaveFd *multiFd = NULL; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virCommand) cmd = NULL; + g_autofree char *helper_path = NULL; + g_autofree char *sun_path = g_strdup_printf("%s/restore-multifd.sock", cfg->saveDir); + int ret = -1; + + if (!(helper_path = virFileFindResource("libvirt_multifd_helper", + abs_top_builddir "/src", + LIBEXECDIR))) + goto cleanup; + cmd = virCommandNewArgList(helper_path, sun_path, NULL); + virCommandAddArgFormat(cmd, "%d", nchannels); + virCommandAddArgFormat(cmd, "%d", saveFd->fd); + virCommandPassFD(cmd, saveFd->fd, 0); + + /* Perform parallel multifd migration from files (main fd + channels) */ + if (!(multiFd = qemuSaveImageCreateMultiFd(driver, vm, cmd, saveFd->path, oflags, cfg, nchannels))) + goto cleanup; + if (qemuSaveImageStartVM(conn, driver, vm, NULL, data, sun_path, + false, reset_nvram, false, asyncJob) < 0) + goto cleanup; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_MIGRATION_CAP_MULTIFD)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("QEMU multifd not supported")); + goto cleanup; + } else { + g_autoptr(qemuMigrationParams) migParams = qemuMigrationParamsNew(); + bool bwParam = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_BANDWIDTH); + + if (bwParam) { + if (qemuMigrationParamsSetULL(migParams, + QEMU_MIGRATION_PARAM_MAX_BANDWIDTH, + QEMU_DOMAIN_MIG_BANDWIDTH_MAX * 1024 * 1024) < 0) + goto cleanup; + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; + } else { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { + qemuMonitorSetMigrationSpeed(priv->mon, + QEMU_DOMAIN_MIG_BANDWIDTH_MAX); + priv->migMaxBandwidth = QEMU_DOMAIN_MIG_BANDWIDTH_MAX; + qemuDomainObjExitMonitor(vm); + } + } + qemuMigrationParamsSetCap(migParams, QEMU_MIGRATION_CAP_MULTIFD); + if (qemuMigrationParamsSetInt(migParams, + QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, + nchannels) < 0) + goto cleanup; + if (qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); + goto cleanup; + } + /* multifd helper can now connect, then wait for migration to complete */ + if (virCommandRunAsync(cmd, NULL) < 0) + goto cleanup; + + if (qemuMigrationDstWaitForCompletion(driver, vm, asyncJob, false) < 0) + goto cleanup; + + if (qemuSaveImageCloseMultiFd(multiFd, nchannels, vm) < 0) + goto cleanup; + } + qemuDomainEventEmitJobCompleted(driver, vm); + ret = 0; + + cleanup: + ret = qemuSaveImageFreeMultiFd(multiFd, vm, nchannels, ret); + return ret; +} + + /* qemuSaveImageGetCompressionProgram: * @imageFormat: String representation from qemu.conf for the compression * image format being used (dump, save, or snapshot). @@ -758,6 +841,7 @@ qemuSaveImageStartVM(virConnectPtr conn, bool started = false; virObjectEvent *event; VIR_AUTOCLOSE intermediatefd = -1; + g_autofree char *migrate_from = NULL; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); @@ -804,8 +888,14 @@ qemuSaveImageStartVM(virConnectPtr conn, if (cookie && !cookie->slirpHelper) priv->disableSlirp = true; + if (fd) { + migrate_from = g_strdup("stdio"); + } else { + migrate_from = g_strdup_printf("unix://%s", path); + } + if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, - asyncJob, "stdio", *fd, path, wait_incoming, + asyncJob, migrate_from, fd ? *fd : -1, path, wait_incoming, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, start_flags) == 0) @@ -829,7 +919,7 @@ qemuSaveImageStartVM(virConnectPtr conn, VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); virErrorRestore(&orig_err); } - if (VIR_CLOSE(*fd) < 0) { + if (fd && VIR_CLOSE(*fd) < 0) { virReportSystemError(errno, _("cannot close file: %s"), path); rc = -1; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 7be0892dde..ae7b3faa17 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -99,7 +99,7 @@ qemuSaveImageStartVM(virConnectPtr conn, bool reset_nvram, bool wait_incoming, virDomainAsyncJob asyncJob) - ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); + ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); int qemuSaveImageOpen(virQEMUDriver *driver, @@ -117,6 +117,13 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, bool use_raw_on_fail) ATTRIBUTE_NONNULL(2); +int qemuSaveImageLoadMultiFd(virConnectPtr conn, virDomainObj *vm, int oflags, + virQEMUSaveData *data, bool reset_nvram, + virQEMUSaveFd *saveFd, int nchannels, + virDomainAsyncJob asyncJob) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) + ATTRIBUTE_NONNULL(6) G_GNUC_WARN_UNUSED_RESULT; + int qemuSaveImageCreate(virQEMUDriver *driver, virDomainObj *vm, -- 2.34.1

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- docs/manpages/virsh.rst | 23 ++++++++++++++----- tools/virsh-domain.c | 49 +++++++++++++++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 15 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index e8568559fa..2bce701057 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3801,15 +3801,18 @@ save :: save domain state-file [--bypass-cache] [--xml file] + [--parallel] [--parallel-connections connections] [{--running | --paused}] [--verbose] -Saves a running domain (RAM, but not disk state) to a state file so that -it can be restored -later. Once saved, the domain will no longer be running on the -system, thus the memory allocated for the domain will be free for -other domains to use. ``virsh restore`` restores from this state file. +Saves a paused or running domain (RAM, but not disk state) to one or more +state files, so that it can be restored later. +Once saved, the domain will no longer be running on the system, +thus the memory allocated for the domain will be free for +other domains to use. ``virsh restore`` restores from state file/s. + If *--bypass-cache* is specified, the save will avoid the file system -cache, although this may slow down the operation. +cache; depending on the specific scenario this may slow down or speed up +the operation. The progress may be monitored using ``domjobinfo`` virsh command and canceled with ``domjobabort`` command (sent by another virsh instance). Another option @@ -3831,6 +3834,14 @@ based on the state the domain was in when the save was done; passing either the *--running* or *--paused* flag will allow overriding which state the ``restore`` should use. +*--parallel* option will cause the save data to be sent over multiple +parallel connections to multiple files. The main save file is specified +with ``state-file``, and a number of additional connections can be +set using *--parallel-connections*, which will save to files named +``state-file``.1 , ``state-file``.2 ... up to ``connections``. + +Parallel connections may help in speeding up the save operation. + Domain saved state files assume that disk images will be unchanged between the creation and restore point. For a more complete system restore point, where the disk state is saved alongside the memory diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index df8df9c2f3..1e3adaa1be 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4164,6 +4164,14 @@ static const vshCmdOptDef opts_save[] = { .type = VSH_OT_BOOL, .help = N_("avoid file system cache when saving") }, + {.name = "parallel", + .type = VSH_OT_BOOL, + .help = N_("enable parallel save to files") + }, + {.name = "parallel-connections", + .type = VSH_OT_INT, + .help = N_("number of connections/files for parallel save") + }, {.name = "xml", .type = VSH_OT_STRING, .completer = virshCompletePathLocalExisting, @@ -4193,6 +4201,11 @@ doSave(void *opaque) g_autoptr(virshDomain) dom = NULL; const char *name = NULL; const char *to = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + int maxparams = 0; + int intOpt = 0; + int rv = -1; unsigned int flags = 0; const char *xmlfile = NULL; g_autofree char *xml = NULL; @@ -4206,29 +4219,46 @@ doSave(void *opaque) goto out_sig; #endif /* !WIN32 */ - if (vshCommandOptStringReq(ctl, cmd, "file", &to) < 0) + if ((rv = vshCommandOptStringReq(ctl, cmd, "file", &to)) < 0) { goto out; - + } else { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_SAVE_PARAM_FILE, to) < 0) + goto out; + } if (vshCommandOptBool(cmd, "bypass-cache")) flags |= VIR_DOMAIN_SAVE_BYPASS_CACHE; + if (vshCommandOptBool(cmd, "parallel")) + flags |= VIR_DOMAIN_SAVE_PARALLEL; + if ((rv = vshCommandOptInt(ctl, cmd, "parallel-connections", &intOpt)) < 0) { + goto out; + } else if (rv > 0) { + if (virTypedParamsAddInt(¶ms, &nparams, &maxparams, + VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, intOpt) < 0) + goto out; + } if (vshCommandOptBool(cmd, "running")) flags |= VIR_DOMAIN_SAVE_RUNNING; if (vshCommandOptBool(cmd, "paused")) flags |= VIR_DOMAIN_SAVE_PAUSED; - if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) + if ((rv = vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile)) < 0) goto out; if (!(dom = virshCommandOptDomain(ctl, cmd, &name))) goto out; - if (xmlfile && - virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) { - vshReportError(ctl); - goto out; + if (xmlfile) { + if (virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) { + vshReportError(ctl); + goto out; + } else if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_SAVE_PARAM_DXML, xml) < 0) + goto out; } - - if (flags || xml) { + if (flags & VIR_DOMAIN_SAVE_PARALLEL) { + rc = virDomainSaveParametersFlags(dom, params, nparams, flags); + } else if (flags || xml) { rc = virDomainSaveFlags(dom, to, xml, flags); } else { rc = virDomainSave(dom, to); @@ -4242,6 +4272,7 @@ doSave(void *opaque) data->ret = 0; out: + virTypedParamsFree(params, nparams); #ifndef WIN32 pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); out_sig: -- 2.34.1

Signed-off-by: Claudio Fontana <cfontana@suse.de> --- docs/manpages/virsh.rst | 11 +++++++++- tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++++++++------- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 2bce701057..21bfc8b16b 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3752,12 +3752,14 @@ restore :: restore state-file [--bypass-cache] [--xml file] + [--parallel] [--parallel-connections connections] [{--running | --paused}] [--reset-nvram] Restores a domain from a ``virsh save`` state file. See *save* for more info. If *--bypass-cache* is specified, the restore will avoid the file system -cache, although this may slow down the operation. +cache; depending on the specific scenario this may slow down or speed up +the operation. *--xml* ``file`` is usually omitted, but can be used to supply an alternative XML file for use on the restored guest with changes only @@ -3773,6 +3775,13 @@ domain should be started in. If *--reset-nvram* is specified, any existing NVRAM file will be deleted and re-initialized from its pristine template. +*--parallel* option will cause the save data to be loaded from multiple +state files over parallel connections. The main save file is specified +with ``state-file``, and the number of additional channels can be set +using *--parallel-connections* + +Parallel connections may help in speeding up the save operation. + ``Note``: To avoid corrupting file system contents within the domain, you should not reuse the saved state file for a second ``restore`` unless you have also reverted all storage volumes back to the same contents as when diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 1e3adaa1be..3a5df609a7 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5302,6 +5302,14 @@ static const vshCmdOptDef opts_restore[] = { .type = VSH_OT_BOOL, .help = N_("avoid file system cache when restoring") }, + {.name = "parallel", + .type = VSH_OT_BOOL, + .help = N_("enable parallel restore") + }, + {.name = "parallel-connections", + .type = VSH_OT_INT, + .help = N_("number of connections/files for parallel restore") + }, {.name = "xml", .type = VSH_OT_STRING, .completer = virshCompletePathLocalExisting, @@ -5326,17 +5334,36 @@ static bool cmdRestore(vshControl *ctl, const vshCmd *cmd) { const char *from = NULL; + virTypedParameterPtr params = NULL; + int nparams = 0; + int maxparams = 0; + int intOpt = 0; unsigned int flags = 0; const char *xmlfile = NULL; g_autofree char *xml = NULL; virshControl *priv = ctl->privData; - int rc; + int rc = -1; - if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) - return false; + if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) { + goto out; + } else { + if (virTypedParamsAddString(¶ms, &nparams, &maxparams, + VIR_SAVE_PARAM_FILE, from) < 0) + goto out; + } if (vshCommandOptBool(cmd, "bypass-cache")) flags |= VIR_DOMAIN_SAVE_BYPASS_CACHE; + if (vshCommandOptBool(cmd, "parallel")) + flags |= VIR_DOMAIN_SAVE_PARALLEL; + if ((rc = vshCommandOptInt(ctl, cmd, "parallel-connections", &intOpt)) < 0) { + goto out; + } else if (rc > 0) { + rc = -1; + if (virTypedParamsAddInt(¶ms, &nparams, &maxparams, + VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, intOpt) < 0) + goto out; + } if (vshCommandOptBool(cmd, "running")) flags |= VIR_DOMAIN_SAVE_RUNNING; if (vshCommandOptBool(cmd, "paused")) @@ -5345,13 +5372,15 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_SAVE_RESET_NVRAM; if (vshCommandOptStringReq(ctl, cmd, "xml", &xmlfile) < 0) - return false; + goto out; if (xmlfile && virFileReadAll(xmlfile, VSH_MAX_XML_FILE, &xml) < 0) - return false; + goto out; - if (flags || xml) { + if (flags & VIR_DOMAIN_SAVE_PARALLEL) { + rc = virDomainRestoreParametersFlags(priv->conn, params, nparams, flags); + } else if (flags || xml) { rc = virDomainRestoreFlags(priv->conn, from, xml, flags); } else { rc = virDomainRestore(priv->conn, from); @@ -5359,11 +5388,13 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd) if (rc < 0) { vshError(ctl, _("Failed to restore domain from %s"), from); - return false; + goto out; } vshPrintExtra(ctl, _("Domain restored from %s\n"), from); - return true; + out: + virTypedParamsFree(params, nparams); + return rc >= 0; } /* -- 2.34.1

use zstd which is the only really interesting one. Signed-off-by: Claudio Fontana <cfontana@suse.de> --- src/qemu/qemu_capabilities.c | 4 +++ src/qemu/qemu_capabilities.h | 3 ++ src/qemu/qemu_migration.c | 6 ++++ src/qemu/qemu_migration_params.c | 49 ++++++++++++++++---------------- src/qemu/qemu_migration_params.h | 6 ++++ src/qemu/qemu_saveimage.c | 6 ++++ 6 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index c5afe1439e..bc6bb9c5ac 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -672,6 +672,9 @@ VIR_ENUM_IMPL(virQEMUCaps, "virtio-iommu-pci", /* QEMU_CAPS_DEVICE_VIRTIO_IOMMU_PCI */ "virtio-iommu.boot-bypass", /* QEMU_CAPS_VIRTIO_IOMMU_BOOT_BYPASS */ "virtio-net.rss", /* QEMU_CAPS_VIRTIO_NET_RSS */ + + /* 430 */ + "migration-param.multifd-compression", /* QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION */ ); @@ -1609,6 +1612,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsQMPSchemaQueries[] = { { "migrate-set-parameters/arg-type/downtime-limit", QEMU_CAPS_MIGRATION_PARAM_DOWNTIME }, { "migrate-set-parameters/arg-type/xbzrle-cache-size", QEMU_CAPS_MIGRATION_PARAM_XBZRLE_CACHE_SIZE }, { "migrate-set-parameters/arg-type/block-bitmap-mapping/bitmaps/transform", QEMU_CAPS_MIGRATION_PARAM_BLOCK_BITMAP_MAPPING }, + { "migrate-set-parameters/arg-type/multifd-compression", QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION }, { "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS }, { "nbd-server-add/arg-type/bitmap", QEMU_CAPS_NBD_BITMAP }, { "netdev_add/arg-type/+vhost-vdpa", QEMU_CAPS_NETDEV_VHOST_VDPA }, diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9b240e47fb..c67188a9dc 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -648,6 +648,9 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_VIRTIO_IOMMU_BOOT_BYPASS, /* virtio-iommu.boot-bypass */ QEMU_CAPS_VIRTIO_NET_RSS, /* virtio-net rss feature */ + /* 430 */ + QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION, /* multifd-compression in migrate-set-parameters */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7aec2dc377..f23b09b76b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5950,6 +5950,12 @@ qemuMigrationSrcToFileAux(virQEMUDriver *driver, virDomainObj *vm, QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, nchannels) < 0) return -1; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION)) { + if (qemuMigrationParamsSetString(migParams, + QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION, + "zstd") < 0) + return -1; + } } if (needParams && qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 36174a66d8..f6b9dc337d 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -115,6 +115,7 @@ VIR_ENUM_IMPL(qemuMigrationParam, "xbzrle-cache-size", "max-postcopy-bandwidth", "multifd-channels", + "multifd-compression", ); typedef struct _qemuMigrationParamsAlwaysOnItem qemuMigrationParamsAlwaysOnItem; @@ -234,6 +235,7 @@ static const qemuMigrationParamType qemuMigrationParamTypes[] = { [QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE] = QEMU_MIGRATION_PARAM_TYPE_ULL, [QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH] = QEMU_MIGRATION_PARAM_TYPE_ULL, [QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS] = QEMU_MIGRATION_PARAM_TYPE_INT, + [QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION] = QEMU_MIGRATION_PARAM_TYPE_STRING, }; G_STATIC_ASSERT(G_N_ELEMENTS(qemuMigrationParamTypes) == QEMU_MIGRATION_PARAM_LAST); @@ -898,31 +900,6 @@ qemuMigrationParamsApply(virQEMUDriver *driver, } -/** - * qemuMigrationParamsSetString: - * @migrParams: migration parameter object - * @param: parameter to set - * @value: new value - * - * Enables and sets the migration parameter @param in @migrParams. Returns 0 on - * success and -1 on error. Libvirt error is reported. - */ -static int -qemuMigrationParamsSetString(qemuMigrationParams *migParams, - qemuMigrationParam param, - const char *value) -{ - if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_STRING) < 0) - return -1; - - migParams->params[param].value.s = g_strdup(value); - - migParams->params[param].set = true; - - return 0; -} - - /* qemuMigrationParamsEnableTLS * @driver: pointer to qemu driver * @vm: domain object @@ -1144,6 +1121,28 @@ qemuMigrationParamsSetULL(qemuMigrationParams *migParams, return 0; } +/** + * qemuMigrationParamsSetString: + * @migrParams: migration parameter object + * @param: parameter to set + * @value: new value + * + * Enables and sets the migration parameter @param in @migrParams. Returns 0 on + * success and -1 on error. Libvirt error is reported. + */ +int +qemuMigrationParamsSetString(qemuMigrationParams *migParams, + qemuMigrationParam param, + const char *value) +{ + if (qemuMigrationParamsCheckType(param, QEMU_MIGRATION_PARAM_TYPE_STRING) < 0) + return -1; + + migParams->params[param].value.s = g_strdup(value); + migParams->params[param].set = true; + return 0; +} + /** * Returns -1 on error, diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h index 99af73b4a4..23a4e0c8a2 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -60,6 +60,7 @@ typedef enum { QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE, QEMU_MIGRATION_PARAM_MAX_POSTCOPY_BANDWIDTH, QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, + QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION, QEMU_MIGRATION_PARAM_LAST } qemuMigrationParam; @@ -137,6 +138,11 @@ qemuMigrationParamsSetULL(qemuMigrationParams *migParams, qemuMigrationParam param, unsigned long long value); +int +qemuMigrationParamsSetString(qemuMigrationParams *migParams, + qemuMigrationParam param, + const char *value); + int qemuMigrationParamsGetULL(qemuMigrationParams *migParams, qemuMigrationParam param, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index ecdab98168..3eac72f7cd 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -608,6 +608,12 @@ int qemuSaveImageLoadMultiFd(virConnectPtr conn, virDomainObj *vm, int oflags, QEMU_MIGRATION_PARAM_MULTIFD_CHANNELS, nchannels) < 0) goto cleanup; + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_PARAM_MULTIFD_COMPRESSION)) { + if (qemuMigrationParamsSetString(migParams, + QEMU_MIGRATION_PARAM_MULTIFD_COMPRESSION, + "zstd") < 0) + goto cleanup; + } if (qemuMigrationParamsApply(driver, vm, asyncJob, migParams) < 0) goto cleanup; -- 2.34.1

On 4/26/22 6:47 PM, Claudio Fontana wrote:
This is the multifd save prototype in its first semi-functional state, now with both save and restore minimally functional.
Still as mentioned before, likely there are quite a few rough edges,
let me know what you think about this possible option.
changes from v2 are many, mainly:
* added ability to restore the VM from disk using multifd
* fixed the multifd-helper to work in both directions, assuming the need to listen for save, and connect for restore.
* fixed a large number of bugs, and probably introduced some :-)
KNOWN ISSUES:
1) this applies only to virsh save and virsh restore for now (no managed save etc).
2) the .pl scripts to generate the headers for the new APIs do not reliably work for me, for the Restore case. I get:
src/remote/remote_daemon_dispatch_stubs.h:10080:9: error: too few arguments to function ‘virDomainRestoreParametersFlags’
if (virDomainRestoreParametersFlags(params, nparams, args->flags) < 0)
To work around this I had to fixup the header manually to look like:
...(conn, params, nparams, args->flags) < 0)
3) after the restore the cpus are still paused, need to investigate. Requires a "kick" via QMP "cont" command to get moving currently.
Thanks for your thoughts,
Claudio
Claudio Fontana (19): iohelper: introduce new struct to carry copy operation parameters iohelper: refactor copy operation as a separate function libvirt: introduce virDomainSaveParametersFlags public API libvirt: introduce virDomainRestoreParametersFlags public API remote: Add RPC support for the virDomainSaveParametersFlags API remote: Add RPC support for the virDomainRestoreParametersFlags API qemu: add a stub for virDomainSaveParametersFlags API qemu: add a stub for virDomainRestoreParametersFlags API qemu: saveimage: introduce virQEMUSaveFd iohelper: move runIO function to a separate module runio: add arguments to extend use beyond just stdin and stdout multifd-helper: new helper for parallel save/restore qemu: wire up saveimage code with the multifd helper qemu: implement qemuMigrationSrcToFilesMultiFd qemu: add parameter to qemuMigrationDstRun to skip waiting qemu: implement qemuSaveImageLoadMultiFd tools: add parallel parameter to virsh save command tools: add parallel parameter to virsh restore command qemu: add migration parameter multifd-compression
docs/manpages/virsh.rst | 34 ++- include/libvirt/libvirt-domain.h | 13 + src/driver-hypervisor.h | 14 + src/libvirt-domain.c | 99 +++++++ src/libvirt_private.syms | 1 + src/libvirt_public.syms | 6 + src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_driver.c | 233 +++++++++++---- src/qemu/qemu_migration.c | 155 ++++++---- src/qemu/qemu_migration.h | 16 +- src/qemu/qemu_migration_params.c | 71 +++-- src/qemu/qemu_migration_params.h | 15 + src/qemu/qemu_process.c | 3 +- src/qemu/qemu_process.h | 5 +- src/qemu/qemu_saveimage.c | 480 ++++++++++++++++++++++++------- src/qemu/qemu_saveimage.h | 49 +++- src/qemu/qemu_snapshot.c | 6 +- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 29 +- src/remote_protocol-structs | 17 ++ src/util/iohelper.c | 150 +--------- src/util/meson.build | 15 + src/util/multifd-helper.c | 250 ++++++++++++++++ src/util/runio.c | 214 ++++++++++++++ src/util/runio.h | 38 +++ src/util/virthread.c | 5 + src/util/virthread.h | 1 + tools/virsh-domain.c | 96 +++++-- 29 files changed, 1599 insertions(+), 426 deletions(-) create mode 100644 src/util/multifd-helper.c create mode 100644 src/util/runio.c create mode 100644 src/util/runio.h
participants (3)
-
Claudio Fontana
-
Daniel P. Berrangé
-
Jim Fehlig