this is in preparation for a minor refactoring of the copy function itself out of runIO(). Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- 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.35.3

where it can be reused by other helpers. No changes other than the move. Note that this makes iohelper now dependent on -lutil and -lacl. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/util/iohelper.c | 175 ---------------------------------------- src/util/meson.build | 5 ++ src/util/virfile.c | 186 +++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 2 + 4 files changed, 193 insertions(+), 175 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 1584321839..b3aaf82b9f 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -41,181 +41,6 @@ #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; -} - static const char *program_name; G_GNUC_NORETURN static void diff --git a/src/util/meson.build b/src/util/meson.build index 24350a3e67..17755373c8 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -175,6 +175,7 @@ keycode_dep = declare_dependency( io_helper_sources = [ 'iohelper.c', + 'virfile.c', ] virt_util_lib = static_library( @@ -213,6 +214,10 @@ if conf.has('WITH_LIBVIRTD') files(io_helper_sources), dtrace_gen_headers, ], + 'deps': [ + acl_dep, + libutil_dep, + ], } endif diff --git a/src/util/virfile.c b/src/util/virfile.c index 130b0fbace..0bfdbea81f 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -4577,3 +4577,189 @@ virFileSetCOW(const char *path, return 0; #endif /* ! __linux__ */ } + +#ifndef WIN32 +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; +} + +#else /* WIN32 */ + +off_t +runIO(const char *path G_GNUC_UNUSED, + int fd G_GNUC_UNUSED, + int oflags G_GNUC_UNUSED) +{ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("runIO unsupported on this platform")); + return -1; +} +#endif /* WIN32 */ diff --git a/src/util/virfile.h b/src/util/virfile.h index b04386f6e6..0dc336e339 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -383,3 +383,5 @@ int virFileDataSync(int fd); int virFileSetCOW(const char *path, virTristateBool state); + +off_t runIO(const char *path, int fd, int oflags); -- 2.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/util/iohelper.c | 17 +----------- src/util/virfile.c | 64 +++++++++++++++++++++++++++++++-------------- src/util/virfile.h | 2 +- 3 files changed, 46 insertions(+), 37 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index ce10ccb905..055540c8c4 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -58,7 +58,6 @@ int main(int argc, char **argv) { const char *path; - int oflags = -1; int fd = -1; program_name = argv[0]; @@ -79,25 +78,11 @@ main(int argc, char **argv) program_name, argv[3]); exit(EXIT_FAILURE); } -#ifdef F_GETFL - oflags = fcntl(fd, F_GETFL); -#else - /* Stupid mingw. */ - if (fd == STDIN_FILENO) - oflags = O_RDONLY; - else if (fd == STDOUT_FILENO) - oflags = O_WRONLY; -#endif - if (oflags < 0) { - fprintf(stderr, _("%s: unable to determine access mode of fd %d"), - program_name, fd); - exit(EXIT_FAILURE); - } } else { /* unknown argc pattern */ usage(EXIT_FAILURE); } - if (fd < 0 || virFileDiskCopy(path, fd, oflags) < 0) + if (fd < 0 || virFileDiskCopy(fd, path, -1, "stdio") < 0) goto error; return 0; diff --git a/src/util/virfile.c b/src/util/virfile.c index 4aa7be4740..8b07db6679 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -4670,19 +4670,46 @@ runIOCopy(const struct runIOParams p) return total; } +/** + * virFileDiskCopy: run IO to copy data between storage and a pipe or socket. + * + * @disk_fd: the already open regular file or block device + * @disk_path: the pathname corresponding to disk_fd (for error reporting) + * @remote_fd: the pipe or socket + * Use -1 to auto-choose between STDIN or STDOUT. + * @remote_path: the pathname corresponding to remote_fd (for error reporting) + * + * Note that the direction of the transfer is detected based on the @disk_fd + * file access mode (man 2 open). Therefore @disk_fd must be opened with + * O_RDONLY or O_WRONLY. O_RDWR is not supported. + * + * virFileDiskCopy always closes the file descriptor disk_fd, + * and any error during close(2) is reported and considered a failure. + * + * Returns: bytes transferred or < 0 on failure. + */ off_t -virFileDiskCopy(const char *path, int fd, int oflags) +virFileDiskCopy(int disk_fd, const char *disk_path, int remote_fd, const char *remote_path) { int ret = -1; off_t total = 0; struct stat sb; struct runIOParams p; + int oflags = -1; - if (fstat(fd, &sb) < 0) { + oflags = fcntl(disk_fd, F_GETFL); + + if (oflags < 0) { virReportSystemError(errno, - _("Unable to access file descriptor %d path %s"), - fd, path); + _("unable to determine access mode of %s"), + disk_path); + goto cleanup; + } + if (fstat(disk_fd, &sb) < 0) { + virReportSystemError(errno, + _("unable to stat file descriptor %d path %s"), + disk_fd, disk_path); goto cleanup; } p.isBlockDev = S_ISBLK(sb.st_mode); @@ -4691,23 +4718,21 @@ virFileDiskCopy(const char *path, int fd, int oflags) switch (oflags & O_ACCMODE) { case O_RDONLY: p.isWrite = false; - p.fdin = fd; - p.fdinname = path; - p.fdout = STDOUT_FILENO; - p.fdoutname = "stdout"; + p.fdin = disk_fd; + p.fdinname = disk_path; + p.fdout = remote_fd >= 0 ? remote_fd : STDOUT_FILENO; + p.fdoutname = remote_path; break; case O_WRONLY: p.isWrite = true; - p.fdin = STDIN_FILENO; - p.fdinname = "stdin"; - p.fdout = fd; - p.fdoutname = path; + p.fdin = remote_fd >= 0 ? remote_fd : STDIN_FILENO; + p.fdinname = remote_path; + p.fdout = disk_fd; + p.fdoutname = disk_path; break; - case O_RDWR: default: - virReportSystemError(EINVAL, - _("Unable to process file with flags %d"), + virReportSystemError(EINVAL, _("Unable to process file with flags %d"), (oflags & O_ACCMODE)); goto cleanup; } @@ -4716,12 +4741,12 @@ virFileDiskCopy(const char *path, int fd, int oflags) if (!p.isBlockDev && p.isDirect) { off_t off; if (p.isWrite) { - if ((off = lseek(fd, 0, SEEK_END)) != 0) { + if ((off = lseek(disk_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) { + } else if ((off = lseek(disk_fd, 0, SEEK_CUR)) != 0) { virReportSystemError(off < 0 ? errno : EINVAL, "%s", _("O_DIRECT read needs entire seekable file")); goto cleanup; @@ -4743,9 +4768,8 @@ virFileDiskCopy(const char *path, int fd, int oflags) ret = 0; cleanup: - if (VIR_CLOSE(fd) < 0 && - ret == 0) { - virReportSystemError(errno, _("Unable to close %s"), path); + if (VIR_CLOSE(disk_fd) < 0 && ret == 0) { + virReportSystemError(errno, _("Unable to close %s"), disk_path); ret = -1; } return ret; diff --git a/src/util/virfile.h b/src/util/virfile.h index 59efb760f3..8e378efe30 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -384,4 +384,4 @@ int virFileDataSync(int fd); int virFileSetCOW(const char *path, virTristateBool state); -off_t virFileDiskCopy(const char *path, int fd, int oflags); +off_t virFileDiskCopy(int disk_fd, const char *disk_path, int remote_fd, const char *remote_path); -- 2.35.3

create an alternative to virDomainSaveFlags that is more extendible via parameters. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- include/libvirt/libvirt-domain.h | 26 +++++++++++++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 48 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ 4 files changed, 86 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 2edef9c4e1..52c86f3813 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1562,6 +1562,10 @@ int virDomainSaveFlags (virDomainPtr domain, const char *to, const char *dxml, unsigned int flags); +int virDomainSaveParams (virDomainPtr domain, + virTypedParameterPtr params, + int nparams, + unsigned int flags); int virDomainRestore (virConnectPtr conn, const char *from); int virDomainRestoreFlags (virConnectPtr conn, @@ -1569,6 +1573,28 @@ int virDomainRestoreFlags (virConnectPtr conn, const char *dxml, unsigned int flags); +/** + * VIR_SAVE_PARAM_FILE: + * + * the parameter used to specify the savestate file to save to or restore from. + * + * Since: 8.4.0 + */ +# define VIR_SAVE_PARAM_FILE "file" + +/** + * VIR_SAVE_PARAM_DXML: + * + * an optional parameter used to adjust guest xml on restore. + * If the hypervisor supports it, it can be used to alter + * host-specific portions of the domain XML that will be used when + * restoring an image. For example, it is possible to alter the + * device while the domain is stopped. + * + * Since: 8.4.0 + */ +# define VIR_SAVE_PARAM_DXML "dxml" + /* 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..434a7d7cc4 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 +(*virDrvDomainSaveParams)(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; + virDrvDomainSaveParams domainSaveParams; virDrvDomainRestore domainRestore; virDrvDomainRestoreFlags domainRestoreFlags; virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 27993829a0..ca2fe5e99d 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -999,6 +999,54 @@ virDomainSaveFlags(virDomainPtr domain, const char *to, return -1; } +/** + * virDomainSaveParams: + * @domain: a domain object + * @params: save parameters + * @nparams: number of save parameters + * @flags: bitwise-OR of virDomainSaveRestoreFlags + * + * This method extends virDomainSaveFlags by adding parameters. + * + * Returns 0 in case of success and -1 in case of failure. + * + * Since: 8.4.0 + */ +int +virDomainSaveParams(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->domainSaveParams) { + if (conn->driver->domainSaveParams(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..32ffbbaf84 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.4.0 { + global: + virDomainSaveParams; +} LIBVIRT_8.0.0; + # .... define new API here using predicted next version number .... -- 2.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- 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..65fc19643a 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 */ + .domainSaveParams = remoteDomainSaveParams, /* 8.4.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..ef52ff346a 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; @@ -980,6 +983,12 @@ struct remote_domain_save_flags_args { unsigned int flags; }; +struct remote_domain_save_params_args { + remote_nonnull_domain dom; + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + struct remote_domain_restore_args { remote_nonnull_string from; }; @@ -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_PARAMS = 440 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index d88176781d..f5e0a60396 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_params_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_PARAMS = 440, }; -- 2.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- 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 65fc19643a..423f5f9fb9 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8449,6 +8449,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSaveParams = remoteDomainSaveParams, /* 8.4.0 */ .domainRestore = remoteDomainRestore, /* 0.3.0 */ .domainRestoreFlags = remoteDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParams = remoteDomainRestoreParams, /* 8.4.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 ef52ff346a..085631c11b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -999,6 +999,11 @@ struct remote_domain_restore_flags_args { unsigned int flags; }; +struct remote_domain_restore_params_args { + remote_typed_param params<REMOTE_DOMAIN_SAVE_PARAMS_MAX>; + unsigned int flags; +}; + struct remote_domain_save_image_get_xml_desc_args { remote_nonnull_string file; unsigned int flags; @@ -6935,5 +6940,12 @@ enum remote_procedure { * @generate: both * @acl: domain:hibernate */ - REMOTE_PROC_DOMAIN_SAVE_PARAMS = 440 + REMOTE_PROC_DOMAIN_SAVE_PARAMS = 440, + + /** + * @generate: both + * @acl: domain:start + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index f5e0a60396..4ffdce5679 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_params_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_PARAMS = 440, + REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, }; -- 2.35.3

whops this is pretty wrong. On 5/6/22 3:10 PM, Claudio Fontana wrote: Could you help me understand what is the right check to use here? Should I do like before, if (flags || xml) { rc = virDomainSaveFlags(dom, to, xml, flags); } else { rc = virDomainSave(dom, to); } and only involve virDomainSaveParams when introduced later? How should we decide whether to trigger virDomainSaveFlags or virDomainSaveParams ...?

On 5/6/22 3:24 PM, Daniel P. Berrangé wrote: Very helpful, thanks. Claudio

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(a)suse.de&gt; --- po/POTFILES.in | 1 + src/libvirt_private.syms | 1 + src/util/meson.build | 15 +++ src/util/multifd-helper.c | 249 ++++++++++++++++++++++++++++++++++++++ src/util/virthread.c | 5 + src/util/virthread.h | 1 + 6 files changed, 272 insertions(+) create mode 100644 src/util/multifd-helper.c diff --git a/po/POTFILES.in b/po/POTFILES.in index 0d9adb0758..4efb330262 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -241,6 +241,7 @@ @SRCDIR(a)src/storage_file/storage_source_backingstore.c @SRCDIR(a)src/test/test_driver.c @SRCDIR(a)src/util/iohelper.c +@SRCDIR(a)src/util/multifd-helper.c @SRCDIR(a)src/util/viralloc.c @SRCDIR(a)src/util/virarptable.c @SRCDIR(a)src/util/viraudit.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 17755373c8..4cbc6107c1 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -178,6 +178,11 @@ io_helper_sources = [ 'virfile.c', ] +multifd_helper_sources = [ + 'multifd-helper.c', + 'virfile.c', +] + virt_util_lib = static_library( 'virt_util', [ @@ -219,6 +224,16 @@ if conf.has('WITH_LIBVIRTD') libutil_dep, ], } + virt_helpers += { + 'name': 'libvirt_multifd_helper', + 'sources': [ + files(multifd_helper_sources), + dtrace_gen_headers, + ], + 'deps': [ + libutil_dep, + ], + } 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..59657578e2 --- /dev/null +++ b/src/util/multifd-helper.c @@ -0,0 +1,249 @@ +/* + * 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" + +#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 = virFileDiskCopy(c->filefd, c->path, c->clientfd, "socket"); +} + +static off_t waitClientThreads(multiFdConnData *conn, int n) +{ + int idx; + off_t total = 0; + for (idx = 0; idx < n; idx++) { + multiFdConnData *c = &conn[idx]; + 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 idx; + args->total = -1; + + for (idx = 0; idx < args->nchannels + 1; idx++) { + /* Perform outgoing connections */ + multiFdConnData *c = &args->conn[idx]; + 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 (idx = 0; idx < args->nchannels + 1; idx++) { + multiFdConnData *c = &args->conn[idx]; + VIR_FORCE_CLOSE(c->clientfd); + } +} + +static void saveThreadFunc(void *a) +{ + multiFdThreadArgs *args = a; + int idx; + 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 (idx = 0; idx < args->nchannels + 1; idx++) { + /* Wait for incoming connection. */ + multiFdConnData *c = &args->conn[idx]; + 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 (idx = 0; idx < args->nchannels + 1; idx++) { + multiFdConnData *c = &args->conn[idx]; + 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 idx; + + /* 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 (idx = 3; idx < 3 + args.nchannels + 1; idx++) { + multiFdConnData *c = &args.conn[idx - 3]; + + if (virStrToLong_i(argv[idx], NULL, 10, &c->filefd) < 0) { + fprintf(stderr, _("%s: malformed FD %s"), program_name, argv[idx]); + 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; + virStrcpyStatic(args.serv_addr.sun_path, args.sun_path); + + 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.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_driver.c | 51 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4a74713738..e3582f62a7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5806,12 +5806,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, + unsigned int flags, + int (*ensureACL)(virConnectPtr, virDomainDef *)) { virQEMUDriver *driver = conn->privateData; qemuDomainObjPrivate *priv = NULL; @@ -5840,7 +5840,7 @@ qemuDomainRestoreFlags(virConnectPtr conn, if (fd < 0) goto cleanup; - if (virDomainRestoreFlagsEnsureACL(conn, def) < 0) + if (ensureACL(conn, def) < 0) goto cleanup; if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) { @@ -5908,11 +5908,47 @@ qemuDomainRestoreFlags(virConnectPtr conn, return ret; } +static int +qemuDomainRestoreFlags(virConnectPtr conn, + const char *path, + const char *dxml, + unsigned int flags) +{ + return qemuDomainRestoreInternal(conn, path, dxml, flags, + virDomainRestoreFlagsEnsureACL); +} + static int qemuDomainRestore(virConnectPtr conn, const char *path) { - return qemuDomainRestoreFlags(conn, path, NULL, 0); + return qemuDomainRestoreInternal(conn, path, NULL, 0, + virDomainRestoreEnsureACL); +} + +static int +qemuDomainRestoreParams(virConnectPtr conn, + virTypedParameterPtr params, int nparams, + unsigned int flags) +{ + const char *path = NULL; + const char *dxml = NULL; + int ret = -1; + + if (virTypedParamsValidate(params, nparams, + VIR_SAVE_PARAM_FILE, VIR_TYPED_PARAM_STRING, + VIR_SAVE_PARAM_DXML, VIR_TYPED_PARAM_STRING, + 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; + + ret = qemuDomainRestoreInternal(conn, path, dxml, flags, + virDomainRestoreParamsEnsureACL); + return ret; } static char * @@ -20884,6 +20920,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSaveParams = qemuDomainSaveParams, /* 8.4.0 */ .domainRestore = qemuDomainRestore, /* 0.2.0 */ .domainRestoreFlags = qemuDomainRestoreFlags, /* 0.9.4 */ + .domainRestoreParams = qemuDomainRestoreParams, /* 8.4.0 */ .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = qemuDomainCoreDump, /* 0.7.0 */ -- 2.35.3

and its companion param VIR_SAVE_PARAM_PARALLEL_CONNECTIONS Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_driver.c | 17 +++++++++++------ src/qemu/qemu_saveimage.c | 1 + src/qemu/qemu_saveimage.h | 1 + src/qemu/qemu_snapshot.c | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e3582f62a7..e276a66815 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); @@ -2815,16 +2815,19 @@ qemuDomainSaveParams(virDomainPtr dom, 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, -1); + 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; @@ -2832,6 +2835,8 @@ qemuDomainSaveParams(virDomainPtr dom, 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, @@ -2849,7 +2854,7 @@ qemuDomainSaveParams(virDomainPtr dom, goto cleanup; ret = qemuDomainSaveInternal(driver, vm, to, compressed, - compressor, dxml, flags); + compressor, dxml, nconn, flags); cleanup: virDomainObjEndAPI(&vm); @@ -2906,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; diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 4fd4c5cfcd..7c76db359e 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 G_GNUC_UNUSED, unsigned int flags, virDomainAsyncJob asyncJob) { 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.35.3

use this data type to encapsulate the pathname, file descriptor, wrapper, and need to unlink. This will make management of the resources associated with an FD used for QEMU save/restore much easier, reducing the amount of explicit cleanup required. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_saveimage.c | 117 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_saveimage.h | 18 ++++++ 2 files changed, 135 insertions(+) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 7c76db359e..67c93e3865 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 diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index b3d5c02fd6..41937e5eb5 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, -- 2.35.3

all the logic to open a fd, create a wrapper etc, is boilerplate code that is best reused, so change the Open function to take an existing already initialized virQEMUSaveFd. Adapt all callers of qemuSaveImageOpen. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_driver.c | 100 +++++++++++++++++++++++--------------- src/qemu/qemu_saveimage.c | 86 ++++++++------------------------ src/qemu/qemu_saveimage.h | 9 ++-- 3 files changed, 82 insertions(+), 113 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ff2be6ffe9..e4cdd93b50 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5825,12 +5825,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 | @@ -5841,10 +5842,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 (ensureACL(conn, def) < 0) @@ -5898,16 +5907,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); @@ -5969,15 +5975,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) @@ -5987,7 +5994,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; } @@ -5999,22 +6007,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) @@ -6041,15 +6050,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; } @@ -6057,8 +6066,8 @@ qemuDomainSaveImageDefineXML(virConnectPtr conn, const char *path, ret = 0; cleanup: - VIR_FORCE_CLOSE(fd); virQEMUSaveDataFree(data); + ret = virQEMUSaveFdFini(&saveFd, NULL, ret); return ret; } @@ -6070,8 +6079,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); @@ -6093,15 +6103,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; } @@ -6152,16 +6166,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; } @@ -6205,15 +6228,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 91fba7bd7d..5a569fa52e 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -518,94 +518,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; } @@ -636,7 +591,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; @@ -645,7 +600,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; @@ -661,10 +616,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 41937e5eb5..5dc63f3661 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -92,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.35.3

add APIs to Create, Close and Free MultiFD files to associate with multifd channels. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_saveimage.c | 87 +++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_saveimage.h | 11 +++++ 2 files changed, 98 insertions(+) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 5a569fa52e..38ebc7dd66 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -365,6 +365,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 idx; + + if (!multiFd) + return ret; + + for (idx = 0; idx < nconn; idx++) { + ret = virQEMUSaveFdFini(&multiFd[idx], vm, ret); + } + /* + * do it again to unlink all in the error case, + * if error happened in the middle of previous loop. + */ + for (idx = 0; idx < nconn; idx++) { + ret = virQEMUSaveFdFini(&multiFd[idx], 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 idx; + + if (!multiFd) + return 0; + + for (idx = 0; idx < nconn; idx++) { + if (virQEMUSaveFdClose(&multiFd[idx], 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 idx; + + for (idx = 0; idx < nconn; idx++) { + virQEMUSaveFd *m = &multiFd[idx]; + if (virQEMUSaveFdInit(m, path, idx + 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 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.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 3 +++ tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 + tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 + 34 files changed, 39 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 1ed4cda7f0..581b6a40df 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 */ + "migrate-multifd", /* QEMU_CAPS_MIGRATE_MULTIFD */ ); @@ -1230,6 +1233,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { { "rdma-pin-all", QEMU_CAPS_MIGRATE_RDMA }, + { "multifd", QEMU_CAPS_MIGRATE_MULTIFD }, }; /* Use virQEMUCapsQMPSchemaQueries for querying parameters of events */ diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 9b240e47fb..b089f83da1 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_MIGRATE_MULTIFD, /* migrate can set multifd parameter */ + QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml index 5adf904fc4..4ca2cfa81c 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml @@ -148,6 +148,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml index a84adc2610..1db978eb4c 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml @@ -153,6 +153,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml index c494254c4d..251d4dfd29 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml @@ -145,6 +145,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml index d2582fa297..a4af47c6a4 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml @@ -145,6 +145,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml index 4f36186044..2bab764867 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml @@ -115,6 +115,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml index 18e5ebd4f4..aa8a9812e5 100644 --- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml @@ -188,6 +188,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100240</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml index 12c5ebe6f3..bd89f0c6b2 100644 --- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml @@ -195,6 +195,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml index ee536b7b63..369ef707b9 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml @@ -163,6 +163,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml index 10f5a9e2c5..16c867a46b 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml @@ -160,6 +160,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml index 069777a49b..b584ba7352 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml @@ -128,6 +128,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml index 6b61214a0b..5023028678 100644 --- a/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml @@ -206,6 +206,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='virtio-blk.queue-size'/> + <flag name='migrate-multifd'/> <version>4002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml index 4fd02e786d..c45b2e6cf6 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml @@ -175,6 +175,7 @@ <flag name='virtio-blk.queue-size'/> <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> + <flag name='migrate-multifd'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml index f2f3558fdc..a3ad743d70 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml @@ -181,6 +181,7 @@ <flag name='virtio-blk.queue-size'/> <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> + <flag name='migrate-multifd'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml index 557949d6d6..e1b5cac26b 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml @@ -167,6 +167,7 @@ <flag name='virtio-blk.queue-size'/> <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> + <flag name='migrate-multifd'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml index f301d8a926..796adb9066 100644 --- a/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml @@ -215,6 +215,7 @@ <flag name='virtio-blk.queue-size'/> <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> + <flag name='migrate-multifd'/> <version>5000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100241</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml index 3a330ebdc0..cb203df125 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml @@ -87,6 +87,7 @@ <flag name='input-linux'/> <flag name='query-display-options'/> <flag name='memory-backend-file.prealloc-threads'/> + <flag name='migrate-multifd'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml index 53fcbf3417..7479d942a2 100644 --- a/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml @@ -219,6 +219,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>5001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml index 824224302c..268d1444ad 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml @@ -182,6 +182,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml index b949f88b5a..eabf4b600c 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml @@ -186,6 +186,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml index 873923992d..0dbaf5a5ec 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml @@ -172,6 +172,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml index 5e9560d7b7..b0fbab9cb5 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml @@ -140,6 +140,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml index 3998da9253..1a1717bf2a 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml @@ -223,6 +223,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>5002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml index 51d3628eeb..1c18d122e2 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml @@ -190,6 +190,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml index 2e5d0f197a..8fa4cb2307 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml @@ -148,6 +148,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>39100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml index 3498d6255b..70c67202b1 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml @@ -232,6 +232,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100242</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml index ddeca62290..a5ec77878f 100644 --- a/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml @@ -236,6 +236,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6001000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml index 5538940372..92d8ceff7e 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml @@ -201,6 +201,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6001050</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml index 9c9d9aa08e..f219912927 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml @@ -197,6 +197,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml index dba5ecaf87..38fd3878ea 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml @@ -238,6 +238,7 @@ <flag name='memory-backend-file.prealloc-threads'/> <flag name='virtio-iommu-pci'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6002000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100244</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml index 257b0f625d..522e225c8f 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml @@ -209,6 +209,7 @@ <flag name='virtio-iommu-pci'/> <flag name='virtio-iommu.boot-bypass'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6002092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>61700243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml index 1ddca7d767..1eb43799c0 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml @@ -210,6 +210,7 @@ <flag name='virtio-iommu-pci'/> <flag name='virtio-iommu.boot-bypass'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>6002092</version> <kvmVersion>0</kvmVersion> <microcodeVersion>42900243</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml index 8074c97ecd..e5023c4219 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml @@ -242,6 +242,7 @@ <flag name='virtio-iommu-pci'/> <flag name='virtio-iommu.boot-bypass'/> <flag name='virtio-net.rss'/> + <flag name='migrate-multifd'/> <version>7000000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>43100243</microcodeVersion> -- 2.35.3

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(a)suse.de&gt; --- 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 e4cdd93b50..480f9b4ed9 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); @@ -5908,7 +5908,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); @@ -6229,7 +6229,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); @@ -6492,7 +6492,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 004e84556c..93cd446b23 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 d8b6635511..d58e070bf7 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -749,6 +749,7 @@ qemuSaveImageStartVM(virConnectPtr conn, const char *path, bool start_paused, bool reset_nvram, + bool wait_incoming, virDomainAsyncJob asyncJob) { qemuDomainObjPrivate *priv = vm->privateData; @@ -803,7 +804,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.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- docs/manpages/virsh.rst | 23 +++++++++++++++++------ tools/virsh-domain.c | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index e73e590754..e9012b85d1 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3803,15 +3803,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 @@ -3833,6 +3836,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 18498540f6..19359683ed 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4174,6 +4174,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, @@ -4206,6 +4214,8 @@ doSave(void *opaque) 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; @@ -4228,6 +4238,15 @@ doSave(void *opaque) } 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(&params, &nparams, &maxparams, + VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, intOpt) < 0) + goto out; + } if (vshCommandOptBool(cmd, "running")) flags |= VIR_DOMAIN_SAVE_RUNNING; if (vshCommandOptBool(cmd, "paused")) -- 2.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- docs/formatsnapshot.rst | 5 +++-- include/libvirt/libvirt-domain.h | 1 + src/libvirt-domain.c | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/docs/formatsnapshot.rst b/docs/formatsnapshot.rst index 9b874e4890..dd742a3063 100644 --- a/docs/formatsnapshot.rst +++ b/docs/formatsnapshot.rst @@ -38,8 +38,9 @@ full system Libvirt can manage all three types of snapshots. For now, VM state (memory) snapshots are created only by the ``virDomainSave()``, ``virDomainSaveFlags``, -and ``virDomainManagedSave()`` functions, and restored via the -``virDomainRestore()``, ``virDomainRestoreFlags()``, ``virDomainCreate()``, and +``virDomainSaveParams`` and ``virDomainManagedSave()`` functions, and restored +via the ``virDomainRestore()``, ``virDomainRestoreFlags()``, +``virDomainRestoreParams``, ``virDomainCreate()``, and ``virDomainCreateWithFlags()`` functions (as well as via domain autostart). With managed snapshots, libvirt tracks all information internally; with save images, the user tracks the snapshot file, but libvirt provides functions such as diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index d130fbf481..5e858f854b 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1544,6 +1544,7 @@ int virDomainPMWakeup (virDomainPtr domain, * virDomainSaveRestoreFlags: * * Flags for use in virDomainSaveFlags(), virDomainManagedSave(), + * virDomainSaveParams(), virDomainRestoreParams(), * virDomainRestoreFlags(), and virDomainSaveImageDefineXML(). Not all * flags apply to all these functions. * diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0d6afcc1c6..d1d62daa71 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -859,9 +859,9 @@ virDomainPMWakeup(virDomainPtr dom, * listed as running anymore (this ends the life of a transient domain). * Use virDomainRestore() to restore a domain after saving. * - * See virDomainSaveFlags() for more control. Also, a save file can - * be inspected or modified slightly with virDomainSaveImageGetXMLDesc() - * and virDomainSaveImageDefineXML(). + * See virDomainSaveFlags() and virDomainSaveParams() for more control. + * Also, a save file can be inspected or modified slightly with + * virDomainSaveImageGetXMLDesc() and virDomainSaveImageDefineXML(). * * Returns 0 in case of success and -1 in case of failure. * -- 2.35.3

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- include/libvirt/libvirt-domain.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 5e858f854b..1ae8b1c77d 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -1618,6 +1618,17 @@ int virDomainRestoreParams (virConnectPtr conn, */ # define VIR_SAVE_PARAM_PARALLEL_CONNECTIONS "parallel.connections" +/** + * VIR_SAVE_PARAM_PARALLEL_COMPRESSION: + * + * this optional parameter is used in conjunction with the flag + * VIR_DOMAIN_SAVE_PARALLEL during save to ask the hypervisor for + * compressed channels to be used using this algorithm. + * + * Since: 8.4.0 + */ +# define VIR_SAVE_PARAM_PARALLEL_COMPRESSION "parallel.compression" + /* See below for virDomainSaveImageXMLFlags */ char * virDomainSaveImageGetXMLDesc (virConnectPtr conn, const char *file, -- 2.35.3

One thing I was thinking about, beyond what is in this patch, is that for the QEMU driver we could save the number of channels used for the --parallel save in the qemu SaveImage header as well. This way, on restore libvirt would automatically know how many file it needs to load, and the user does not need to know beforehand which --parallel-connections NN to use. Still the --parallel on resume would be needed in the current design, but that could theoretically be dropped as well. However, I wonder if that would create issues for other hypervisors implementing this, ie I presume keeping the --parallel flag would be the safest option. Thanks, Claudio On 5/6/22 3:11 PM, Claudio Fontana wrote:

Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_driver.c | 11 ++++++----- src/qemu/qemu_saveimage.c | 1 + src/qemu/qemu_saveimage.h | 1 + src/qemu/qemu_snapshot.c | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6b238c7677..473ec49bea 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2641,7 +2641,8 @@ static int qemuDomainSaveInternal(virQEMUDriver *driver, virDomainObj *vm, const char *path, int compressed, virCommand *compressor, - const char *xmlin, int nconn, unsigned int flags) + const char *xmlin, int nconn, const char *pcomp, + unsigned int flags) { g_autofree char *xml = NULL; bool was_running = false; @@ -2722,7 +2723,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, xml = NULL; ret = qemuSaveImageCreate(driver, vm, path, data, compressor, - nconn, flags, VIR_ASYNC_JOB_SAVE); + nconn, pcomp, flags, VIR_ASYNC_JOB_SAVE); if (ret < 0) goto endjob; @@ -2791,7 +2792,7 @@ qemuDomainSaveFlags(virDomainPtr dom, const char *path, const char *dxml, goto cleanup; ret = qemuDomainSaveInternal(driver, vm, path, compressed, - compressor, dxml, -1, flags); + compressor, dxml, -1, NULL, flags); cleanup: virDomainObjEndAPI(&vm); @@ -2854,7 +2855,7 @@ qemuDomainSaveParams(virDomainPtr dom, goto cleanup; ret = qemuDomainSaveInternal(driver, vm, to, compressed, - compressor, dxml, nconn, flags); + compressor, dxml, nconn, NULL, flags); cleanup: virDomainObjEndAPI(&vm); @@ -2911,7 +2912,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, -1, flags); + compressor, NULL, -1, NULL, flags); if (ret == 0) vm->hasManagedSave = true; diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index c0920c5baa..0187e47e28 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -481,6 +481,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, virQEMUSaveData *data, virCommand *compressor, int nconn, + const char *pcomp G_GNUC_UNUSED, unsigned int flags, virDomainAsyncJob asyncJob) { diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index eb0734101e..cae3b15882 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -132,6 +132,7 @@ qemuSaveImageCreate(virQEMUDriver *driver, virQEMUSaveData *data, virCommand *compressor, int nconn, + const char *pcomp, unsigned int flags, virDomainAsyncJob asyncJob); diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 626a5a14b9..daa72983b3 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, -1, 0, + data, compressor, -1, NULL, 0, VIR_ASYNC_JOB_SNAPSHOT)) < 0) goto cleanup; -- 2.35.3

change from static to external linkage, and move the function close to the other similar ones, near qemuMigrationParamsSetULL. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- src/qemu/qemu_migration_params.c | 47 +++++++++++++++----------------- src/qemu/qemu_migration_params.h | 5 ++++ 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 75b29e93a1..f6b9dc337d 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -900,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 @@ -1146,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 8b2d6ab210..23a4e0c8a2 100644 --- a/src/qemu/qemu_migration_params.h +++ b/src/qemu/qemu_migration_params.h @@ -138,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, -- 2.35.3

this completes the save side of the parallel compression support. Signed-off-by: Claudio Fontana <cfontana(a)suse.de&gt; --- docs/manpages/virsh.rst | 4 ++++ tools/virsh-domain.c | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst index 3bf9b54806..912b10d072 100644 --- a/docs/manpages/virsh.rst +++ b/docs/manpages/virsh.rst @@ -3813,6 +3813,7 @@ save save domain state-file [--bypass-cache] [--xml file] [--parallel] [--parallel-connections connections] + [--parallel-compression algo] [{--running | --paused}] [--verbose] Saves a paused or running domain (RAM, but not disk state) to one or more @@ -3853,6 +3854,9 @@ set using *--parallel-connections*, which will save to files named Parallel connections may help in speeding up the save operation. +*--parallel-compression* can be used to ask the hypervisor to provide +compressed channels in the save stream using algorithm ``algo``. + 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 ddedc21573..2236f44a22 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4182,6 +4182,10 @@ static const vshCmdOptDef opts_save[] = { .type = VSH_OT_INT, .help = N_("number of connections/files for parallel save") }, + {.name = "parallel-compression", + .type = VSH_OT_STRING, + .help = N_("compression algorithm and format for parallel save") + }, {.name = "xml", .type = VSH_OT_STRING, .completer = virshCompletePathLocalExisting, @@ -4211,6 +4215,7 @@ doSave(void *opaque) g_autoptr(virshDomain) dom = NULL; const char *name = NULL; const char *to = NULL; + const char *pcomp = NULL; virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; @@ -4247,6 +4252,13 @@ doSave(void *opaque) VIR_SAVE_PARAM_PARALLEL_CONNECTIONS, intOpt) < 0) goto out; } + if ((rv = vshCommandOptStringReq(ctl, cmd, "parallel-compression", &pcomp)) < 0) { + goto out; + } else { + if (virTypedParamsAddString(&params, &nparams, &maxparams, + VIR_SAVE_PARAM_PARALLEL_COMPRESSION, pcomp) < 0) + goto out; + } if (vshCommandOptBool(cmd, "running")) flags |= VIR_DOMAIN_SAVE_RUNNING; if (vshCommandOptBool(cmd, "paused")) -- 2.35.3