[libvirt PATCH v3 00/10] external snapshot revert fixes
This fixes reverting external snapshots to not error out in cases where it should work and makes it correctly load the memory state when reverting to snapshot of running VM. This discards v2 completely and makes changes to v1: - moves qemuSaveImageStartProcess to qemu_process as qemuProcessStartWithMemoryState - change it to use cookie from memory state file instead of using cookie from snapshot xml - comments improvements - introduces new helpers to start and stop decompression process - reintroduces <externalSnapshot/> capability Pavel Hrdina (10): qemu_saveimage: extract starting process to qemuSaveImageStartProcess qemu_saveimage: introduce helpers to decompress memory state file qemu_saveimage: move qemuSaveImageStartProcess to qemu_process qemuProcessStartWithMemoryState: allow setting reason for audit log qemuProcessStartWithMemoryState: add snapshot argument qemuProcessStartWithMemoryState: make it possible to use without data qemu_snapshot: fix reverting external snapshot when not all disks are included qemu_snapshot: correctly load the saved memory state file capabilities: report full external snapshot support NEWS: document support for reverting external snapshots NEWS.rst | 8 + docs/formatcaps.rst | 7 + src/conf/capabilities.c | 1 + src/conf/capabilities.h | 1 + src/conf/schemas/capability.rng | 5 + src/qemu/qemu_capabilities.c | 1 + src/qemu/qemu_process.c | 95 +++++++++ src/qemu/qemu_process.h | 13 ++ src/qemu/qemu_saveimage.c | 189 +++++++++++------- src/qemu/qemu_saveimage.h | 15 ++ src/qemu/qemu_snapshot.c | 91 ++++++--- .../qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + 20 files changed, 328 insertions(+), 107 deletions(-) -- 2.41.0
Part of qemuSaveImageStartVM() function will be used when reverting external snapshots. To avoid duplicating code and logic extract the shared bits into separate function. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_saveimage.c | 96 +++++++++++++++++++++++++++------------ src/qemu/qemu_saveimage.h | 11 +++++ 2 files changed, 78 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 41310d6a9a..27b95b03ae 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -565,42 +565,50 @@ qemuSaveImageOpen(virQEMUDriver *driver, return ret; } +/** + * qemuSaveImageStartProcess: + * @conn: connection object + * @driver: qemu driver object + * @vm: domain object + * @fd: FD pointer of memory state file + * @path: path to memory state file + * @data: data from memory state file + * @asyncJob: type of asynchronous job + * @start_flags: flags to start QEMU process with + * @started: boolean to store if QEMU process was started + * + * Start VM with existing memory state. Make sure that the stored memory state + * is correctly decompressed so it can be loaded by QEMU process. + * + * Returns 0 on success, -1 on error. + */ int -qemuSaveImageStartVM(virConnectPtr conn, - virQEMUDriver *driver, - virDomainObj *vm, - int *fd, - virQEMUSaveData *data, - const char *path, - bool start_paused, - bool reset_nvram, - virDomainAsyncJob asyncJob) +qemuSaveImageStartProcess(virConnectPtr conn, + virQEMUDriver *driver, + virDomainObj *vm, + int *fd, + const char *path, + virQEMUSaveData *data, + virDomainAsyncJob asyncJob, + unsigned int start_flags, + bool *started) { qemuDomainObjPrivate *priv = vm->privateData; - int ret = -1; - bool started = false; - virObjectEvent *event; + g_autoptr(qemuDomainSaveCookie) cookie = NULL; + virQEMUSaveHeader *header = &data->header; VIR_AUTOCLOSE intermediatefd = -1; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - virQEMUSaveHeader *header = &data->header; - g_autoptr(qemuDomainSaveCookie) cookie = NULL; int rc = 0; - unsigned int start_flags = VIR_QEMU_PROCESS_START_PAUSED | - VIR_QEMU_PROCESS_START_GEN_VMID; - - if (reset_nvram) - start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM; if (virSaveCookieParseString(data->cookie, (virObject **)&cookie, virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) - goto cleanup; + return -1; if ((header->version == 2) && (header->compressed != QEMU_SAVE_FORMAT_RAW)) { if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed))) - goto cleanup; + return -1; intermediatefd = *fd; *fd = -1; @@ -613,7 +621,7 @@ qemuSaveImageStartVM(virConnectPtr conn, if (virCommandRunAsync(cmd, NULL) < 0) { *fd = intermediatefd; intermediatefd = -1; - goto cleanup; + return -1; } } @@ -622,7 +630,7 @@ qemuSaveImageStartVM(virConnectPtr conn, */ if (cookie && qemuDomainFixupCPUs(vm, &cookie->cpu) < 0) - goto cleanup; + return -1; if (cookie && !cookie->slirpHelper) priv->disableSlirp = true; @@ -631,12 +639,12 @@ qemuSaveImageStartVM(virConnectPtr conn, asyncJob, "stdio", *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, start_flags) == 0) - started = true; + *started = true; if (intermediatefd != -1) { virErrorPtr orig_err = NULL; - if (!started) { + if (!*started) { /* if there was an error setting up qemu, the intermediate * process will wait forever to write to stdout, so we * must manually kill it and ignore any error related to @@ -656,15 +664,45 @@ qemuSaveImageStartVM(virConnectPtr conn, rc = -1; } - virDomainAuditStart(vm, "restored", started); - if (!started || rc < 0) - goto cleanup; + virDomainAuditStart(vm, "restored", *started); + if (!*started || rc < 0) + return -1; /* qemuProcessStart doesn't unset the qemu error reporting infrastructure * in case of migration (which is used in this case) so we need to reset it * so that the handle to virtlogd is not held open unnecessarily */ qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL); + return 0; +} + +int +qemuSaveImageStartVM(virConnectPtr conn, + virQEMUDriver *driver, + virDomainObj *vm, + int *fd, + virQEMUSaveData *data, + const char *path, + bool start_paused, + bool reset_nvram, + virDomainAsyncJob asyncJob) +{ + int ret = -1; + bool started = false; + virObjectEvent *event; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + virQEMUSaveHeader *header = &data->header; + unsigned int start_flags = VIR_QEMU_PROCESS_START_PAUSED | + VIR_QEMU_PROCESS_START_GEN_VMID; + + if (reset_nvram) + start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM; + + if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, data, + asyncJob, start_flags, &started) < 0) { + goto cleanup; + } + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, VIR_DOMAIN_EVENT_STARTED_RESTORED); diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 30cf4b1ee0..24249ddf4c 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -57,6 +57,17 @@ qemuSaveImageUpdateDef(virQEMUDriver *driver, virDomainDef *def, const char *newxml); +int +qemuSaveImageStartProcess(virConnectPtr conn, + virQEMUDriver *driver, + virDomainObj *vm, + int *fd, + const char *path, + virQEMUSaveData *data, + virDomainAsyncJob asyncJob, + unsigned int start_flags, + bool *started); + int qemuSaveImageStartVM(virConnectPtr conn, virQEMUDriver *driver, -- 2.41.0
These new helpers separates the code from the logic used to start new QEMU process with memory state and will make it easier to move qemuSaveImageStartProcess() into qemu_process.c file. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_saveimage.c | 155 +++++++++++++++++++++++++++----------- src/qemu/qemu_saveimage.h | 15 ++++ 2 files changed, 128 insertions(+), 42 deletions(-) diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 27b95b03ae..95fabee907 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -247,6 +247,116 @@ qemuSaveImageGetCompressionCommand(virQEMUSaveFormat compression) } +/** + * qemuSaveImageDecompressionStart: + * @data: data from memory state file + * @fd: pointer to FD of memory state file + * @intermediatefd: pointer to FD to store original @fd + * @errbuf: error buffer for @retcmd + * @retcmd: new virCommand pointer + * + * Start process to decompress VM memory state from @fd. If decompression + * is needed the original FD is stored to @intermediatefd and new FD after + * decompression is stored to @fd so caller can use the same variable + * in both cases. + * + * Function qemuSaveImageDecompressionStop() needs to be used to correctly + * stop the process and swap FD to the original state. + * + * Caller is responsible for freeing @retcmd. + * + * Returns -1 on error, 0 on success. + */ +int +qemuSaveImageDecompressionStart(virQEMUSaveData *data, + int *fd, + int *intermediatefd, + char **errbuf, + virCommand **retcmd) +{ + virQEMUSaveHeader *header = &data->header; + g_autoptr(virCommand) cmd = NULL; + + if (header->version != 2) + return 0; + + if (header->compressed == QEMU_SAVE_FORMAT_RAW) + return 0; + + if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed))) + return -1; + + *intermediatefd = *fd; + *fd = -1; + + virCommandSetInputFD(cmd, *intermediatefd); + virCommandSetOutputFD(cmd, fd); + virCommandSetErrorBuffer(cmd, errbuf); + virCommandDoAsyncIO(cmd); + + if (virCommandRunAsync(cmd, NULL) < 0) { + *fd = *intermediatefd; + *intermediatefd = -1; + return -1; + } + + *retcmd = g_steal_pointer(&cmd); + return 0; +} + + +/** + * qemuSaveImageDecompressionStop: + * @cmd: virCommand pointer + * @fd: pointer to FD of memory state file + * @intermediatefd: pointer to FD to store original @fd + * @errbuf: error buffer for @cmd + * @started: boolean to indicate if QEMU process was started + * @path: path to the memory state file + * + * Stop decompression process and close both @fd and @intermediatefd if + * necessary. + * + * Returns -1 on errro, 0 on success. + */ +int +qemuSaveImageDecompressionStop(virCommand *cmd, + int *fd, + int *intermediatefd, + char *errbuf, + bool started, + const char *path) +{ + int rc = 0; + virErrorPtr orig_err = NULL; + + if (*intermediatefd == -1) + return rc; + + if (!started) { + /* if there was an error setting up qemu, the intermediate + * process will wait forever to write to stdout, so we + * must manually kill it and ignore any error related to + * the process + */ + virErrorPreserveLast(&orig_err); + VIR_FORCE_CLOSE(*intermediatefd); + VIR_FORCE_CLOSE(*fd); + } + + rc = virCommandWait(cmd, NULL); + VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); + virErrorRestore(&orig_err); + + if (VIR_CLOSE(*fd) < 0) { + virReportSystemError(errno, _("cannot close file: %1$s"), path); + rc = -1; + } + + return rc; +} + + /* Helper function to execute a migration to file with a correct save header * the caller needs to make sure that the processors are stopped and do all other * actions besides saving memory */ @@ -595,7 +705,6 @@ qemuSaveImageStartProcess(virConnectPtr conn, { qemuDomainObjPrivate *priv = vm->privateData; g_autoptr(qemuDomainSaveCookie) cookie = NULL; - virQEMUSaveHeader *header = &data->header; VIR_AUTOCLOSE intermediatefd = -1; g_autoptr(virCommand) cmd = NULL; g_autofree char *errbuf = NULL; @@ -605,25 +714,8 @@ qemuSaveImageStartProcess(virConnectPtr conn, virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) return -1; - if ((header->version == 2) && - (header->compressed != QEMU_SAVE_FORMAT_RAW)) { - if (!(cmd = qemuSaveImageGetCompressionCommand(header->compressed))) - return -1; - - intermediatefd = *fd; - *fd = -1; - - virCommandSetInputFD(cmd, intermediatefd); - virCommandSetOutputFD(cmd, fd); - virCommandSetErrorBuffer(cmd, &errbuf); - virCommandDoAsyncIO(cmd); - - if (virCommandRunAsync(cmd, NULL) < 0) { - *fd = intermediatefd; - intermediatefd = -1; - return -1; - } - } + if (qemuSaveImageDecompressionStart(data, fd, &intermediatefd, &errbuf, &cmd) < 0) + return -1; /* No cookie means libvirt which saved the domain was too old to mess up * the CPU definitions. @@ -641,28 +733,7 @@ qemuSaveImageStartProcess(virConnectPtr conn, start_flags) == 0) *started = true; - if (intermediatefd != -1) { - virErrorPtr orig_err = NULL; - - if (!*started) { - /* if there was an error setting up qemu, the intermediate - * process will wait forever to write to stdout, so we - * must manually kill it and ignore any error related to - * the process - */ - virErrorPreserveLast(&orig_err); - VIR_FORCE_CLOSE(intermediatefd); - VIR_FORCE_CLOSE(*fd); - } - - rc = virCommandWait(cmd, NULL); - VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); - virErrorRestore(&orig_err); - } - if (VIR_CLOSE(*fd) < 0) { - virReportSystemError(errno, _("cannot close file: %1$s"), path); - rc = -1; - } + rc = qemuSaveImageDecompressionStop(cmd, fd, &intermediatefd, errbuf, *started, path); virDomainAuditStart(vm, "restored", *started); if (!*started || rc < 0) diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index 24249ddf4c..dcee482066 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -99,6 +99,21 @@ qemuSaveImageGetCompressionProgram(const char *imageFormat, bool use_raw_on_fail) ATTRIBUTE_NONNULL(2); +int +qemuSaveImageDecompressionStart(virQEMUSaveData *data, + int *fd, + int *intermediatefd, + char **errbuf, + virCommand **retcmd); + +int +qemuSaveImageDecompressionStop(virCommand *cmd, + int *fd, + int *intermediatefd, + char *errbuf, + bool started, + const char *path); + int qemuSaveImageCreate(virQEMUDriver *driver, virDomainObj *vm, -- 2.41.0
The function will no longer be used only when restoring VM as it will be used when reverting snapshot as well so move it to qemu_process and rename it accordingly. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 73 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.h | 11 ++++++ src/qemu/qemu_saveimage.c | 75 ++------------------------------------- src/qemu/qemu_saveimage.h | 11 ------ 4 files changed, 86 insertions(+), 84 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 42837c4a8a..0de8ceb244 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8157,6 +8157,79 @@ qemuProcessStart(virConnectPtr conn, } +/** + * qemuProcessStartWithMemoryState: + * @conn: connection object + * @driver: qemu driver object + * @vm: domain object + * @fd: FD pointer of memory state file + * @path: path to memory state file + * @data: data from memory state file + * @asyncJob: type of asynchronous job + * @start_flags: flags to start QEMU process with + * @started: boolean to store if QEMU process was started + * + * Start VM with existing memory state. Make sure that the stored memory state + * is correctly decompressed so it can be loaded by QEMU process. + * + * Returns 0 on success, -1 on error. + */ +int +qemuProcessStartWithMemoryState(virConnectPtr conn, + virQEMUDriver *driver, + virDomainObj *vm, + int *fd, + const char *path, + virQEMUSaveData *data, + virDomainAsyncJob asyncJob, + unsigned int start_flags, + bool *started) +{ + qemuDomainObjPrivate *priv = vm->privateData; + g_autoptr(qemuDomainSaveCookie) cookie = NULL; + VIR_AUTOCLOSE intermediatefd = -1; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *errbuf = NULL; + int rc = 0; + + if (virSaveCookieParseString(data->cookie, (virObject **)&cookie, + virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) + return -1; + + if (qemuSaveImageDecompressionStart(data, fd, &intermediatefd, &errbuf, &cmd) < 0) + return -1; + + /* No cookie means libvirt which saved the domain was too old to mess up + * the CPU definitions. + */ + if (cookie && + qemuDomainFixupCPUs(vm, &cookie->cpu) < 0) + return -1; + + if (cookie && !cookie->slirpHelper) + priv->disableSlirp = true; + + if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, + asyncJob, "stdio", *fd, path, NULL, + VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, + start_flags) == 0) + *started = true; + + rc = qemuSaveImageDecompressionStop(cmd, fd, &intermediatefd, errbuf, *started, path); + + virDomainAuditStart(vm, "restored", *started); + if (!*started || rc < 0) + return -1; + + /* qemuProcessStart doesn't unset the qemu error reporting infrastructure + * in case of migration (which is used in this case) so we need to reset it + * so that the handle to virtlogd is not held open unnecessarily */ + qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL); + + return 0; +} + + int qemuProcessCreatePretendCmdPrepare(virQEMUDriver *driver, virDomainObj *vm, diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index cae1b49756..d758b4f51a 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -23,6 +23,7 @@ #include "qemu_conf.h" #include "qemu_domain.h" +#include "qemu_saveimage.h" #include "vireventthread.h" int qemuProcessPrepareMonitorChr(virDomainChrSourceDef *monConfig, @@ -90,6 +91,16 @@ int qemuProcessStart(virConnectPtr conn, virNetDevVPortProfileOp vmop, unsigned int flags); +int qemuProcessStartWithMemoryState(virConnectPtr conn, + virQEMUDriver *driver, + virDomainObj *vm, + int *fd, + const char *path, + virQEMUSaveData *data, + virDomainAsyncJob asyncJob, + unsigned int start_flags, + bool *started); + int qemuProcessCreatePretendCmdPrepare(virQEMUDriver *driver, virDomainObj *vm, const char *migrateURI, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 95fabee907..1fbc7891b1 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -675,77 +675,6 @@ qemuSaveImageOpen(virQEMUDriver *driver, return ret; } -/** - * qemuSaveImageStartProcess: - * @conn: connection object - * @driver: qemu driver object - * @vm: domain object - * @fd: FD pointer of memory state file - * @path: path to memory state file - * @data: data from memory state file - * @asyncJob: type of asynchronous job - * @start_flags: flags to start QEMU process with - * @started: boolean to store if QEMU process was started - * - * Start VM with existing memory state. Make sure that the stored memory state - * is correctly decompressed so it can be loaded by QEMU process. - * - * Returns 0 on success, -1 on error. - */ -int -qemuSaveImageStartProcess(virConnectPtr conn, - virQEMUDriver *driver, - virDomainObj *vm, - int *fd, - const char *path, - virQEMUSaveData *data, - virDomainAsyncJob asyncJob, - unsigned int start_flags, - bool *started) -{ - qemuDomainObjPrivate *priv = vm->privateData; - g_autoptr(qemuDomainSaveCookie) cookie = NULL; - VIR_AUTOCLOSE intermediatefd = -1; - g_autoptr(virCommand) cmd = NULL; - g_autofree char *errbuf = NULL; - int rc = 0; - - if (virSaveCookieParseString(data->cookie, (virObject **)&cookie, - virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) - return -1; - - if (qemuSaveImageDecompressionStart(data, fd, &intermediatefd, &errbuf, &cmd) < 0) - return -1; - - /* No cookie means libvirt which saved the domain was too old to mess up - * the CPU definitions. - */ - if (cookie && - qemuDomainFixupCPUs(vm, &cookie->cpu) < 0) - return -1; - - if (cookie && !cookie->slirpHelper) - priv->disableSlirp = true; - - if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, - asyncJob, "stdio", *fd, path, NULL, - VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, - start_flags) == 0) - *started = true; - - rc = qemuSaveImageDecompressionStop(cmd, fd, &intermediatefd, errbuf, *started, path); - - virDomainAuditStart(vm, "restored", *started); - if (!*started || rc < 0) - return -1; - - /* qemuProcessStart doesn't unset the qemu error reporting infrastructure - * in case of migration (which is used in this case) so we need to reset it - * so that the handle to virtlogd is not held open unnecessarily */ - qemuMonitorSetDomainLog(qemuDomainGetMonitor(vm), NULL, NULL, NULL); - - return 0; -} int qemuSaveImageStartVM(virConnectPtr conn, @@ -769,8 +698,8 @@ qemuSaveImageStartVM(virConnectPtr conn, if (reset_nvram) start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM; - if (qemuSaveImageStartProcess(conn, driver, vm, fd, path, data, - asyncJob, start_flags, &started) < 0) { + if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, data, + asyncJob, start_flags, &started) < 0) { goto cleanup; } diff --git a/src/qemu/qemu_saveimage.h b/src/qemu/qemu_saveimage.h index dcee482066..e541792153 100644 --- a/src/qemu/qemu_saveimage.h +++ b/src/qemu/qemu_saveimage.h @@ -57,17 +57,6 @@ qemuSaveImageUpdateDef(virQEMUDriver *driver, virDomainDef *def, const char *newxml); -int -qemuSaveImageStartProcess(virConnectPtr conn, - virQEMUDriver *driver, - virDomainObj *vm, - int *fd, - const char *path, - virQEMUSaveData *data, - virDomainAsyncJob asyncJob, - unsigned int start_flags, - bool *started); - int qemuSaveImageStartVM(virConnectPtr conn, virQEMUDriver *driver, -- 2.41.0
When called by snapshot code we will need to use different reason. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 6 +++++- src/qemu/qemu_process.h | 1 + src/qemu/qemu_saveimage.c | 3 ++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0de8ceb244..d414a40fd5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8167,11 +8167,14 @@ qemuProcessStart(virConnectPtr conn, * @data: data from memory state file * @asyncJob: type of asynchronous job * @start_flags: flags to start QEMU process with + * @reason: audit log reason * @started: boolean to store if QEMU process was started * * Start VM with existing memory state. Make sure that the stored memory state * is correctly decompressed so it can be loaded by QEMU process. * + * For audit purposes the expected @reason is one of `restored` or `from-snapshot`. + * * Returns 0 on success, -1 on error. */ int @@ -8183,6 +8186,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, virQEMUSaveData *data, virDomainAsyncJob asyncJob, unsigned int start_flags, + const char *reason, bool *started) { qemuDomainObjPrivate *priv = vm->privateData; @@ -8217,7 +8221,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, rc = qemuSaveImageDecompressionStop(cmd, fd, &intermediatefd, errbuf, *started, path); - virDomainAuditStart(vm, "restored", *started); + virDomainAuditStart(vm, reason, *started); if (!*started || rc < 0) return -1; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index d758b4f51a..26c9a04969 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -99,6 +99,7 @@ int qemuProcessStartWithMemoryState(virConnectPtr conn, virQEMUSaveData *data, virDomainAsyncJob asyncJob, unsigned int start_flags, + const char *reason, bool *started); int qemuProcessCreatePretendCmdPrepare(virQEMUDriver *driver, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 1fbc7891b1..92dcf4b616 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -699,7 +699,8 @@ qemuSaveImageStartVM(virConnectPtr conn, start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM; if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, data, - asyncJob, start_flags, &started) < 0) { + asyncJob, start_flags, "restored", + &started) < 0) { goto cleanup; } -- 2.41.0
When called from snapshot code we will need to pass snapshot object in order to make internal snapshots work correctly. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 9 ++++++++- src/qemu/qemu_process.h | 1 + src/qemu/qemu_saveimage.c | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d414a40fd5..c8430bf7b7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8164,6 +8164,7 @@ qemuProcessStart(virConnectPtr conn, * @vm: domain object * @fd: FD pointer of memory state file * @path: path to memory state file + * @snapshot: internal snapshot to load when starting QEMU process or NULL * @data: data from memory state file * @asyncJob: type of asynchronous job * @start_flags: flags to start QEMU process with @@ -8173,6 +8174,11 @@ qemuProcessStart(virConnectPtr conn, * Start VM with existing memory state. Make sure that the stored memory state * is correctly decompressed so it can be loaded by QEMU process. * + * When reverting to internal snapshot callers needs to pass @snapshot as well + * correctly start QEMU process. + * + * When restoring VM from saved image @snapshot needs to be NULL. + * * For audit purposes the expected @reason is one of `restored` or `from-snapshot`. * * Returns 0 on success, -1 on error. @@ -8183,6 +8189,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, virDomainObj *vm, int *fd, const char *path, + virDomainMomentObj *snapshot, virQEMUSaveData *data, virDomainAsyncJob asyncJob, unsigned int start_flags, @@ -8214,7 +8221,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, priv->disableSlirp = true; if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, - asyncJob, "stdio", *fd, path, NULL, + asyncJob, "stdio", *fd, path, snapshot, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, start_flags) == 0) *started = true; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 26c9a04969..ce2f25a2e7 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -96,6 +96,7 @@ int qemuProcessStartWithMemoryState(virConnectPtr conn, virDomainObj *vm, int *fd, const char *path, + virDomainMomentObj *snapshot, virQEMUSaveData *data, virDomainAsyncJob asyncJob, unsigned int start_flags, diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index 92dcf4b616..89112e3e44 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -698,7 +698,7 @@ qemuSaveImageStartVM(virConnectPtr conn, if (reset_nvram) start_flags |= VIR_QEMU_PROCESS_START_RESET_NVRAM; - if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, data, + if (qemuProcessStartWithMemoryState(conn, driver, vm, fd, path, NULL, data, asyncJob, start_flags, "restored", &started) < 0) { goto cleanup; -- 2.41.0
On Mon, Sep 18, 2023 at 15:29:22 +0200, Pavel Hrdina wrote:
When called from snapshot code we will need to pass snapshot object in order to make internal snapshots work correctly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 9 ++++++++- src/qemu/qemu_process.h | 1 + src/qemu/qemu_saveimage.c | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d414a40fd5..c8430bf7b7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8164,6 +8164,7 @@ qemuProcessStart(virConnectPtr conn, * @vm: domain object * @fd: FD pointer of memory state file * @path: path to memory state file + * @snapshot: internal snapshot to load when starting QEMU process or NULL * @data: data from memory state file * @asyncJob: type of asynchronous job * @start_flags: flags to start QEMU process with @@ -8173,6 +8174,11 @@ qemuProcessStart(virConnectPtr conn, * Start VM with existing memory state. Make sure that the stored memory state * is correctly decompressed so it can be loaded by QEMU process. * + * When reverting to internal snapshot callers needs to pass @snapshot as well + * correctly start QEMU process.
It doesn't mention that callers must not pass the other state-related parameters as both methods can't be used at once.
+ * + * When restoring VM from saved image @snapshot needs to be NULL.
This is only an implication (one way) that says that with a saveimage @snapshot must be null, but not that it's also the other way around.
When used with internal snapshots there is no memory state file so we have no data to load and decompression is not needed. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c8430bf7b7..f96918073f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8165,7 +8165,7 @@ qemuProcessStart(virConnectPtr conn, * @fd: FD pointer of memory state file * @path: path to memory state file * @snapshot: internal snapshot to load when starting QEMU process or NULL - * @data: data from memory state file + * @data: data from memory state file or NULL * @asyncJob: type of asynchronous job * @start_flags: flags to start QEMU process with * @reason: audit log reason @@ -8175,7 +8175,8 @@ qemuProcessStart(virConnectPtr conn, * is correctly decompressed so it can be loaded by QEMU process. * * When reverting to internal snapshot callers needs to pass @snapshot as well - * correctly start QEMU process. + * correctly start QEMU process. In addition there is no memory state file so + * it's safe to pass NULL as @data. * * When restoring VM from saved image @snapshot needs to be NULL. * @@ -8203,9 +8204,16 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, g_autofree char *errbuf = NULL; int rc = 0; - if (virSaveCookieParseString(data->cookie, (virObject **)&cookie, - virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) - return -1; + if (data) { + if (virSaveCookieParseString(data->cookie, (virObject **)&cookie, + virDomainXMLOptionGetSaveCookie(driver->xmlopt)) < 0) + return -1; + + if (qemuSaveImageDecompressionStart(data, fd, &intermediatefd, + &errbuf, &cmd) < 0) { + return -1; + } + } if (qemuSaveImageDecompressionStart(data, fd, &intermediatefd, &errbuf, &cmd) < 0) return -1; @@ -8226,7 +8234,10 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, start_flags) == 0) *started = true; - rc = qemuSaveImageDecompressionStop(cmd, fd, &intermediatefd, errbuf, *started, path); + if (data) { + rc = qemuSaveImageDecompressionStop(cmd, fd, &intermediatefd, errbuf, + *started, path); + } virDomainAuditStart(vm, reason, *started); if (!*started || rc < 0) -- 2.41.0
On Mon, Sep 18, 2023 at 15:29:23 +0200, Pavel Hrdina wrote:
When used with internal snapshots there is no memory state file so we have no data to load and decompression is not needed.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_process.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c8430bf7b7..f96918073f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8165,7 +8165,7 @@ qemuProcessStart(virConnectPtr conn, * @fd: FD pointer of memory state file * @path: path to memory state file * @snapshot: internal snapshot to load when starting QEMU process or NULL - * @data: data from memory state file + * @data: data from memory state file or NULL * @asyncJob: type of asynchronous job * @start_flags: flags to start QEMU process with * @reason: audit log reason @@ -8175,7 +8175,8 @@ qemuProcessStart(virConnectPtr conn, * is correctly decompressed so it can be loaded by QEMU process. * * When reverting to internal snapshot callers needs to pass @snapshot as well - * correctly start QEMU process. + * correctly start QEMU process. In addition there is no memory state file so + * it's safe to pass NULL as @data.
This does not fully address my comment from previous patch. It says that @data 'can' be NULL, not that it MUST be inull if @snapshot is non-null.
We need to skip all disks that have snapshot type other than 'external'. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_snapshot.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index cdc8e12cff..44bd97e564 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2054,6 +2054,9 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, virDomainSnapshotDiskDef *snapdisk = &tmpsnapdef->disks[i]; virDomainDiskDef *domdisk = domdef->disks[i]; + if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + continue; + if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false) < 0) return -1; } @@ -2094,6 +2097,9 @@ qemuSnapshotRevertExternalActive(virDomainObj *vm, return -1; for (i = 0; i < tmpsnapdef->ndisks; i++) { + if (tmpsnapdef->disks[i].snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + continue; + if (qemuSnapshotDiskPrepareOne(snapctxt, vm->def->disks[i], tmpsnapdef->disks + i, @@ -2184,6 +2190,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm, for (i = 0; i < curdef->nrevertdisks; i++) { virDomainSnapshotDiskDef *snapdisk = &(curdef->revertdisks[i]); + if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + continue; + if (virStorageSourceInit(snapdisk->src) < 0 || virStorageSourceUnlink(snapdisk->src) < 0) { VIR_WARN("Failed to remove snapshot image '%s'", @@ -2199,6 +2208,9 @@ qemuSnapshotRevertExternalFinish(virDomainObj *vm, for (i = 0; i < curdef->ndisks; i++) { virDomainSnapshotDiskDef *snapdisk = &(curdef->disks[i]); + if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) + continue; + if (virStorageSourceInit(snapdisk->src) < 0 || virStorageSourceUnlink(snapdisk->src) < 0) { VIR_WARN("Failed to remove snapshot image '%s'", -- 2.41.0
Original code assumed that the memory state file is only migration stream but it has additional metadata stored by libvirt. To correctly load the memory state file we need to reuse code that is used when restoring domain from saved image. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_snapshot.c | 79 ++++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 44bd97e564..e52d264826 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2000,6 +2000,21 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm, } +typedef struct _qemuSnapshotRevertMemoryData { + int fd; + char *path; + virQEMUSaveData *data; +} qemuSnapshotRevertMemoryData; + +static void +qemuSnapshotClearRevertMemoryData(qemuSnapshotRevertMemoryData *memdata) +{ + VIR_FORCE_CLOSE(memdata->fd); + virQEMUSaveDataFree(memdata->data); +} +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuSnapshotRevertMemoryData, qemuSnapshotClearRevertMemoryData); + + /** * qemuSnapshotRevertExternalPrepare: * @vm: domain object @@ -2007,15 +2022,13 @@ qemuSnapshotRevertWriteMetadata(virDomainObj *vm, * @snap: snapshot object we are reverting to * @config: live domain definition * @inactiveConfig: offline domain definition - * memsnapFD: pointer to store memory state file FD or NULL - * memsnapPath: pointer to store memory state file path or NULL + * @memdata: struct with data to load memory state * * Prepare new temporary snapshot definition @tmpsnapdef that will * be used while creating new overlay files after reverting to snapshot * @snap. In case we are reverting to snapshot with memory state it will - * open it and pass FD via @memsnapFD and path to the file via - * @memsnapPath, caller is responsible for freeing both @memsnapFD and - * memsnapPath. + * open it and store necessary data in @memdata. Caller is responsible + * to clear the data by using qemuSnapshotClearRevertMemoryData(). * * Returns 0 in success, -1 on error. */ @@ -2025,8 +2038,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, virDomainMomentObj *snap, virDomainDef *config, virDomainDef *inactiveConfig, - int *memsnapFD, - char **memsnapPath) + qemuSnapshotRevertMemoryData *memdata) { size_t i; bool active = virDomainObjIsActive(vm); @@ -2061,12 +2073,21 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, return -1; } - if (memsnapFD && memsnapPath && snapdef->memorysnapshotfile) { + if (memdata && snapdef->memorysnapshotfile) { virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver; - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virDomainDef) savedef = NULL; - *memsnapPath = snapdef->memorysnapshotfile; - *memsnapFD = qemuDomainOpenFile(cfg, NULL, *memsnapPath, O_RDONLY, NULL); + memdata->path = snapdef->memorysnapshotfile; + memdata->fd = qemuSaveImageOpen(driver, NULL, memdata->path, + &savedef, &memdata->data, + false, NULL, + false, false); + + if (memdata->fd < 0) + return -1; + + if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt)) + return -1; } return 0; @@ -2250,13 +2271,12 @@ qemuSnapshotRevertActive(virDomainObj *vm, virObjectEvent *event = NULL; virObjectEvent *event2 = NULL; virDomainMomentObj *loadSnap = NULL; - VIR_AUTOCLOSE memsnapFD = -1; - char *memsnapPath = NULL; int detail; bool defined = false; - qemuDomainSaveCookie *cookie = (qemuDomainSaveCookie *) snapdef->cookie; int rc; g_autoptr(virDomainSnapshotDef) tmpsnapdef = NULL; + g_auto(qemuSnapshotRevertMemoryData) memdata = { -1, NULL, NULL }; + bool started = false; start_flags |= VIR_QEMU_PROCESS_START_PAUSED; @@ -2280,7 +2300,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap, *config, *inactiveConfig, - &memsnapFD, &memsnapPath) < 0) { + &memdata) < 0) { return -1; } } else { @@ -2294,28 +2314,24 @@ qemuSnapshotRevertActive(virDomainObj *vm, virDomainObjAssignDef(vm, config, true, NULL); - /* No cookie means libvirt which saved the domain was too old to - * mess up the CPU definitions. - */ - if (cookie && - qemuDomainFixupCPUs(vm, &cookie->cpu) < 0) + if (qemuProcessStartWithMemoryState(snapshot->domain->conn, driver, vm, + &memdata.fd, memdata.path, loadSnap, + memdata.data, VIR_ASYNC_JOB_SNAPSHOT, + start_flags, "from-snapshot", + &started) < 0) { + if (started) { + qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + VIR_ASYNC_JOB_SNAPSHOT, + VIR_QEMU_PROCESS_STOP_MIGRATED); + } return -1; + } - rc = qemuProcessStart(snapshot->domain->conn, driver, vm, - cookie ? cookie->cpu : NULL, - VIR_ASYNC_JOB_SNAPSHOT, NULL, memsnapFD, - memsnapPath, loadSnap, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE, - start_flags); - virDomainAuditStart(vm, "from-snapshot", rc >= 0); detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, detail); virObjectEventStateQueue(driver->domainEventState, event); - if (rc < 0) - return -1; - if (virDomainSnapshotIsExternal(snap)) { if (qemuSnapshotRevertExternalActive(vm, tmpsnapdef) < 0) @@ -2423,8 +2439,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, return -1; if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap, - NULL, *inactiveConfig, - NULL, NULL) < 0) { + NULL, *inactiveConfig, NULL) < 0) { return -1; } -- 2.41.0
Now that deleting and reverting external snapshots is implemented we can report that in capabilities so management applications can use that information and start using external snapshots. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- docs/formatcaps.rst | 7 +++++++ src/conf/capabilities.c | 1 + src/conf/capabilities.h | 1 + src/conf/schemas/capability.rng | 5 +++++ src/qemu/qemu_capabilities.c | 1 + tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.aarch64.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc.xml | 1 + tests/qemucaps2xmloutdata/caps.ppc64.xml | 1 + tests/qemucaps2xmloutdata/caps.riscv64.xml | 1 + tests/qemucaps2xmloutdata/caps.s390x.xml | 1 + tests/qemucaps2xmloutdata/caps.sparc.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml | 1 + tests/qemucaps2xmloutdata/caps.x86_64.xml | 1 + 14 files changed, 24 insertions(+) diff --git a/docs/formatcaps.rst b/docs/formatcaps.rst index bb8bc663d2..95502c511f 100644 --- a/docs/formatcaps.rst +++ b/docs/formatcaps.rst @@ -134,6 +134,12 @@ The ``<guest/>`` element will typically wrap up the following elements: creating external disk snapshots is supported. If absent, creating external snapshots may still be supported, but it requires attempting the API and checking for an error to find out for sure. :since:`Since 1.2.3` + ``externalSnapshot`` + If this element is present, the hypervisor supports deleting and + reverting external snapshots including memory state. Support for creation + of external snapshots is reported via the ``disksnapshot`` feature flag. + Management applications can now switch from internal snapshots to external + snapshots. :since:`Since 9.7.0` Examples ~~~~~~~~ @@ -318,6 +324,7 @@ capabilities enabled in the chip and BIOS you will see: <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 56768ce6e0..34f04cb7d3 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -508,6 +508,7 @@ static const struct virCapsGuestFeatureInfo virCapsGuestFeatureInfos[VIR_CAPS_GU [VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false }, [VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true }, [VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true }, + [VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT] = { "externalSnapshot", false }, }; diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index c78e3e52fa..9eaf6e2807 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -40,6 +40,7 @@ typedef enum { VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT, VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT, VIR_CAPS_GUEST_FEATURE_TYPE_HAP, + VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT, VIR_CAPS_GUEST_FEATURE_TYPE_LAST } virCapsGuestFeatureType; diff --git a/src/conf/schemas/capability.rng b/src/conf/schemas/capability.rng index 83b414961a..b1968df258 100644 --- a/src/conf/schemas/capability.rng +++ b/src/conf/schemas/capability.rng @@ -493,6 +493,11 @@ <empty/> </element> </optional> + <optional> + <element name="externalSnapshot"> + <empty/> + </element> + </optional> </interleave> </element> </define> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 3a1bfbf74d..83119e871a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1122,6 +1122,7 @@ virQEMUCapsInitGuestFromBinary(virCaps *caps, virCapabilitiesAddGuestFeature(guest, VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT); virCapabilitiesAddGuestFeatureWithToggle(guest, VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT, true, false); + virCapabilitiesAddGuestFeature(guest, VIR_CAPS_GUEST_FEATURE_TYPE_EXTERNAL_SNAPSHOT); if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_TCG)) { virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU, diff --git a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml index b53a886b90..b9a5b5a1d6 100644 --- a/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml +++ b/tests/qemucaps2xmloutdata/caps.aarch64+hvf.xml @@ -21,6 +21,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml b/tests/qemucaps2xmloutdata/caps.aarch64.xml index 5dca6d3102..61512ed740 100644 --- a/tests/qemucaps2xmloutdata/caps.aarch64.xml +++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml @@ -21,6 +21,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.ppc.xml b/tests/qemucaps2xmloutdata/caps.ppc.xml index e7ba391795..86d6b85ae0 100644 --- a/tests/qemucaps2xmloutdata/caps.ppc.xml +++ b/tests/qemucaps2xmloutdata/caps.ppc.xml @@ -19,6 +19,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.ppc64.xml b/tests/qemucaps2xmloutdata/caps.ppc64.xml index 85623f3980..90859f9594 100644 --- a/tests/qemucaps2xmloutdata/caps.ppc64.xml +++ b/tests/qemucaps2xmloutdata/caps.ppc64.xml @@ -20,6 +20,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.riscv64.xml b/tests/qemucaps2xmloutdata/caps.riscv64.xml index 09b7eb7f2f..c6fa950211 100644 --- a/tests/qemucaps2xmloutdata/caps.riscv64.xml +++ b/tests/qemucaps2xmloutdata/caps.riscv64.xml @@ -19,6 +19,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.s390x.xml b/tests/qemucaps2xmloutdata/caps.s390x.xml index bb82a15040..6379ab73e6 100644 --- a/tests/qemucaps2xmloutdata/caps.s390x.xml +++ b/tests/qemucaps2xmloutdata/caps.s390x.xml @@ -20,6 +20,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.sparc.xml b/tests/qemucaps2xmloutdata/caps.sparc.xml index 9d977c4102..b5b158e430 100644 --- a/tests/qemucaps2xmloutdata/caps.sparc.xml +++ b/tests/qemucaps2xmloutdata/caps.sparc.xml @@ -19,6 +19,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml b/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml index 356819a6c6..f5e49ba4db 100644 --- a/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml +++ b/tests/qemucaps2xmloutdata/caps.x86_64+hvf.xml @@ -22,6 +22,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> diff --git a/tests/qemucaps2xmloutdata/caps.x86_64.xml b/tests/qemucaps2xmloutdata/caps.x86_64.xml index 35359780c4..8dd1439a80 100644 --- a/tests/qemucaps2xmloutdata/caps.x86_64.xml +++ b/tests/qemucaps2xmloutdata/caps.x86_64.xml @@ -22,6 +22,7 @@ <cpuselection/> <deviceboot/> <disksnapshot default='on' toggle='no'/> + <externalSnapshot/> </features> </guest> -- 2.41.0
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- NEWS.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 926620b03f..940e6e348a 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -23,6 +23,14 @@ v9.8.0 (unreleased) * **New features** + * QEMU: implement reverting external snapshots + + Reverting external snapshots is now possible using the existing API + ``virDomainSnapshotRevert()``. Management application can check host + capabilities for ``<externalSnapshot/>`` element within the list of + guest features to see if the current libvirt supports both deleting + and reverting external snapshots. + * **Improvements** * **Bug fixes** -- 2.41.0
On Mon, Sep 18, 2023 at 15:29:17 +0200, Pavel Hrdina wrote:
This fixes reverting external snapshots to not error out in cases where it should work and makes it correctly load the memory state when reverting to snapshot of running VM.
This discards v2 completely and makes changes to v1:
- moves qemuSaveImageStartProcess to qemu_process as qemuProcessStartWithMemoryState
- change it to use cookie from memory state file instead of using cookie from snapshot xml
- comments improvements
- introduces new helpers to start and stop decompression process
- reintroduces <externalSnapshot/> capability
Pavel Hrdina (10): qemu_saveimage: extract starting process to qemuSaveImageStartProcess qemu_saveimage: introduce helpers to decompress memory state file qemu_saveimage: move qemuSaveImageStartProcess to qemu_process qemuProcessStartWithMemoryState: allow setting reason for audit log qemuProcessStartWithMemoryState: add snapshot argument qemuProcessStartWithMemoryState: make it possible to use without data qemu_snapshot: fix reverting external snapshot when not all disks are included qemu_snapshot: correctly load the saved memory state file capabilities: report full external snapshot support NEWS: document support for reverting external snapshots
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Pavel Hrdina -
Peter Krempa