[PATCH 00/17] qemu: Fix regresion when loading internal snapshots and few cleanups
This series: 1) Fixes the regression in loading internal snapshots: https://gitlab.com/libvirt/libvirt/-/issues/771 2) Fixes bugs in cleanup paths of snapshot reversion where we'd keep an inactive transient VM definition in the domain list (Noticed when debugging the former issue) 3) Cleans up qemu commandline generator of unneeded arguments for snapshot reversion after recent removal of old code 4) Renames the argument used to revert internal snapshots to something more obvious. 5) Cleans some unneeded passing of the qemu driver struct Peter Krempa (17): qemuProcessStartWithMemoryState: Don't setup qemu for incoming migration when reverting internal snapshot NEWS: Mention fix for internal snapshot reversion regression qemuSnapshotRevertActive: Remove transient domain on failure qemuSnapshotRevertInactive: Ensure all error paths handle transient domains properly qemuBuildCommandLine: Drop 'snapshot' argument qemuProcessLaunch: Rename 'snapshot' to 'internalSnapshotRevert' qemuProcessStart: Rename 'snapshot' to 'internalSnapshotRevert' qemuProcessStartWithMemoryState: Rename 'snapshot' to 'internalSnapshotRevert' qemuExtDevicesCleanupHost: Use 'virQEMUDriverConfig' instead of 'virQEMUDriver' qemuCheckpointDiscardAllMetadata: Remove 'driver' argument qemuSnapshotDiscardAllMetadata: Remove 'driver' argument qemuDomainRemoveInactiveCommon: Remove 'driver' argument qemuProcessStop: Drop 'driver' argument qemuDomainRemoveInactiveLocked: Remove 'driver' argument qemuProcessReconnect: Modernize local variable setup qemuProcessReconnectData: Drop 'driver' struct and clean up qemuDomainRemoveInactive: Remove 'driver' argument NEWS.rst | 10 ++++ src/qemu/qemu_checkpoint.c | 5 +- src/qemu/qemu_checkpoint.h | 3 +- src/qemu/qemu_command.c | 5 +- src/qemu/qemu_command.h | 1 - src/qemu/qemu_domain.c | 25 +++++----- src/qemu/qemu_domain.h | 6 +-- src/qemu/qemu_driver.c | 46 ++++++++---------- src/qemu/qemu_extdevice.c | 18 +++---- src/qemu/qemu_extdevice.h | 4 +- src/qemu/qemu_migration.c | 20 ++++---- src/qemu/qemu_process.c | 98 +++++++++++++++++++------------------- src/qemu/qemu_process.h | 7 ++- src/qemu/qemu_saveimage.c | 2 +- src/qemu/qemu_snapshot.c | 75 ++++++++++++++--------------- src/qemu/qemu_snapshot.h | 3 +- src/qemu/qemu_tpm.c | 14 ++---- src/qemu/qemu_tpm.h | 4 +- 18 files changed, 167 insertions(+), 179 deletions(-) -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The memory/device state of the VM for an internal snapshot is restored by qemu itself via a QMP command and is taken from the qcow2 image, thus we don't actually do any form of incoming migration. Commit 5b324c0a739fe00 which refactored the setup of the incoming migration state didn't take the above into account and inadvertently caused that qemu is being started with '-incoming defer' also when libvirt would want to revert an internal snapshot. Now when qemu expects incoming migration it doesn't activate the block backends as that would cause locking problems and image inconsistency, but also doesn't allow the use of the images. Since the block backends are not activated qemu then thinks that they don't actually support internal snapshots and reports: error: operation failed: load of internal snapshot 'foo1' job failed: Device 'libvirt-1-format' is writable but does not support snapshots Due to the above bug it's not possible to revert to internal snapshots in libvirt-11.2 and libvirt-11.3. Fixes: 5b324c0a739fe00cbec209219db4488742492112 Resolves: https://issues.redhat.com/browse/RHEL-88747 Closes: https://gitlab.com/libvirt/libvirt/-/issues/771 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1af91c5909..5f2203dd13 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8636,9 +8636,13 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, /* The fd passed to qemuProcessIncomingDefNew is used to create the migration * URI, so it must be called after starting the decompression program. */ - incoming = qemuProcessIncomingDefNew(driver, vm, NULL, "stdio", fd, path, data, migParams); - if (!incoming) - return -1; + if (!snapshot) { + /* Internal snapshots are reverted by a QMP command after qemu is started, + * so we don't actually want to setup incoming migration. */ + if (!(incoming = qemuProcessIncomingDefNew(driver, vm, NULL, "stdio", + fd, path, data, migParams))) + return -1; + } /* No cookie means libvirt which saved the domain was too old to mess up * the CPU definitions. -- 2.49.0
On 5/15/25 09:28, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
The memory/device state of the VM for an internal snapshot is restored by qemu itself via a QMP command and is taken from the qcow2 image, thus we don't actually do any form of incoming migration.
Commit 5b324c0a739fe00 which refactored the setup of the incoming migration state didn't take the above into account and inadvertently caused that qemu is being started with '-incoming defer' also when libvirt would want to revert an internal snapshot.
Now when qemu expects incoming migration it doesn't activate the block backends as that would cause locking problems and image inconsistency, but also doesn't allow the use of the images. Since the block backends are not activated qemu then thinks that they don't actually support internal snapshots and reports:
error: operation failed: load of internal snapshot 'foo1' job failed: Device 'libvirt-1-format' is writable but does not support snapshots
Due to the above bug it's not possible to revert to internal snapshots in libvirt-11.2 and libvirt-11.3.
Fixes: 5b324c0a739fe00cbec209219db4488742492112 Resolves: https://issues.redhat.com/browse/RHEL-88747 Closes: https://gitlab.com/libvirt/libvirt/-/issues/771 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1af91c5909..5f2203dd13 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8636,9 +8636,13 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, /* The fd passed to qemuProcessIncomingDefNew is used to create the migration * URI, so it must be called after starting the decompression program. */ - incoming = qemuProcessIncomingDefNew(driver, vm, NULL, "stdio", fd, path, data, migParams); - if (!incoming) - return -1; + if (!snapshot) { + /* Internal snapshots are reverted by a QMP command after qemu is started, + * so we don't actually want to setup incoming migration. */ + if (!(incoming = qemuProcessIncomingDefNew(driver, vm, NULL, "stdio", + fd, path, data, migParams))) + return -1; + }
/* No cookie means libvirt which saved the domain was too old to mess up * the CPU definitions.
From: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index ad8910da4c..6cf2a47d3d 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,16 @@ v11.4.0 (unreleased) * **Bug fixes** + * qemu: Fix failure when reverting to internal snapshots + + Due to a regression in how libvirt starts qemu when reverting to a snapshot + it became impossible to revert to internal snapshots in ``libvirt-11.2`` and + ``libvirt-11.3`` releases. Attempts to revert would produce the following + error:: + + error: operation failed: load of internal snapshot 'foo1' job failed: Device 'libvirt-1-format' is writable but does not support snapshots + + There is no workaround for this except for avoiding the broken versions. v11.3.0 (2025-05-02) ==================== -- 2.49.0
On 5/15/25 09:28, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- NEWS.rst | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/NEWS.rst b/NEWS.rst index ad8910da4c..6cf2a47d3d 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -21,6 +21,16 @@ v11.4.0 (unreleased)
* **Bug fixes**
Thanks for fixing my bug! Below are some word-smithing suggestions you're free to ignore :-).
+ * qemu: Fix failure when reverting to internal snapshots + + Due to a regression in how libvirt starts qemu when reverting to a snapshot + it became impossible to revert to internal snapshots in ``libvirt-11.2`` and + ``libvirt-11.3`` releases. Attempts to revert would produce the following + error::
A regression in libvirt-11.2 and libvirt-11.3 prevents reverting to an internal snapshot. Attempts to revert would produce the following error:
+ + error: operation failed: load of internal snapshot 'foo1' job failed: Device 'libvirt-1-format' is writable but does not support snapshots + + There is no workaround for this except for avoiding the broken versions.
The only workaround is to avoid the broken versions. Either way Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
From: Peter Krempa <pkrempa@redhat.com> Code paths which deal with stopping of the qemu process need extra handling for transient definitions as they need to be removed from the domain list when we'd be leaving them inactive. In case of snapshot code it's on failure to revert a snapshot as we stop the qemu process but the failure to revert may mean that the new process will not be started. I've observed this when I was fixing the recent bug in snapshot reversion which left the domain in unusable state after failure to revert: $ virsh list foo error: Requested operation is not valid: domain is not running $ virsh undefine foo error: Requested operation is not valid: cannot undefine transient domain Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index f9b18f94b6..6926d1a0e4 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2628,13 +2628,12 @@ qemuSnapshotRevertActive(virDomainObj *vm, if (virDomainSnapshotIsExternal(snap)) { if (!(tmpsnapdef = virDomainSnapshotDefNew())) - return -1; + goto error; if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap, *config, *inactiveConfig, - &memdata) < 0) { - return -1; - } + &memdata) < 0) + goto error; } else { loadSnap = snap; } @@ -2656,7 +2655,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, VIR_ASYNC_JOB_SNAPSHOT, VIR_QEMU_PROCESS_STOP_MIGRATED); } - return -1; + goto error; } detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; @@ -2667,7 +2666,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, if (virDomainSnapshotIsExternal(snap)) { if (qemuSnapshotRevertExternalActive(vm, tmpsnapdef) < 0) - return -1; + goto error; qemuSnapshotRevertExternalFinish(vm, tmpsnapdef, snap); } @@ -2689,16 +2688,22 @@ qemuSnapshotRevertActive(virDomainObj *vm, if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("guest unexpectedly quit")); - return -1; + goto error; } rc = qemuProcessStartCPUs(driver, vm, VIR_DOMAIN_RUNNING_FROM_SNAPSHOT, VIR_ASYNC_JOB_SNAPSHOT); if (rc < 0) - return -1; + goto error; } return qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined); + + error: + if (!virDomainObjIsActive(vm)) + qemuDomainRemoveInactive(driver, vm, 0, false); + + return -1; } -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Only the internal snapshot code paths were able to handle transient domains properly in case when startup of the process failed. Unify the error paths on an 'error' label with proper handling. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_snapshot.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 6926d1a0e4..b66b83e230 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2773,24 +2773,20 @@ qemuSnapshotRevertInactive(virDomainObj *vm, if (virDomainSnapshotIsExternal(snap)) { if (!(tmpsnapdef = virDomainSnapshotDefNew())) - return -1; + goto error; if (qemuSnapshotRevertExternalPrepare(vm, tmpsnapdef, snap, - NULL, *inactiveConfig, NULL) < 0) { - return -1; - } + NULL, *inactiveConfig, NULL) < 0) + goto error; if (qemuSnapshotRevertExternalInactive(tmpsnapdef, - *inactiveConfig) < 0) { - return -1; - } + *inactiveConfig) < 0) + goto error; qemuSnapshotRevertExternalFinish(vm, tmpsnapdef, snap); } else { - if (qemuSnapshotInternalRevertInactive(vm, snap) < 0) { - qemuDomainRemoveInactive(driver, vm, 0, false); - return -1; - } + if (qemuSnapshotInternalRevertInactive(vm, snap) < 0) + goto error; } if (*inactiveConfig) { @@ -2810,10 +2806,9 @@ qemuSnapshotRevertInactive(virDomainObj *vm, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags); virDomainAuditStart(vm, "from-snapshot", rc >= 0); - if (rc < 0) { - qemuDomainRemoveInactive(driver, vm, 0, false); - return -1; - } + if (rc < 0) + goto error; + detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STARTED, @@ -2829,6 +2824,12 @@ qemuSnapshotRevertInactive(virDomainObj *vm, } return qemuSnapshotRevertWriteMetadata(vm, snap, driver, cfg, defined); + + error: + if (!virDomainObjIsActive(vm)) + qemuDomainRemoveInactive(driver, vm, 0, false); + + return -1; } -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> After recent refactors that removed legacy way to revert snapshots we no longer need to know the snapshot state during commandline build. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 5 ++--- src/qemu/qemu_command.h | 1 - src/qemu/qemu_process.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bf03561fde..1ee4aa4829 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10471,7 +10471,6 @@ qemuBuildCompatDeprecatedCommandLine(virCommand *cmd, virCommand * qemuBuildCommandLine(virDomainObj *vm, const char *migrateURI, - virDomainMomentObj *snapshot, virNetDevVPortProfileOp vmop, size_t *nnicindexes, int **nicindexes) @@ -10485,8 +10484,8 @@ qemuBuildCommandLine(virDomainObj *vm, virDomainDef *def = vm->def; virQEMUCaps *qemuCaps = priv->qemuCaps; - VIR_DEBUG("Building qemu commandline for def=%s(%p) migrateURI=%s snapshot=%p vmop=%d", - def->name, def, migrateURI, snapshot, vmop); + VIR_DEBUG("Building qemu commandline for def=%s(%p) migrateURI=%s vmop=%d", + def->name, def, migrateURI, vmop); if (qemuBuildCommandLineValidate(driver, def) < 0) return NULL; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index a386c34f20..2d43cf5506 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -43,7 +43,6 @@ VIR_ENUM_DECL(qemuSoundCodec); virCommand * qemuBuildCommandLine(virDomainObj *vm, const char *migrateURI, - virDomainMomentObj *snapshot, virNetDevVPortProfileOp vmop, size_t *nnicindexes, int **nicindexes); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5f2203dd13..46a33036b7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8046,7 +8046,7 @@ qemuProcessLaunch(virConnectPtr conn, if (!(cmd = qemuBuildCommandLine(vm, incoming ? "defer" : NULL, - snapshot, vmop, + vmop, &nnicindexes, &nicindexes))) goto cleanup; @@ -8712,7 +8712,6 @@ qemuProcessCreatePretendCmdBuild(virDomainObj *vm, { return qemuBuildCommandLine(vm, migrateURI, - NULL, VIR_NETDEV_VPORT_PROFILE_OP_NO_OP, NULL, NULL); -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Make it obvious that the variable is used for internal snapshot reversion by renaming it. This is necessary mainly as the function parameters are not documented, but makes it obvious also if they were. We can also report the name of the sanpshot rather than a pointer that says absolutely nothing to the reader. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 15 ++++++++------- src/qemu/qemu_process.h | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 46a33036b7..d86138aef4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7963,7 +7963,7 @@ qemuProcessLaunch(virConnectPtr conn, virDomainObj *vm, virDomainAsyncJob asyncJob, qemuProcessIncomingDef *incoming, - virDomainMomentObj *snapshot, + virDomainMomentObj *internalSnapshotRevert, virNetDevVPortProfileOp vmop, unsigned int flags) { @@ -7983,12 +7983,13 @@ qemuProcessLaunch(virConnectPtr conn, VIR_DEBUG("conn=%p driver=%p vm=%p name=%s id=%d asyncJob=%d " "incoming.uri=%s " "incoming.fd=%d incoming.path=%s " - "snapshot=%p vmop=%d flags=0x%x", + "internalSnapshotRevert='%s' vmop=%d flags=0x%x", conn, driver, vm, vm->def->name, vm->def->id, asyncJob, NULLSTR(incoming ? incoming->uri : NULL), incoming ? incoming->fd : -1, NULLSTR(incoming ? incoming->path : NULL), - snapshot, vmop, flags); + NULLSTR(internalSnapshotRevert ? internalSnapshotRevert->def->name : NULL), + vmop, flags); /* Okay, these are just internal flags, * but doesn't hurt to check */ @@ -8252,9 +8253,9 @@ qemuProcessLaunch(virConnectPtr conn, qemuDomainVcpuPersistOrder(vm->def); - if (snapshot) { + if (internalSnapshotRevert) { VIR_DEBUG("reverting internal snapshot via QMP"); - if (qemuSnapshotInternalRevert(vm, snapshot, asyncJob) < 0) + if (qemuSnapshotInternalRevert(vm, internalSnapshotRevert, asyncJob) < 0) goto cleanup; } @@ -8306,14 +8307,14 @@ qemuProcessLaunch(virConnectPtr conn, /* Since CPUs were not started yet, the balloon could not return the memory * to the host and thus cur_balloon needs to be updated so that GetXMLdesc * and friends return the correct size in case they can't grab the job */ - if (!incoming && !snapshot && + if (!incoming && !internalSnapshotRevert && qemuProcessRefreshBalloonState(vm, asyncJob) < 0) goto cleanup; if (flags & VIR_QEMU_PROCESS_START_AUTODESTROY) virCloseCallbacksDomainAdd(vm, conn, qemuProcessAutoDestroy); - if (!incoming && !snapshot) { + if (!incoming && !internalSnapshotRevert) { VIR_DEBUG("Setting up transient disk"); if (qemuProcessSetupDisksTransient(vm, asyncJob) < 0) goto cleanup; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 1729b39d73..021dbbd960 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -153,7 +153,7 @@ int qemuProcessLaunch(virConnectPtr conn, virDomainObj *vm, virDomainAsyncJob asyncJob, qemuProcessIncomingDef *incoming, - virDomainMomentObj *snapshot, + virDomainMomentObj *internalSnapshotRevert, virNetDevVPortProfileOp vmop, unsigned int flags); -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Make it obvious that the variable is used for internal snapshot reversion by renaming it. This is necessary mainly as the function parameters are not documented, but makes it obvious also if they were. We can also report the name of the sanpshot rather than a pointer that says absolutely nothing to the reader. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 11 ++++++----- src/qemu/qemu_process.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d86138aef4..f5543542d5 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8472,7 +8472,7 @@ qemuProcessStart(virConnectPtr conn, qemuProcessIncomingDef *incoming, int migrateFd, const char *migratePath, - virDomainMomentObj *snapshot, + virDomainMomentObj *internalSnapshotRevert, qemuMigrationParams *migParams, virNetDevVPortProfileOp vmop, unsigned int flags) @@ -8486,11 +8486,12 @@ qemuProcessStart(virConnectPtr conn, VIR_DEBUG("conn=%p driver=%p vm=%p name=%s id=%d asyncJob=%s " "incoming=%p migrateFd=%d migratePath=%s " - "snapshot=%p vmop=%d flags=0x%x", + "internalSnapshotRevert=%s vmop=%d flags=0x%x", conn, driver, vm, vm->def->name, vm->def->id, virDomainAsyncJobTypeToString(asyncJob), incoming, migrateFd, NULLSTR(migratePath), - snapshot, vmop, flags); + NULLSTR(internalSnapshotRevert ? internalSnapshotRevert->def->name : NULL), + vmop, flags); virCheckFlagsGoto(VIR_QEMU_PROCESS_START_COLD | VIR_QEMU_PROCESS_START_PAUSED | @@ -8498,7 +8499,7 @@ qemuProcessStart(virConnectPtr conn, VIR_QEMU_PROCESS_START_GEN_VMID | VIR_QEMU_PROCESS_START_RESET_NVRAM, cleanup); - if (!incoming && !snapshot) + if (!incoming && !internalSnapshotRevert) flags |= VIR_QEMU_PROCESS_START_NEW; if (qemuProcessInit(driver, vm, updatedCPU, @@ -8519,7 +8520,7 @@ qemuProcessStart(virConnectPtr conn, } if ((rv = qemuProcessLaunch(conn, driver, vm, asyncJob, incoming, - snapshot, vmop, flags)) < 0) { + internalSnapshotRevert, vmop, flags)) < 0) { if (rv == -2) relabel = true; goto stop; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 021dbbd960..6d5f189aa4 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -91,7 +91,7 @@ int qemuProcessStart(virConnectPtr conn, qemuProcessIncomingDef *incoming, int stdin_fd, const char *stdin_path, - virDomainMomentObj *snapshot, + virDomainMomentObj *internalSnapshotRevert, qemuMigrationParams *migParams, virNetDevVPortProfileOp vmop, unsigned int flags); -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Make it obvious that the variable is used for internal snapshot reversion by renaming it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f5543542d5..bed517184f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8580,7 +8580,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 + * @internalSnapshotRevert: internal snapshot to load when starting QEMU process or NULL * @data: data from memory state file or NULL * @migParams: Migration params to use on restore or NULL * @asyncJob: type of asynchronous job @@ -8591,11 +8591,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 caller needs to pass @snapshot + * When reverting to internal snapshot caller needs to pass @internalSnapshotRevert * to correctly start QEMU process, @fd, @path, @data needs to be NULL. * * When restoring VM from saved image caller needs to pass @fd, @path and - * @data to correctly start QEMU process, @snapshot needs to be NULL. + * @data to correctly start QEMU process, @internalSnapshotRevert needs to be NULL. * * For audit purposes the expected @reason is one of `restored` or `from-snapshot`. * @@ -8607,7 +8607,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, virDomainObj *vm, int *fd, const char *path, - virDomainMomentObj *snapshot, + virDomainMomentObj *internalSnapshotRevert, virQEMUSaveData *data, qemuMigrationParams *migParams, virDomainAsyncJob asyncJob, @@ -8638,7 +8638,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, /* The fd passed to qemuProcessIncomingDefNew is used to create the migration * URI, so it must be called after starting the decompression program. */ - if (!snapshot) { + if (!internalSnapshotRevert) { /* Internal snapshots are reverted by a QMP command after qemu is started, * so we don't actually want to setup incoming migration. */ if (!(incoming = qemuProcessIncomingDefNew(driver, vm, NULL, "stdio", @@ -8656,7 +8656,7 @@ qemuProcessStartWithMemoryState(virConnectPtr conn, priv->disableSlirp = true; if (qemuProcessStart(conn, driver, vm, cookie ? cookie->cpu : NULL, - asyncJob, incoming, *fd, path, snapshot, + asyncJob, incoming, *fd, path, internalSnapshotRevert, migParams, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE, start_flags) == 0) *started = true; -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Refactor the function and all callees to use the driver config instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_extdevice.c | 18 ++++++++++-------- src/qemu/qemu_extdevice.h | 4 ++-- src/qemu/qemu_process.c | 2 +- src/qemu/qemu_tpm.c | 14 +++++--------- src/qemu/qemu_tpm.h | 4 ++-- 6 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5603feaa05..235bfd65cb 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5816,7 +5816,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, if (rmdir(chkDir) < 0 && errno != ENOENT) VIR_WARN("unable to remove checkpoint directory %s", chkDir); } - qemuExtDevicesCleanupHost(driver, vm->def, flags, migration); + qemuExtDevicesCleanupHost(cfg, vm->def, flags, migration); } diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 31c7a14156..8df93a77ce 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -58,7 +58,7 @@ qemuExtDeviceLogCommand(virQEMUDriver *driver, /* * qemuExtDevicesInitPaths: * - * @driver: QEMU driver + * @cfg: QEMU driver config * @def: domain definition * * Initialize paths of external devices so that it is known where state is @@ -66,7 +66,7 @@ qemuExtDeviceLogCommand(virQEMUDriver *driver, * changes. */ int -qemuExtDevicesInitPaths(virQEMUDriver *driver, +qemuExtDevicesInitPaths(virQEMUDriverConfig *cfg, virDomainDef *def) { size_t i; @@ -75,7 +75,7 @@ qemuExtDevicesInitPaths(virQEMUDriver *driver, virDomainTPMDef *tpm = def->tpms[i]; if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR && - qemuExtTPMInitPaths(driver, def, tpm) < 0) + qemuExtTPMInitPaths(cfg, def, tpm) < 0) return -1; } @@ -95,10 +95,11 @@ int qemuExtDevicesPrepareDomain(virQEMUDriver *driver, virDomainObj *vm) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); int ret = 0; size_t i; - if (qemuExtDevicesInitPaths(driver, vm->def) < 0) + if (qemuExtDevicesInitPaths(cfg, vm->def) < 0) return -1; for (i = 0; i < vm->def->nvideos; i++) { @@ -151,21 +152,21 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver, void -qemuExtDevicesCleanupHost(virQEMUDriver *driver, +qemuExtDevicesCleanupHost(virQEMUDriverConfig *cfg, virDomainDef *def, virDomainUndefineFlagsValues flags, bool migration) { size_t i; - if (qemuExtDevicesInitPaths(driver, def) < 0) + if (qemuExtDevicesInitPaths(cfg, def) < 0) return; for (i = 0; i < def->ntpms; i++) { virDomainTPMDef *tpm = def->tpms[i]; if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) - qemuExtTPMCleanupHost(driver, tpm, flags, migration); + qemuExtTPMCleanupHost(cfg, tpm, flags, migration); } } @@ -282,10 +283,11 @@ qemuExtDevicesStop(virQEMUDriver *driver, virDomainObj *vm, bool migration) { + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); virDomainDef *def = vm->def; size_t i; - if (qemuExtDevicesInitPaths(driver, def) < 0) + if (qemuExtDevicesInitPaths(cfg, def) < 0) return; for (i = 0; i < def->nvideos; i++) { diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 36f7fb77a8..a8ed2aa7e4 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -31,7 +31,7 @@ int qemuExtDeviceLogCommand(virQEMUDriver *driver, G_GNUC_WARN_UNUSED_RESULT; int -qemuExtDevicesInitPaths(virQEMUDriver *driver, +qemuExtDevicesInitPaths(virQEMUDriverConfig *cfg, virDomainDef *def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; @@ -45,7 +45,7 @@ int qemuExtDevicesPrepareHost(virQEMUDriver *driver, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; -void qemuExtDevicesCleanupHost(virQEMUDriver *driver, +void qemuExtDevicesCleanupHost(virQEMUDriverConfig *cfg, virDomainDef *def, virDomainUndefineFlagsValues flags, bool migration) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index bed517184f..5636d4fb76 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9577,7 +9577,7 @@ qemuProcessReconnect(void *opaque) if (qemuDomainMasterKeyReadFile(priv) < 0) goto error; - if (qemuExtDevicesInitPaths(driver, obj->def) < 0) + if (qemuExtDevicesInitPaths(cfg, obj->def) < 0) goto error; /* If we are connecting to a guest started by old libvirt there is no diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 0d3be3971a..b2f76e6b8b 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -926,7 +926,7 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, /** * qemuTPMEmulatorCleanupHost: - * @driver: QEMU driver + * @cfg: QEMU driver config * @tpm: TPM definition * @flags: flags indicating whether to keep or remove TPM persistent state * @migration: whether cleanup is due to a successful outgoing or failed @@ -935,13 +935,11 @@ qemuTPMEmulatorInitPaths(virDomainTPMDef *tpm, * Clean up persistent storage for the swtpm. */ static void -qemuTPMEmulatorCleanupHost(virQEMUDriver *driver, +qemuTPMEmulatorCleanupHost(virQEMUDriverConfig *cfg, virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, bool migration) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - /* Never remove the state in case of migration with shared storage. */ if (migration && virFileIsSharedFS(tpm->data.emulator.source_path, cfg->sharedFilesystems) == 1) @@ -1275,12 +1273,10 @@ qemuTPMCanMigrateSharedStorage(virDomainDef *def) int -qemuExtTPMInitPaths(virQEMUDriver *driver, +qemuExtTPMInitPaths(virQEMUDriverConfig *cfg, virDomainDef *def, virDomainTPMDef *tpm) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - return qemuTPMEmulatorInitPaths(tpm, cfg->swtpmStorageDir, cfg->swtpmLogDir, @@ -1311,12 +1307,12 @@ qemuExtTPMPrepareHost(virQEMUDriver *driver, void -qemuExtTPMCleanupHost(virQEMUDriver *driver, +qemuExtTPMCleanupHost(virQEMUDriverConfig *cfg, virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, bool migration) { - qemuTPMEmulatorCleanupHost(driver, tpm, flags, migration); + qemuTPMEmulatorCleanupHost(cfg, tpm, flags, migration); } diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 37813087cf..f0f16392a1 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -22,7 +22,7 @@ #include "vircommand.h" -int qemuExtTPMInitPaths(virQEMUDriver *driver, +int qemuExtTPMInitPaths(virQEMUDriverConfig *cfg, virDomainDef *def, virDomainTPMDef *tpm) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) @@ -35,7 +35,7 @@ int qemuExtTPMPrepareHost(virQEMUDriver *driver, ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; -void qemuExtTPMCleanupHost(virQEMUDriver *driver, +void qemuExtTPMCleanupHost(virQEMUDriverConfig *cfg, virDomainTPMDef *tpm, virDomainUndefineFlagsValues flags, bool migration) -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The function can extract it from @vm. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_checkpoint.c | 5 ++--- src/qemu/qemu_checkpoint.h | 3 +-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index cf44e45aa1..af847cf1f2 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -305,11 +305,10 @@ qemuCheckpointDiscard(virQEMUDriver *driver, int -qemuCheckpointDiscardAllMetadata(virQEMUDriver *driver, - virDomainObj *vm) +qemuCheckpointDiscardAllMetadata(virDomainObj *vm) { virQEMUMomentRemove rem = { - .driver = driver, + .driver = QEMU_DOMAIN_PRIVATE(vm)->driver, .vm = vm, .metadata_only = true, .momentDiscard = qemuCheckpointDiscard, diff --git a/src/qemu/qemu_checkpoint.h b/src/qemu/qemu_checkpoint.h index fc1c35cdd2..2c72213598 100644 --- a/src/qemu/qemu_checkpoint.h +++ b/src/qemu/qemu_checkpoint.h @@ -33,8 +33,7 @@ qemuCheckpointObjFromName(virDomainObj *vm, const char *name); int -qemuCheckpointDiscardAllMetadata(virQEMUDriver *driver, - virDomainObj *vm); +qemuCheckpointDiscardAllMetadata(virDomainObj *vm); virDomainCheckpointPtr qemuCheckpointCreateXML(virDomainPtr domain, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 235bfd65cb..fe969ca3d3 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5807,7 +5807,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, VIR_WARN("unable to remove snapshot directory %s", snapDir); } /* Remove any checkpoint metadata prior to removing the domain */ - if (qemuCheckpointDiscardAllMetadata(driver, vm) < 0) { + if (qemuCheckpointDiscardAllMetadata(vm) < 0) { VIR_WARN("unable to remove all checkpoints for domain %s", vm->def->name); } else { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a34d6f1437..546545dd01 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6599,7 +6599,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, ncheckpoints); goto endjob; } - if (qemuCheckpointDiscardAllMetadata(driver, vm) < 0) + if (qemuCheckpointDiscardAllMetadata(vm) < 0) goto endjob; } -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The function can extract it from @vm. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_snapshot.c | 5 ++--- src/qemu/qemu_snapshot.h | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fe969ca3d3..fa1615ceba 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5797,7 +5797,7 @@ qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, g_autofree char *chkDir = NULL; /* Remove any snapshot metadata prior to removing the domain */ - if (qemuSnapshotDiscardAllMetadata(driver, vm) < 0) { + if (qemuSnapshotDiscardAllMetadata(vm) < 0) { VIR_WARN("unable to remove all snapshots for domain %s", vm->def->name); } else { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 546545dd01..6d43c2415c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6587,7 +6587,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, nsnapshots); goto endjob; } - if (qemuSnapshotDiscardAllMetadata(driver, vm) < 0) + if (qemuSnapshotDiscardAllMetadata(vm) < 0) goto endjob; } if (!virDomainObjIsActive(vm) && diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index b66b83e230..db5ba1eece 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -4135,11 +4135,10 @@ qemuSnapshotDiscard(virQEMUDriver *driver G_GNUC_UNUSED, int -qemuSnapshotDiscardAllMetadata(virQEMUDriver *driver, - virDomainObj *vm) +qemuSnapshotDiscardAllMetadata(virDomainObj *vm) { virQEMUMomentRemove rem = { - .driver = driver, + .driver = QEMU_DOMAIN_PRIVATE(vm)->driver, .vm = vm, .metadata_only = true, .momentDiscard = qemuSnapshotDiscard, diff --git a/src/qemu/qemu_snapshot.h b/src/qemu/qemu_snapshot.h index f38c2acfb3..2e5c3b5423 100644 --- a/src/qemu/qemu_snapshot.h +++ b/src/qemu/qemu_snapshot.h @@ -50,8 +50,7 @@ qemuSnapshotRevert(virDomainObj *vm, unsigned int flags); int -qemuSnapshotDiscardAllMetadata(virQEMUDriver *driver, - virDomainObj *vm); +qemuSnapshotDiscardAllMetadata(virDomainObj *vm); int qemuSnapshotDelete(virDomainObj *vm, -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The function can fetch it from @vm. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fa1615ceba..8ad7a57a85 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5787,12 +5787,11 @@ int qemuDomainMomentDiscardAll(void *payload, static void -qemuDomainRemoveInactiveCommon(virQEMUDriver *driver, - virDomainObj *vm, +qemuDomainRemoveInactiveCommon(virDomainObj *vm, virDomainUndefineFlagsValues flags, bool migration) { - g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(QEMU_DOMAIN_PRIVATE(vm)->driver); g_autofree char *snapDir = NULL; g_autofree char *chkDir = NULL; @@ -5836,7 +5835,7 @@ qemuDomainRemoveInactive(virQEMUDriver *driver, return; } - qemuDomainRemoveInactiveCommon(driver, vm, flags, migration); + qemuDomainRemoveInactiveCommon(vm, flags, migration); virDomainObjListRemove(driver->domains, vm); } @@ -5858,7 +5857,7 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, return; } - qemuDomainRemoveInactiveCommon(driver, vm, 0, false); + qemuDomainRemoveInactiveCommon(vm, 0, false); virDomainObjListRemoveLocked(driver->domains, vm); } -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> For now it's extracted as a temporary variable but in long term it ought to be eliminated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 22 ++++++++-------------- src/qemu/qemu_migration.c | 8 ++++---- src/qemu/qemu_process.c | 21 +++++++++++---------- src/qemu/qemu_process.h | 3 +-- src/qemu/qemu_saveimage.c | 2 +- src/qemu/qemu_snapshot.c | 18 ++++++------------ 6 files changed, 31 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6d43c2415c..50a5cdd594 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2057,8 +2057,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, if (vm->job->asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_DESTROYED, - VIR_ASYNC_JOB_NONE, stopFlags); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_DESTROYED, VIR_ASYNC_JOB_NONE, stopFlags); if ((flags & VIR_DOMAIN_DESTROY_REMOVE_LOGS) && qemuDomainRemoveLogs(driver, vm->def->name) < 0) @@ -2681,8 +2680,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, goto endjob; /* Shut it down */ - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SAVED, - VIR_ASYNC_JOB_SAVE, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_SAVED, VIR_ASYNC_JOB_SAVE, 0); virDomainAuditStop(vm, "saved"); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_SAVED); @@ -3212,8 +3210,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, endjob: if ((ret == 0) && (flags & VIR_DUMP_CRASH)) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED, - VIR_ASYNC_JOB_DUMP, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_CRASHED, VIR_ASYNC_JOB_DUMP, 0); virDomainAuditStop(vm, "crashed"); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, @@ -3535,8 +3532,7 @@ processGuestPanicEvent(virQEMUDriver *driver, G_GNUC_FALLTHROUGH; case VIR_DOMAIN_LIFECYCLE_ACTION_DESTROY: - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_CRASHED, - VIR_ASYNC_JOB_DUMP, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_CRASHED, VIR_ASYNC_JOB_DUMP, 0); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); @@ -3890,7 +3886,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, eventReason); - qemuProcessStop(driver, vm, stopReason, VIR_ASYNC_JOB_NONE, stopFlags); + qemuProcessStop(vm, stopReason, VIR_ASYNC_JOB_NONE, stopFlags); virDomainAuditStop(vm, auditReason); virObjectEventStateQueue(driver->domainEventState, event); @@ -4050,15 +4046,13 @@ processNbdkitExitedEvent(virDomainObj *vm, static void -processShutdownCompletedEvent(virQEMUDriver *driver, - virDomainObj *vm) +processShutdownCompletedEvent(virDomainObj *vm) { if (qemuProcessBeginStopJob(vm, VIR_JOB_DESTROY, true) < 0) return; if (virDomainObjIsActive(vm)) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, - VIR_ASYNC_JOB_NONE, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_UNKNOWN, VIR_ASYNC_JOB_NONE, 0); } qemuProcessEndStopJob(vm); @@ -4129,7 +4123,7 @@ static void qemuProcessEventHandler(void *data, void *opaque) processNbdkitExitedEvent(vm, processEvent->data); break; case QEMU_PROCESS_EVENT_SHUTDOWN_COMPLETED: - processShutdownCompletedEvent(driver, vm); + processShutdownCompletedEvent(vm); break; case QEMU_PROCESS_EVENT_LAST: break; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index bb4d74a65d..f38733f904 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3438,7 +3438,7 @@ qemuMigrationDstPrepareActive(virQEMUDriver *driver, if (!relabel) stopFlags |= VIR_QEMU_PROCESS_STOP_NO_RELABEL; virDomainAuditStart(vm, "migrated", false); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_MIGRATION_IN, stopFlags); virPortAllocatorRelease(priv->nbdPort); priv->nbdPort = 0; @@ -4082,7 +4082,7 @@ qemuMigrationSrcComplete(virQEMUDriver *driver, * up domain shutdown until SPICE server transfers its data */ qemuMigrationSrcWaitForSpice(vm); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED, asyncJob, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_MIGRATED, asyncJob, VIR_QEMU_PROCESS_STOP_MIGRATED); virDomainAuditStop(vm, "migrated"); @@ -6301,7 +6301,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, * confirm step. */ if (!v3proto) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_MIGRATED, VIR_ASYNC_JOB_MIGRATION_OUT, VIR_QEMU_PROCESS_STOP_MIGRATED); virDomainAuditStop(vm, "migrated"); @@ -6954,7 +6954,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, if (qemuDomainObjIsActive(vm)) { if (doKill) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_MIGRATION_IN, VIR_QEMU_PROCESS_STOP_MIGRATED); virDomainAuditStop(vm, "failed"); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5636d4fb76..1432ec28d7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5961,7 +5961,7 @@ qemuProcessInit(virQEMUDriver *driver, stopFlags = VIR_QEMU_PROCESS_STOP_NO_RELABEL; if (migration) stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob, stopFlags); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob, stopFlags); return -1; } @@ -8568,7 +8568,7 @@ qemuProcessStart(virConnectPtr conn, stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED; if (priv->mon) qemuMonitorSetDomainLog(priv->mon, NULL, NULL, NULL); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob, stopFlags); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob, stopFlags); goto cleanup; } @@ -8917,15 +8917,16 @@ qemuProcessEndStopJob(virDomainObj *vm) } -void qemuProcessStop(virQEMUDriver *driver, - virDomainObj *vm, - virDomainShutoffReason reason, - virDomainAsyncJob asyncJob, - unsigned int flags) +void +qemuProcessStop(virDomainObj *vm, + virDomainShutoffReason reason, + virDomainAsyncJob asyncJob, + unsigned int flags) { int ret; int retries = 0; qemuDomainObjPrivate *priv = vm->privateData; + virQEMUDriver *driver = priv->driver; virErrorPtr orig_err; virDomainDef *def = vm->def; size_t i; @@ -9264,7 +9265,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, if (qemuProcessBeginStopJob(dom, VIR_JOB_DESTROY, true) < 0) return; - qemuProcessStop(driver, dom, VIR_DOMAIN_SHUTOFF_DESTROYED, + qemuProcessStop(dom, VIR_DOMAIN_SHUTOFF_DESTROYED, VIR_ASYNC_JOB_NONE, stopFlags); virDomainAuditStop(dom, "destroyed"); @@ -9828,7 +9829,7 @@ qemuProcessReconnect(void *opaque) * thread didn't have a chance to start playing with the domain yet * (it's all we can do anyway). */ - qemuProcessStop(driver, obj, state, VIR_ASYNC_JOB_NONE, stopFlags); + qemuProcessStop(obj, state, VIR_ASYNC_JOB_NONE, stopFlags); } goto cleanup; } @@ -9868,7 +9869,7 @@ qemuProcessReconnectHelper(virDomainObj *obj, * is no thread that could be doing anything else with the same domain * object. */ - qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, + qemuProcessStop(obj, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_NONE, 0); qemuDomainRemoveInactiveLocked(src->driver, obj); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index 6d5f189aa4..b8c4af4aaf 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -176,8 +176,7 @@ int qemuProcessBeginStopJob(virDomainObj *vm, virDomainJob job, bool forceKill); void qemuProcessEndStopJob(virDomainObj *vm); -void qemuProcessStop(virQEMUDriver *driver, - virDomainObj *vm, +void qemuProcessStop(virDomainObj *vm, virDomainShutoffReason reason, virDomainAsyncJob asyncJob, unsigned int flags); diff --git a/src/qemu/qemu_saveimage.c b/src/qemu/qemu_saveimage.c index a6a5fb486e..702df641a0 100644 --- a/src/qemu/qemu_saveimage.c +++ b/src/qemu/qemu_saveimage.c @@ -773,7 +773,7 @@ qemuSaveImageStartVM(virConnectPtr conn, cleanup: if (ret < 0 && started) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob, VIR_QEMU_PROCESS_STOP_MIGRATED); } return ret; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index db5ba1eece..a17b976381 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -595,7 +595,7 @@ qemuSnapshotCreateActiveInternal(virQEMUDriver *driver, if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) { event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, VIR_ASYNC_JOB_SNAPSHOT, 0); virDomainAuditStop(vm, "from-snapshot"); resume = false; @@ -1694,8 +1694,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriver *driver, if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_HALT) { event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, - VIR_ASYNC_JOB_SNAPSHOT, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, VIR_ASYNC_JOB_SNAPSHOT, 0); virDomainAuditStop(vm, "from-snapshot"); resume = false; thaw = false; @@ -2615,9 +2614,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, /* Transitions 2, 3, 5, 6, 8, 9 */ if (virDomainObjIsActive(vm)) { /* Transitions 5, 6, 8, 9 */ - qemuProcessStop(driver, vm, - VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, - VIR_ASYNC_JOB_SNAPSHOT, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, VIR_ASYNC_JOB_SNAPSHOT, 0); virDomainAuditStop(vm, "from-snapshot"); detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, @@ -2651,7 +2648,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, start_flags, "from-snapshot", &started) < 0) { if (started) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_SNAPSHOT, VIR_QEMU_PROCESS_STOP_MIGRATED); } @@ -2761,8 +2758,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, if (virDomainObjIsActive(vm)) { /* Transitions 4, 7 */ - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, - VIR_ASYNC_JOB_SNAPSHOT, 0); + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT, VIR_ASYNC_JOB_SNAPSHOT, 0); virDomainAuditStop(vm, "from-snapshot"); detail = VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT; event = virDomainEventLifecycleNewFromObj(vm, @@ -4356,8 +4352,6 @@ qemuSnapshotDelete(virDomainObj *vm, virDomainMomentObj *snap = NULL; bool metadata_only = !!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY); bool stop_qemu = false; - qemuDomainObjPrivate *priv = vm->privateData; - virQEMUDriver *driver = priv->driver; g_autoslist(qemuSnapshotDeleteExternalData) externalData = NULL; virCheckFlags(VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | @@ -4393,7 +4387,7 @@ qemuSnapshotDelete(virDomainObj *vm, endjob: if (stop_qemu) { - qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN, + qemuProcessStop(vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN, VIR_ASYNC_JOB_SNAPSHOT, 0); } -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The function can extract the value from @vm's private data. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 5 ++--- src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_process.c | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8ad7a57a85..90258e9603 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5849,8 +5849,7 @@ qemuDomainRemoveInactive(virQEMUDriver *driver, * from locked list method. */ void -qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, - virDomainObj *vm) +qemuDomainRemoveInactiveLocked(virDomainObj *vm) { if (vm->persistent) { /* Short-circuit, we don't want to remove a persistent domain */ @@ -5859,7 +5858,7 @@ qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, qemuDomainRemoveInactiveCommon(vm, 0, false); - virDomainObjListRemoveLocked(driver->domains, vm); + virDomainObjListRemoveLocked(QEMU_DOMAIN_PRIVATE(vm)->driver->domains, vm); } void diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 8c1993ec64..56683848aa 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -697,8 +697,7 @@ void qemuDomainRemoveInactive(virQEMUDriver *driver, bool migration); void -qemuDomainRemoveInactiveLocked(virQEMUDriver *driver, - virDomainObj *vm); +qemuDomainRemoveInactiveLocked(virDomainObj *vm); void qemuDomainSetFakeReboot(virDomainObj *vm, bool value); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1432ec28d7..50c7b47256 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9871,7 +9871,7 @@ qemuProcessReconnectHelper(virDomainObj *obj, */ qemuProcessStop(obj, VIR_DOMAIN_SHUTOFF_FAILED, VIR_ASYNC_JOB_NONE, 0); - qemuDomainRemoveInactiveLocked(src->driver, obj); + qemuDomainRemoveInactiveLocked(obj); virDomainObjEndAPI(&obj); g_clear_object(&data->identity); -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Assign local variables directly and use autofree for temproary ones. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 50c7b47256..3bad805d3d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9537,16 +9537,16 @@ struct qemuProcessReconnectData { static void qemuProcessReconnect(void *opaque) { - struct qemuProcessReconnectData *data = opaque; - virQEMUDriver *driver = data->driver; + g_autofree struct qemuProcessReconnectData *data = opaque; virDomainObj *obj = data->obj; - qemuDomainObjPrivate *priv; + qemuDomainObjPrivate *priv = obj->privateData; + virQEMUDriver *driver = priv->driver; + g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_auto(virDomainJobObj) oldjob = { .cb = NULL, }; int state; int reason; - g_autoptr(virQEMUDriverConfig) cfg = NULL; size_t i; unsigned int stopFlags = 0; bool jobStarted = false; @@ -9554,10 +9554,6 @@ qemuProcessReconnect(void *opaque) virIdentitySetCurrent(data->identity); g_clear_object(&data->identity); - VIR_FREE(data); - - cfg = virQEMUDriverGetConfig(driver); - priv = obj->privateData; virDomainObjPreserveJob(obj->job, &oldjob); if (oldjob.asyncJob == VIR_ASYNC_JOB_MIGRATION_IN) -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> Nobody reads the struct member any more. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_process.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3bad805d3d..9d53768fed 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9512,7 +9512,6 @@ qemuProcessReloadMachineTypes(virDomainObj *vm) struct qemuProcessReconnectData { - virQEMUDriver *driver; virDomainObj *obj; virIdentity *identity; }; @@ -9832,10 +9831,9 @@ qemuProcessReconnect(void *opaque) static int qemuProcessReconnectHelper(virDomainObj *obj, - void *opaque) + void *opaque G_GNUC_UNUSED) { virThread thread; - struct qemuProcessReconnectData *src = opaque; struct qemuProcessReconnectData *data; g_autofree char *name = NULL; @@ -9845,7 +9843,6 @@ qemuProcessReconnectHelper(virDomainObj *obj, data = g_new0(struct qemuProcessReconnectData, 1); - memcpy(data, src, sizeof(*data)); data->obj = obj; data->identity = virIdentityGetCurrent(); @@ -9887,9 +9884,8 @@ qemuProcessReconnectHelper(virDomainObj *obj, void qemuProcessReconnectAll(virQEMUDriver *driver) { - struct qemuProcessReconnectData data = {.driver = driver}; virDomainObjListForEach(driver->domains, true, - qemuProcessReconnectHelper, &data); + qemuProcessReconnectHelper, NULL); } -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The function can fetch it from @vm. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 5 ++--- src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_driver.c | 20 ++++++++++---------- src/qemu/qemu_migration.c | 12 ++++++------ src/qemu/qemu_process.c | 4 ++-- src/qemu/qemu_snapshot.c | 4 ++-- 6 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 90258e9603..a7e4198316 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5825,8 +5825,7 @@ qemuDomainRemoveInactiveCommon(virDomainObj *vm, * The caller must hold a lock to the vm. */ void -qemuDomainRemoveInactive(virQEMUDriver *driver, - virDomainObj *vm, +qemuDomainRemoveInactive(virDomainObj *vm, virDomainUndefineFlagsValues flags, bool migration) { @@ -5837,7 +5836,7 @@ qemuDomainRemoveInactive(virQEMUDriver *driver, qemuDomainRemoveInactiveCommon(vm, flags, migration); - virDomainObjListRemove(driver->domains, vm); + virDomainObjListRemove(QEMU_DOMAIN_PRIVATE(vm)->driver->domains, vm); } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 56683848aa..ee6074c9f6 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -691,8 +691,7 @@ int qemuDomainMomentDiscardAll(void *payload, const char *name, void *data); -void qemuDomainRemoveInactive(virQEMUDriver *driver, - virDomainObj *vm, +void qemuDomainRemoveInactive(virDomainObj *vm, virDomainUndefineFlagsValues flags, bool migration); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 50a5cdd594..893fa0c66c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1577,7 +1577,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, goto cleanup; if (qemuProcessBeginJob(vm, VIR_DOMAIN_JOB_OPERATION_START, flags) < 0) { - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); goto cleanup; } @@ -1586,7 +1586,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, VIR_NETDEV_VPORT_PROFILE_OP_CREATE, start_flags) < 0) { virDomainAuditStart(vm, "booted", false); - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); qemuProcessEndJob(vm); goto cleanup; } @@ -2071,7 +2071,7 @@ qemuDomainDestroyFlags(virDomainPtr dom, ret = 0; endjob: if (ret == 0) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); qemuProcessEndStopJob(vm); cleanup: @@ -2707,7 +2707,7 @@ qemuDomainSaveInternal(virQEMUDriver *driver, } virDomainObjEndAsyncJob(vm); if (ret == 0) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); cleanup: virQEMUSaveDataFree(data); @@ -3243,7 +3243,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom, virDomainObjEndAsyncJob(vm); if (ret == 0 && flags & VIR_DUMP_CRASH) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); cleanup: virDomainObjEndAPI(&vm); @@ -3563,7 +3563,7 @@ processGuestPanicEvent(virQEMUDriver *driver, endjob: virDomainObjEndAsyncJob(vm); if (removeInactive) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); } @@ -3891,7 +3891,7 @@ processMonitorEOFEvent(virQEMUDriver *driver, virObjectEventStateQueue(driver->domainEventState, event); endjob: - qemuDomainRemoveInactive(driver, vm, 0, migration); + qemuDomainRemoveInactive(vm, 0, migration); qemuProcessEndStopJob(vm); } @@ -5853,7 +5853,7 @@ qemuDomainRestoreInternal(virConnectPtr conn, virFileWrapperFdFree(wrapperFd); virQEMUSaveDataFree(data); if (vm && ret < 0) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); virDomainObjEndAPI(&vm); return ret; } @@ -6505,7 +6505,7 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, } else { /* Brand new domain. Remove it */ VIR_INFO("Deleting domain '%s'", vm->def->name); - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); } } @@ -6648,7 +6648,7 @@ qemuDomainUndefineFlags(virDomainPtr dom, */ vm->persistent = 0; if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, flags, false); + qemuDomainRemoveInactive(vm, flags, false); ret = 0; endjob: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f38733f904..2a01ca58bd 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3609,7 +3609,7 @@ qemuMigrationDstPrepareFresh(virQEMUDriver *driver, * and there is no 'goto cleanup;' in the middle of those */ VIR_FREE(priv->origname); virDomainObjRemoveTransientDef(vm); - qemuDomainRemoveInactive(driver, vm, 0, true); + qemuDomainRemoveInactive(vm, 0, true); } virDomainObjEndAPI(&vm); virErrorRestore(&origErr); @@ -4256,7 +4256,7 @@ qemuMigrationSrcConfirm(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM, true); + qemuDomainRemoveInactive(vm, VIR_DOMAIN_UNDEFINE_TPM, true); } cleanup: @@ -6335,7 +6335,7 @@ qemuMigrationSrcPerformJob(virQEMUDriver *driver, virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); vm->persistent = 0; } - qemuDomainRemoveInactive(driver, vm, 0, true); + qemuDomainRemoveInactive(vm, 0, true); } virErrorRestore(&orig_err); @@ -6420,7 +6420,7 @@ qemuMigrationSrcPerformPhase(virQEMUDriver *driver, } if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0, true); + qemuDomainRemoveInactive(vm, 0, true); return ret; } @@ -6977,7 +6977,7 @@ qemuMigrationDstFinishActive(virQEMUDriver *driver, } if (!qemuDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, VIR_DOMAIN_UNDEFINE_TPM, true); + qemuDomainRemoveInactive(vm, VIR_DOMAIN_UNDEFINE_TPM, true); virErrorRestore(&orig_err); return NULL; @@ -7113,7 +7113,7 @@ qemuMigrationProcessUnattended(virQEMUDriver *driver, qemuMigrationJobFinish(vm); if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0, true); + qemuDomainRemoveInactive(vm, 0, true); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9d53768fed..ae54c807eb 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -9273,7 +9273,7 @@ qemuProcessAutoDestroy(virDomainObj *dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - qemuDomainRemoveInactive(driver, dom, 0, !!(stopFlags & VIR_QEMU_PROCESS_STOP_MIGRATED)); + qemuDomainRemoveInactive(dom, 0, !!(stopFlags & VIR_QEMU_PROCESS_STOP_MIGRATED)); qemuProcessEndStopJob(dom); @@ -9795,7 +9795,7 @@ qemuProcessReconnect(void *opaque) if (jobStarted) virDomainObjEndJob(obj); if (!virDomainObjIsActive(obj)) - qemuDomainRemoveInactive(driver, obj, 0, false); + qemuDomainRemoveInactive(obj, 0, false); virDomainObjEndAPI(&obj); virIdentitySetCurrent(NULL); return; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index a17b976381..8128154749 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -2698,7 +2698,7 @@ qemuSnapshotRevertActive(virDomainObj *vm, error: if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); return -1; } @@ -2823,7 +2823,7 @@ qemuSnapshotRevertInactive(virDomainObj *vm, error: if (!virDomainObjIsActive(vm)) - qemuDomainRemoveInactive(driver, vm, 0, false); + qemuDomainRemoveInactive(vm, 0, false); return -1; } -- 2.49.0
On 5/15/25 17:28, Peter Krempa via Devel wrote:
This series:
1) Fixes the regression in loading internal snapshots: https://gitlab.com/libvirt/libvirt/-/issues/771
2) Fixes bugs in cleanup paths of snapshot reversion where we'd keep an inactive transient VM definition in the domain list (Noticed when debugging the former issue)
3) Cleans up qemu commandline generator of unneeded arguments for snapshot reversion after recent removal of old code
4) Renames the argument used to revert internal snapshots to something more obvious.
5) Cleans some unneeded passing of the qemu driver struct
Peter Krempa (17): qemuProcessStartWithMemoryState: Don't setup qemu for incoming migration when reverting internal snapshot NEWS: Mention fix for internal snapshot reversion regression qemuSnapshotRevertActive: Remove transient domain on failure qemuSnapshotRevertInactive: Ensure all error paths handle transient domains properly qemuBuildCommandLine: Drop 'snapshot' argument qemuProcessLaunch: Rename 'snapshot' to 'internalSnapshotRevert' qemuProcessStart: Rename 'snapshot' to 'internalSnapshotRevert' qemuProcessStartWithMemoryState: Rename 'snapshot' to 'internalSnapshotRevert' qemuExtDevicesCleanupHost: Use 'virQEMUDriverConfig' instead of 'virQEMUDriver' qemuCheckpointDiscardAllMetadata: Remove 'driver' argument qemuSnapshotDiscardAllMetadata: Remove 'driver' argument qemuDomainRemoveInactiveCommon: Remove 'driver' argument qemuProcessStop: Drop 'driver' argument qemuDomainRemoveInactiveLocked: Remove 'driver' argument qemuProcessReconnect: Modernize local variable setup qemuProcessReconnectData: Drop 'driver' struct and clean up qemuDomainRemoveInactive: Remove 'driver' argument
NEWS.rst | 10 ++++ src/qemu/qemu_checkpoint.c | 5 +- src/qemu/qemu_checkpoint.h | 3 +- src/qemu/qemu_command.c | 5 +- src/qemu/qemu_command.h | 1 - src/qemu/qemu_domain.c | 25 +++++----- src/qemu/qemu_domain.h | 6 +-- src/qemu/qemu_driver.c | 46 ++++++++---------- src/qemu/qemu_extdevice.c | 18 +++---- src/qemu/qemu_extdevice.h | 4 +- src/qemu/qemu_migration.c | 20 ++++---- src/qemu/qemu_process.c | 98 +++++++++++++++++++------------------- src/qemu/qemu_process.h | 7 ++- src/qemu/qemu_saveimage.c | 2 +- src/qemu/qemu_snapshot.c | 75 ++++++++++++++--------------- src/qemu/qemu_snapshot.h | 3 +- src/qemu/qemu_tpm.c | 14 ++---- src/qemu/qemu_tpm.h | 4 +- 18 files changed, 167 insertions(+), 179 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (3)
-
Jim Fehlig -
Michal Prívozník -
Peter Krempa