From: Peter Krempa <pkrempa@redhat.com> For socket activation to work our systemd unit files use the following pattern: [virtlogd.socket] <----(After)--- [virtlogd.service] [virtqemud.socket] <----(After)--- [virtqemud.service] Now the qemu daemon also wants to use the services provided by those daemons so we have dependency between the two too: [virtlogd.socket] <----(After)--- [virtlogd.service] ^ +-------------(After+Requires)-------+ | [virtqemud.socket] <----(After)--- [virtqemud.service] Now on startup everything is fine, because with socket activation, when 'virtqemud.service' wants to use 'virtlogd' services the socket is already up due to the dependency+ordering and opening a connection will cause 'virtlogd.service' to be socket-activated. On shutdown though there's no transitive 'After' ordering between 'virtqemud.service' and 'virtlogd.service' and thus nothing explicitly telling systemd that if virtlogd was started. In fact systemd is free to translate it that 'virtlogd' and 'virtqemud' need to be stopped before stopping 'virtlogd.socket'. To illustrate what happens consider the following scenario: A host is running a VM under virtqemud. 'virtqemud' is configured to attempt shutdown on the VMs before killing them (daemon-based guest shutdown, but the same reproduces also with libvirt-guests). The host is being rebooted. (virtqemud attempts to shut down guests, but guest takes more than the configured shutdown inhibition timeout, journald output follows): 06:44:02 fedora systemd-logind[664]: Delay lock is active (UID 0/root, PID 991/virtqemud) but inhibitor timeout is reached. 06:44:02 fedora systemd-logind[664]: System is rebooting. [...] 06:44:02 fedora virtlogd[802]: 802: debug : virSystemdNotify:667 : Notify 'STOPPING=1' 06:44:02 fedora systemd[1]: Stopping virtlogd.service - libvirt logging daemon... 06:44:02 fedora systemd[1]: Stopping virtqemud.service - libvirt QEMU daemon... 06:44:02 fedora virtqemud[991]: 991: debug : virSystemdNotify:667 : Notify 'STOPPING=1' 06:44:02 fedora systemd[1]: virtlogd.service: Deactivated successfully. 06:44:02 fedora systemd[1]: Stopped virtlogd.service - libvirt logging daemon. (the shutdown times out, virtqemud kills the unresponsive vm) 06:44:27 fedora virtqemud[991]: 1053: debug : qemuProcessStop:8916 : Shutting down vm=0x7f71ac032670 name=virt-vm1 id=1 pid=805, reason=destroyed, asyncJob=none, flags=0x0 06:44:27 fedora virtqemud[991]: 1053: debug : qemuDomainLogAppendMessage:5757 : Append log message (vm='virt-vm1' message='2026-06-15 10:44:27.427+0000: shutting down, reason=destroyed ) stdioLogD=1 06:44:27 fedora virtqemud[991]: 1053: error : virNetSocketReadWire:1767 : Cannot recv data: Connection reset by peer 06:44:27 fedora virtqemud[991]: 1053: debug : qemuProcessKill:8811 : vm=0x7f71ac032670 name=virt-vm1 pid=805 flags=0x5 Now the log shows that we want to add VM log file message in 'qemuDomainLogAppendMessage' but it fails because virtlogd is dead already. Now the same happens also with 'virtlockd' but with much worse outcome, especially if the configured action is to save the VMs because shutdown of 'virtlockd' when locks are held ends up 'fencing' the VMs by killing them. The same also happens when libvirt-guests is used to shutdown the guests instead. This patch adds an explicit 'After=virtlo[ck|g]d.service' to the daemons containing the qemu driver to ensure that the shutdown ordering makes sense. This doesn't break socket activation (e.g. the log/lock daemons are not started unless first invoked). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/virtqemud.service.extra.in | 4 ++++ src/remote/libvirtd.service.in | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/qemu/virtqemud.service.extra.in b/src/qemu/virtqemud.service.extra.in index cc16b6a9bb..3cc2edcfd0 100644 --- a/src/qemu/virtqemud.service.extra.in +++ b/src/qemu/virtqemud.service.extra.in @@ -6,6 +6,10 @@ Requires=virtlogd.socket Wants=virtlockd.socket After=virtlogd.socket After=virtlockd.socket +# To ensure that our helper daemons are not shut down before the main daemon +# shuts down we need also explicit ordering with the .service unit +After=virtlogd.service +After=virtlock.service Wants=systemd-machined.service After=systemd-machined.service After=remote-fs.target diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in index b0a062e885..f26494d646 100644 --- a/src/remote/libvirtd.service.in +++ b/src/remote/libvirtd.service.in @@ -15,6 +15,10 @@ Requires=virtlogd.socket Wants=virtlockd.socket After=virtlogd.socket After=virtlockd.socket +# To ensure that our helper daemons are not shut down before the main daemon +# shuts down we need also explicit ordering with the .service unit +After=virtlogd.service +After=virtlock.service Wants=systemd-machined.service After=network.target After=dbus.service -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Replace the implementation in 'virDomainLeaseInsert' by VIR_APPEND_ELEMENT and remove the unneeded helpers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 22 ++++------------------ src/conf/domain_conf.h | 4 +--- src/libvirt_private.syms | 2 -- 3 files changed, 5 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2d3e646bcb..6375eecffb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16158,25 +16158,11 @@ int virDomainLeaseIndex(virDomainDef *def, } -void virDomainLeaseInsertPreAlloc(virDomainDef *def) -{ - VIR_EXPAND_N(def->leases, def->nleases, 1); -} - -void virDomainLeaseInsert(virDomainDef *def, virDomainLeaseDef *lease) -{ - virDomainLeaseInsertPreAlloc(def); - virDomainLeaseInsertPreAlloced(def, lease); -} - - -void virDomainLeaseInsertPreAlloced(virDomainDef *def, - virDomainLeaseDef *lease) +void +virDomainLeaseInsert(virDomainDef *def, + virDomainLeaseDef *lease) { - if (lease == NULL) - VIR_SHRINK_N(def->leases, def->nleases, 1); - else - def->leases[def->nleases-1] = lease; + VIR_APPEND_ELEMENT(def->leases, def->nleases, lease); } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index d17f6352bd..3fec157a26 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -4200,9 +4200,7 @@ const char *virDomainControllerAliasFind(const virDomainDef *def, int virDomainLeaseIndex(virDomainDef *def, virDomainLeaseDef *lease); void virDomainLeaseInsert(virDomainDef *def, virDomainLeaseDef *lease); -void virDomainLeaseInsertPreAlloc(virDomainDef *def); -void virDomainLeaseInsertPreAlloced(virDomainDef *def, - virDomainLeaseDef *lease); + virDomainLeaseDef * virDomainLeaseRemoveAt(virDomainDef *def, size_t i); virDomainLeaseDef * diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 30c4564456..be073ced43 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -515,8 +515,6 @@ virDomainLaunchSecurityTypeToString; virDomainLeaseDefFree; virDomainLeaseIndex; virDomainLeaseInsert; -virDomainLeaseInsertPreAlloc; -virDomainLeaseInsertPreAlloced; virDomainLeaseRemove; virDomainLeaseRemoveAt; virDomainLifecycleActionTypeFromString; -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Historically the parser for the '<lease>' device type didn't require the presence of '<lockspace>', although the schema requires it. Commit 8d635a0bf24c refactored the parser and added a failure state if the fetching of '<lockspace>' fails, including the case when it's missing but didn't report an error, thus generating useless error message. Since the intention based on the docs, schema and implementation in the daemons seems to require 'lockspace' add a proper error message. Fixes: 8d635a0bf24cd1dabcbfc020b9a833a1d06d87a2 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6375eecffb..262aeca26c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7381,8 +7381,11 @@ virDomainLeaseDefParseXML(xmlNodePtr node, goto error; } - if (!(lockspace = virXPathString("string(./lockspace)", ctxt))) + if (!(lockspace = virXPathString("string(./lockspace)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Missing 'lockspace' element for lease")); goto error; + } if (!(targetNode = virXPathNode("./target", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Offset is optional in the schema, add an example in the tests. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxmlconfdata/lease.x86_64-latest.xml | 5 +++++ tests/qemuxmlconfdata/lease.xml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/qemuxmlconfdata/lease.x86_64-latest.xml b/tests/qemuxmlconfdata/lease.x86_64-latest.xml index d400fb1fef..5d231c30c7 100644 --- a/tests/qemuxmlconfdata/lease.x86_64-latest.xml +++ b/tests/qemuxmlconfdata/lease.x86_64-latest.xml @@ -42,6 +42,11 @@ <key>thequickbrownfoxjumpedoverthelazydog</key> <target path='/some/lease/path' offset='1024'/> </lease> + <lease> + <lockspace>else</lockspace> + <key>ble</key> + <target path='/some/lease/path2'/> + </lease> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <audio id='1' type='none'/> diff --git a/tests/qemuxmlconfdata/lease.xml b/tests/qemuxmlconfdata/lease.xml index a4f1d9728d..bf2eefcea5 100644 --- a/tests/qemuxmlconfdata/lease.xml +++ b/tests/qemuxmlconfdata/lease.xml @@ -33,6 +33,11 @@ <key>thequickbrownfoxjumpedoverthelazydog</key> <target path='/some/lease/path' offset='1024'/> </lease> + <lease> + <lockspace>else</lockspace> + <key>ble</key> + <target path='/some/lease/path2'/> + </lease> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <memballoon model='virtio'/> -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Log the individual values in the 'args' structs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/locking/lock_daemon_dispatch.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c index 22c450acd9..f82cfc494d 100644 --- a/src/locking/lock_daemon_dispatch.c +++ b/src/locking/lock_daemon_dispatch.c @@ -48,6 +48,9 @@ virLockSpaceProtocolDispatchAcquireResource(virNetServer *server G_GNUC_UNUSED, virLockSpace *lockspace; unsigned int newFlags; + VIR_DEBUG("args: path='%s', name='%s', flags=0x%x", + args->path, args->name, args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED | @@ -107,6 +110,9 @@ virLockSpaceProtocolDispatchCreateResource(virNetServer *server G_GNUC_UNUSED, virNetServerClientGetPrivateData(client); virLockSpace *lockspace; + VIR_DEBUG("args: path='%s', name='%s', flags=0x%x", + args->path, args->name, args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(0, cleanup); @@ -156,6 +162,9 @@ virLockSpaceProtocolDispatchDeleteResource(virNetServer *server G_GNUC_UNUSED, virNetServerClientGetPrivateData(client); virLockSpace *lockspace; + VIR_DEBUG("args: path='%s', name='%s', flags=0x%x", + args->path, args->name, args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(0, cleanup); @@ -205,6 +214,9 @@ virLockSpaceProtocolDispatchNew(virNetServer *server G_GNUC_UNUSED, virNetServerClientGetPrivateData(client); virLockSpace *lockspace; + VIR_DEBUG("args: path='%s', flags=0x%x", + args->path, args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(0, cleanup); @@ -260,6 +272,9 @@ virLockSpaceProtocolDispatchRegister(virNetServer *server G_GNUC_UNUSED, virLockDaemonClient *priv = virNetServerClientGetPrivateData(client); + VIR_DEBUG("args: owner.name='%s' owner.id=%d owner.pid=%lld flags=0x%x", + args->owner.name, args->owner.id, (long long) args->owner.pid, args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(0, cleanup); @@ -280,8 +295,6 @@ virLockSpaceProtocolDispatchRegister(virNetServer *server G_GNUC_UNUSED, memcpy(priv->ownerUUID, args->owner.uuid, VIR_UUID_BUFLEN); priv->ownerId = args->owner.id; priv->ownerPid = args->owner.pid; - VIR_DEBUG("ownerName=%s ownerId=%d ownerPid=%lld", - priv->ownerName, priv->ownerId, (unsigned long long)priv->ownerPid); rv = 0; @@ -306,6 +319,9 @@ virLockSpaceProtocolDispatchReleaseResource(virNetServer *server G_GNUC_UNUSED, virNetServerClientGetPrivateData(client); virLockSpace *lockspace; + VIR_DEBUG("args: path='%s', name='%s', flags=0x%x", + args->path, args->name, args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(0, cleanup); @@ -356,6 +372,8 @@ virLockSpaceProtocolDispatchRestrict(virNetServer *server G_GNUC_UNUSED, virLockDaemonClient *priv = virNetServerClientGetPrivateData(client); + VIR_DEBUG("args: flags=0x%x", args->flags); + g_mutex_lock(&priv->lock); virCheckFlagsGoto(0, cleanup); @@ -395,6 +413,8 @@ virLockSpaceProtocolDispatchCreateLockSpace(virNetServer *server G_GNUC_UNUSED, virNetServerClientGetPrivateData(client); virLockSpace *lockspace; + VIR_DEBUG("args: path='%s'", args->path); + g_mutex_lock(&priv->lock); if (priv->restricted) { -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Locking the <lease> on hotplug into a paused VM will prevent the VM from being resumed since the resource is already locked: $ virsh start cd --paused Domain 'cd' started $ virsh attach-device --live cd lease2.xml Device attached successfully $ virsh resume cd error: Failed to resume domain 'cd' error: resource busy: Lockspace resource 'qw2' is locked Don't lock the <lease>, just insert it when the VM is paused. Similarly don't try to unlock it on hot-unplug when the VM is paused and thus the resource is unlocked. Closes: https://gitlab.com/libvirt/libvirt/-/work_items/877 Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3d25b4c7d0..31726daf62 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3421,8 +3421,12 @@ qemuDomainAttachLease(virQEMUDriver *driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - if (virDomainLockLeaseAttach(driver->lockManager, cfg->uri, vm, lease) < 0) - return -1; + /* If the VM is paused or in another state we don't actually want to + * lock the lease yet. It will be done when the VM is resumed */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainLockLeaseAttach(driver->lockManager, cfg->uri, vm, lease) < 0) + return -1; + } virDomainLeaseInsert(vm->def, lease); @@ -6462,8 +6466,12 @@ qemuDomainDetachDeviceLease(virQEMUDriver *driver, return -1; } - if (virDomainLockLeaseDetach(driver->lockManager, vm, lease) < 0) - return -1; + /* If the VM is paused or in another state we don't actually want to + * release the lease because it's no longer locked. */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainLockLeaseDetach(driver->lockManager, vm, lease) < 0) + return -1; + } det_lease = virDomainLeaseRemoveAt(vm->def, idx); virDomainLeaseDefFree(det_lease); -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> The current design of the lock drivers holds the locks on the disk images only while the VM is running. There's no concept of 'read' locks. Block jobs can be used also when the VM is paused which is when the automatic locks are released. Allowing block jobs would either corrupt the data for anyone else holding the lock or the job could read garbage if another VM is using it in write mode. As of such there's no way libvirt can declare that block jobs are compatible with what we want to declare in regards to locking. Add a statement to the lock driver config that this is incompatible with blockjobs and forbid blockjobs when disk locks are in use. While the locking backend could be improved, there isn't enough justification to spend that amount of work required especially since now qemu has image locking which includes read locks. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/locking/lockd.conf | 4 ++++ src/locking/sanlock.conf | 4 ++++ src/qemu/qemu_backup.c | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_checkpoint.c | 2 +- src/qemu/qemu_domain.c | 9 ++++++++- src/qemu/qemu_domain.h | 3 ++- src/qemu/qemu_driver.c | 4 ++-- src/qemu/qemu_snapshot.c | 25 +++++++++++++++---------- 9 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf index ed7703ee06..a83be8b599 100644 --- a/src/locking/lockd.conf +++ b/src/locking/lockd.conf @@ -4,6 +4,10 @@ # application wishes to instead manually manage leases in # the guest XML, then this parameter can be disabled # +# Beware that block jobs (external disk snapshots, pull, +# commit, backup) do not work when automatic disk leases +# are enabled. +# #auto_disk_leases = 0 # diff --git a/src/locking/sanlock.conf b/src/locking/sanlock.conf index 3c356bef9c..743921f6c3 100644 --- a/src/locking/sanlock.conf +++ b/src/locking/sanlock.conf @@ -10,6 +10,10 @@ # # Uncomment this to enable automatic lease creation. # +# Beware that block jobs (external disk snapshots, pull, +# commit, backup) do not work when automatic disk leases +# are enabled. +# # NB: the 'host_id' parameter must be set if enabling this # #auto_disk_leases = 1 diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c index a0544c83dc..45f8544957 100644 --- a/src/qemu/qemu_backup.c +++ b/src/qemu/qemu_backup.c @@ -287,7 +287,7 @@ qemuBackupDiskPrepareDataOne(virDomainObj *vm, return -1; } - if (!qemuDomainDiskBlockJobIsSupported(dd->domdisk)) + if (!qemuDomainDiskBlockJobIsSupported(priv->driver, dd->domdisk)) return -1; if (dd->store->format == VIR_STORAGE_FILE_NONE) { diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 324c2635b4..ce957c2e05 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3686,7 +3686,7 @@ qemuBlockCommit(virDomainObj *vm, if (virDomainObjCheckActive(vm) < 0) return NULL; - if (!qemuDomainDiskBlockJobIsSupported(disk)) + if (!qemuDomainDiskBlockJobIsSupported(driver, disk)) return NULL; if (virStorageSourceIsEmpty(disk->src)) { diff --git a/src/qemu/qemu_checkpoint.c b/src/qemu/qemu_checkpoint.c index f063b5a5c0..705086d88e 100644 --- a/src/qemu/qemu_checkpoint.c +++ b/src/qemu/qemu_checkpoint.c @@ -371,7 +371,7 @@ qemuCheckpointPrepare(virQEMUDriver *driver, return -1; } - if (!qemuDomainDiskBlockJobIsSupported(vm->def->disks[i])) + if (!qemuDomainDiskBlockJobIsSupported(priv->driver, vm->def->disks[i])) return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2f1d220966..3751deb85d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10513,7 +10513,8 @@ qemuDomainDiskHasLatencyHistogram(virDomainDiskDef *disk) * Note that this does not verify whether other block jobs are running etc. */ bool -qemuDomainDiskBlockJobIsSupported(virDomainDiskDef *disk) +qemuDomainDiskBlockJobIsSupported(virQEMUDriver *driver, + virDomainDiskDef *disk) { if (qemuDiskBusIsSD(disk->bus)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, @@ -10536,6 +10537,12 @@ qemuDomainDiskBlockJobIsSupported(virDomainDiskDef *disk) return false; } + if (virLockManagerPluginAutoDiskLeases(driver->lockManager)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("block jobs are not supported when automatic disk locking is activated via sanlock/lockd")); + return false; + } + return true; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 24e62dd2e7..da1914979c 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1079,7 +1079,8 @@ bool qemuDomainDiskHasLatencyHistogram(virDomainDiskDef *disk); bool -qemuDomainDiskBlockJobIsSupported(virDomainDiskDef *disk); +qemuDomainDiskBlockJobIsSupported(virQEMUDriver *driver, + virDomainDiskDef *disk); int qemuDomainDefNumaCPUsRectify(virDomainDef *def, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 544955ecf9..d271853515 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14015,7 +14015,7 @@ qemuDomainBlockPullCommon(virDomainObj *vm, if (qemuDomainDiskBlockJobIsActive(disk)) goto endjob; - if (!qemuDomainDiskBlockJobIsSupported(disk)) + if (!qemuDomainDiskBlockJobIsSupported(priv->driver, disk)) goto endjob; if (base && @@ -14543,7 +14543,7 @@ qemuDomainBlockCopyCommon(virDomainObj *vm, if (qemuDomainDiskBlockJobIsActive(disk)) goto endjob; - if (!qemuDomainDiskBlockJobIsSupported(disk)) + if (!qemuDomainDiskBlockJobIsSupported(priv->driver, disk)) goto endjob; if (virStorageSourceIsFD(mirror)) { diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index 07548ca62e..988ecb965a 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -726,7 +726,8 @@ qemuSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDef *snapdisk, static int -qemuSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDef *snapdisk, +qemuSnapshotPrepareDiskExternalActive(virQEMUDriver *driver, + virDomainSnapshotDiskDef *snapdisk, virDomainDiskDef *domdisk) { virStorageType actualType = virStorageSourceGetActualType(snapdisk->src); @@ -740,7 +741,7 @@ qemuSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDef *snapdisk, return -1; } - if (!qemuDomainDiskBlockJobIsSupported(domdisk)) + if (!qemuDomainDiskBlockJobIsSupported(driver, domdisk)) return -1; switch (actualType) { @@ -771,7 +772,8 @@ qemuSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDef *snapdisk, static int -qemuSnapshotPrepareDiskExternal(virDomainDiskDef *disk, +qemuSnapshotPrepareDiskExternal(virQEMUDriver *driver, + virDomainDiskDef *disk, virDomainSnapshotDiskDef *snapdisk, bool active, bool reuse) @@ -805,7 +807,7 @@ qemuSnapshotPrepareDiskExternal(virDomainDiskDef *disk, if (qemuSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0) return -1; } else { - if (qemuSnapshotPrepareDiskExternalActive(snapdisk, disk) < 0) + if (qemuSnapshotPrepareDiskExternalActive(driver, snapdisk, disk) < 0) return -1; } @@ -937,6 +939,7 @@ qemuSnapshotPrepare(virDomainObj *vm, unsigned int *flags) { size_t i; + qemuDomainObjPrivate *priv = vm->privateData; bool active = virDomainObjIsActive(vm); bool reuse = (*flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; bool found_internal = false; @@ -977,7 +980,8 @@ qemuSnapshotPrepare(virDomainObj *vm, break; case VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL: - if (qemuSnapshotPrepareDiskExternal(dom_disk, disk, active, reuse) < 0) + if (qemuSnapshotPrepareDiskExternal(priv->driver, dom_disk, disk, + active, reuse) < 0) return -1; external++; @@ -2445,6 +2449,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, virDomainDef *inactiveConfig, qemuSnapshotRevertMemoryData *memdata) { + qemuDomainObjPrivate *priv = vm->privateData; size_t i; bool active = virDomainObjIsActive(vm); virDomainDef *domdef = NULL; @@ -2477,20 +2482,20 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, if (snapdisk->snapshot != VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) continue; - if (qemuSnapshotPrepareDiskExternal(domdisk, snapdisk, active, false) < 0) + if (qemuSnapshotPrepareDiskExternal(priv->driver, domdisk, snapdisk, + active, false) < 0) return -1; } if (memdata && snapdef->memorysnapshotfile) { - virQEMUDriver *driver = ((qemuDomainObjPrivate *) vm->privateData)->driver; g_autoptr(virDomainDef) savedef = NULL; memdata->path = snapdef->memorysnapshotfile; - if (qemuSaveImageGetMetadata(driver, NULL, memdata->path, NULL, NULL, + if (qemuSaveImageGetMetadata(priv->driver, NULL, memdata->path, NULL, NULL, &savedef, &memdata->data) < 0) return -1; - memdata->fd = qemuSaveImageOpen(driver, memdata->path, + memdata->fd = qemuSaveImageOpen(priv->driver, memdata->path, false, &memdata->wrapperFd, false); if (memdata->fd < 0) return -1; @@ -2500,7 +2505,7 @@ qemuSnapshotRevertExternalPrepare(virDomainObj *vm, if (qemuSaveImageFDSkipHeader(memdata->fd) < 0) return -1; - if (!virDomainDefCheckABIStability(savedef, domdef, driver->xmlopt)) + if (!virDomainDefCheckABIStability(savedef, domdef, priv->driver->xmlopt)) return -1; } -- 2.54.0