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> Avoid the 'virDomainLeaseInsertPreAlloc' + 'virDomainLeaseInsertPreAlloced' dance. It's now not needed since allocation failure will crash the daemon anyways. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 5be567b510..3d25b4c7d0 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3421,15 +3421,11 @@ qemuDomainAttachLease(virQEMUDriver *driver, { g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); - virDomainLeaseInsertPreAlloc(vm->def); - - if (virDomainLockLeaseAttach(driver->lockManager, cfg->uri, - vm, lease) < 0) { - virDomainLeaseInsertPreAlloced(vm->def, NULL); + if (virDomainLockLeaseAttach(driver->lockManager, cfg->uri, vm, lease) < 0) return -1; - } - virDomainLeaseInsertPreAlloced(vm->def, lease); + virDomainLeaseInsert(vm->def, lease); + return 0; } -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Clarify that libvirt will not do any locking on images if the 'lock_manager' field is unset and that qemu still will use the 'flock' based locks on images, which are actually not related to the setting. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu.conf.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in index 97b0141cf6..8ad30a909e 100644 --- a/src/qemu/qemu.conf.in +++ b/src/qemu/qemu.conf.in @@ -901,6 +901,11 @@ # virtlockd, is then our own implementation. Accepted values # are "sanlock" and "lockd". # +# NOTE: This lock mechanism is not related to QEMU's flock() +# based image locking. +# +# If unset no locking will happen on libvirt level. +# #lock_manager = "lockd" -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> The contents of the object we're parsing are freed so no need to have intermediate variables. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 262aeca26c..a9ee5b47d8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7366,22 +7366,19 @@ virDomainLeaseDefParseXML(xmlNodePtr node, xmlXPathContextPtr ctxt) { virDomainLeaseDef *def; - g_autofree char *lockspace = NULL; - g_autofree char *key = NULL; - g_autofree char *path = NULL; xmlNodePtr targetNode = NULL; VIR_XPATH_NODE_AUTORESTORE(ctxt) ctxt->node = node; def = g_new0(virDomainLeaseDef, 1); - if (!(key = virXPathString("string(./key)", ctxt))) { + if (!(def->key = virXPathString("string(./key)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing 'key' element for lease")); goto error; } - if (!(lockspace = virXPathString("string(./lockspace)", ctxt))) { + if (!(def->lockspace = virXPathString("string(./lockspace)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing 'lockspace' element for lease")); goto error; @@ -7393,7 +7390,7 @@ virDomainLeaseDefParseXML(xmlNodePtr node, goto error; } - if (!(path = virXMLPropString(targetNode, "path"))) { + if (!(def->path = virXMLPropString(targetNode, "path"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing 'path' attribute to 'target' element for lease")); goto error; @@ -7403,10 +7400,6 @@ virDomainLeaseDefParseXML(xmlNodePtr node, VIR_XML_PROP_NONE, &def->offset) < 0) goto error; - def->key = g_steal_pointer(&key); - def->lockspace = g_steal_pointer(&lockspace); - def->path = g_steal_pointer(&path); - return def; error: -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Add debug statement when adding a lockspace in the daemon and log the pointer when creating a new lockspace so that it can be inferred from the log what parameters were used. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/locking/lock_daemon.c | 3 +++ src/util/virlockspace.c | 6 ++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 1017fc65b1..2cb7e85b39 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -280,6 +280,9 @@ int virLockDaemonAddLockSpace(virLockDaemon *lockd, virLockSpace *lockspace) { int ret; + + VIR_DEBUG("path='%s' lockspace='%p'", NULLSTR(path), lockspace); + virLockDaemonLock(lockd); ret = virHashAddEntry(lockd->lockspaces, path, lockspace); virLockDaemonUnlock(lockd); diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 19aacf61bc..d3c95840b3 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -231,11 +231,9 @@ static void virLockSpaceResourceDataFree(void *opaque) virLockSpace *virLockSpaceNew(const char *directory) { - virLockSpace *lockspace; - - VIR_DEBUG("directory=%s", NULLSTR(directory)); + virLockSpace *lockspace = g_new0(virLockSpace, 1); - lockspace = g_new0(virLockSpace, 1); + VIR_DEBUG("lockspace=%p directory='%s'", lockspace, NULLSTR(directory)); if (virMutexInit(&lockspace->lock) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Add configuration option so additional lockspaces can be registered. These are useful to use manual <lease>, which can't be used on the default approach of registering the lease against a file. With this it's also now possible to use virtlockd in manual mode (disabling auto_disk_leases). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/locking/libvirt_lockd.aug | 8 ++++++++ src/locking/lock_driver_lockd.c | 13 +++++++++++++ src/locking/lockd.conf | 10 ++++++++++ src/locking/test_libvirt_lockd.aug.in | 4 ++++ 4 files changed, 35 insertions(+) diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug index 8cdb71a8a4..8f4fbb5713 100644 --- a/src/locking/libvirt_lockd.aug +++ b/src/locking/libvirt_lockd.aug @@ -7,13 +7,20 @@ module Libvirt_lockd = let value_sep = del /[ \t]*=[ \t]*/ " = " let indent = del /[ \t]*/ "" + let array_sep = del /,[ \t\n]*/ ", " + let array_start = del /\[[ \t\n]*/ "[ " + let array_end = del /\]/ "]" + let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\"" let bool_val = store /0|1/ let int_val = store /[0-9]+/ + let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" + let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end let str_entry (kw:string) = [ key kw . value_sep . str_val ] let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] let int_entry (kw:string) = [ key kw . value_sep . int_val ] + let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ] (* Each entry in the config is one of the following three ... *) @@ -22,6 +29,7 @@ module Libvirt_lockd = | str_entry "file_lockspace_dir" | str_entry "lvm_lockspace_dir" | str_entry "scsi_lockspace_dir" + | str_array_entry "extra_lockspace_dirs" let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index a3bb285eec..09149bded5 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -74,6 +74,8 @@ struct _virLockManagerLockDaemonDriver { char *fileLockSpaceDir; char *lvmLockSpaceDir; char *scsiLockSpaceDir; + + char **extraLockSpaceDirs; }; static virLockManagerLockDaemonDriver *driver; @@ -111,6 +113,10 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile) if (virConfGetValueBool(conf, "require_lease_for_disks", &driver->requireLeaseForDisks) < 0) return -1; + if (virConfGetValueStringList(conf, "extra_lockspace_dirs", false, + &driver->extraLockSpaceDirs) < 0) + return -1; + return 0; } @@ -288,6 +294,7 @@ static int virLockManagerLockDaemonInit(unsigned int version, const char *configFile, unsigned int flags) { + char **n; VIR_DEBUG("version=%u configFile=%s flags=0x%x", version, NULLSTR(configFile), flags); virCheckFlags(0, -1); @@ -317,6 +324,11 @@ static int virLockManagerLockDaemonInit(unsigned int version, goto error; } + for (n = driver->extraLockSpaceDirs; n && *n; n++) { + if (virLockManagerLockDaemonSetupLockspace(*n) < 0) + goto error; + } + return 0; error: @@ -332,6 +344,7 @@ static int virLockManagerLockDaemonDeinit(void) VIR_FREE(driver->scsiLockSpaceDir); VIR_FREE(driver->lvmLockSpaceDir); VIR_FREE(driver->fileLockSpaceDir); + g_strfreev(driver->extraLockSpaceDirs); VIR_FREE(driver); return 0; diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf index fa437604e6..ed7703ee06 100644 --- a/src/locking/lockd.conf +++ b/src/locking/lockd.conf @@ -65,3 +65,13 @@ # storage. # #scsi_lockspace_dir = "/var/lib/libvirt/lockd/scsivolumes" + + +# +# Additional lockspaces used for manual locks via the <lease> element. +# +# Typically these directories would be located on a shared +# filesystem visible to all hosts accessing the same +# storage. +# +#extra_lockspace_dirs = [ "/custom/lockspace1", "/custom/lockspace2" ] diff --git a/src/locking/test_libvirt_lockd.aug.in b/src/locking/test_libvirt_lockd.aug.in index 0f3b57eb34..249f765916 100644 --- a/src/locking/test_libvirt_lockd.aug.in +++ b/src/locking/test_libvirt_lockd.aug.in @@ -7,3 +7,7 @@ module Test_libvirt_lockd = { "file_lockspace_dir" = "/var/lib/libvirt/lockd/files" } { "lvm_lockspace_dir" = "/var/lib/libvirt/lockd/lvmvolumes" } { "scsi_lockspace_dir" = "/var/lib/libvirt/lockd/scsivolumes" } +{ "extra_lockspace_dirs" + { "1" = "/custom/lockspace1" } + { "2" = "/custom/lockspace2" } +} -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Blockjobs break when automatic disk locking is in use. Add an API to query whether that's the case so that we can forbid blockjobs. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/locking/lock_driver.h | 10 ++++++++++ src/locking/lock_driver_lockd.c | 9 +++++++++ src/locking/lock_driver_sanlock.c | 9 +++++++++ src/locking/lock_manager.c | 10 ++++++++++ src/locking/lock_manager.h | 2 ++ 6 files changed, 41 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index be073ced43..38375109f8 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1838,6 +1838,7 @@ virLockManagerAddResource; virLockManagerFree; virLockManagerInquire; virLockManagerNew; +virLockManagerPluginAutoDiskLeases; virLockManagerPluginGetName; virLockManagerPluginNew; virLockManagerPluginRef; diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h index 087a918882..ded6090033 100644 --- a/src/locking/lock_driver.h +++ b/src/locking/lock_driver.h @@ -281,6 +281,14 @@ typedef int (*virLockDriverInquire)(virLockManager *man, unsigned int flags); +/** + * virLockDriverAutoDiskLeases: + * + * Returns 'true' if automatic disk leases are in use. + */ +typedef bool (*virLockDriverAutoDiskLeases)(void); + + struct _virLockManager { virLockDriver *driver; void *privateData; @@ -309,4 +317,6 @@ struct _virLockDriver { virLockDriverAcquire drvAcquire; virLockDriverRelease drvRelease; virLockDriverInquire drvInquire; + + virLockDriverAutoDiskLeases drvAutoDiskLeases; }; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 09149bded5..0a03eeeada 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -775,6 +775,13 @@ static int virLockManagerLockDaemonInquire(virLockManager *lock G_GNUC_UNUSED, return 0; } + +static bool virLockManagerLockDaemonAutoDiskLease(void) +{ + return driver->autoDiskLease; +} + + virLockDriver virLockDriverImpl = { .version = VIR_LOCK_MANAGER_VERSION, @@ -792,4 +799,6 @@ virLockDriver virLockDriverImpl = .drvRelease = virLockManagerLockDaemonRelease, .drvInquire = virLockManagerLockDaemonInquire, + + .drvAutoDiskLeases = virLockManagerLockDaemonAutoDiskLease, }; diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index a07f3652c1..2e4cfaf548 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -1098,6 +1098,13 @@ static int virLockManagerSanlockInquire(virLockManager *lock, return 0; } + +static bool virLockManagerSandlocAutoDiskLease(void) +{ + return sanlockDriver->autoDiskLease; +} + + virLockDriver virLockDriverImpl = { .version = VIR_LOCK_MANAGER_VERSION, @@ -1115,4 +1122,6 @@ virLockDriver virLockDriverImpl = .drvAcquire = virLockManagerSanlockAcquire, .drvRelease = virLockManagerSanlockRelease, .drvInquire = virLockManagerSanlockInquire, + + .drvAutoDiskLeases = virLockManagerSandlocAutoDiskLease, }; diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index 6f5a7efdbc..70b1c6c286 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -277,6 +277,16 @@ virLockDriver *virLockManagerPluginGetDriver(virLockManagerPlugin *plugin) return plugin->driver; } +bool virLockManagerPluginAutoDiskLeases(virLockManagerPlugin *plugin) +{ + VIR_DEBUG("plugin=%p", plugin); + + if (!plugin->driver->drvAutoDiskLeases()) + return false; + + return plugin->driver->drvAutoDiskLeases(); +} + /** * virLockManagerNew: * @driver: the lock manager implementation to use diff --git a/src/locking/lock_manager.h b/src/locking/lock_manager.h index 774931f7d6..814a8cabde 100644 --- a/src/locking/lock_manager.h +++ b/src/locking/lock_manager.h @@ -38,6 +38,8 @@ bool virLockManagerPluginUsesState(virLockManagerPlugin *plugin); virLockDriver *virLockManagerPluginGetDriver(virLockManagerPlugin *plugin); +bool virLockManagerPluginAutoDiskLeases(virLockManagerPlugin *plugin); + virLockManager *virLockManagerNew(virLockDriver *driver, unsigned int type, size_t nparams, -- 2.54.0

From: Peter Krempa <pkrempa@redhat.com> Since we only do image locking on disk level 'qemuDomainStorageSourceAccessModify' is not the correct place as that is used also from various places which partially modify the image state e.g. with blockjobs. Move the locking code only to disk hot(un)plug code paths and directly apply the same fix as with <lease> where we don't acquire the lock if the VM isn't running. Locking is then handled when resuming the VM. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 12 ------------ src/qemu/qemu_hotplug.c | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 3751deb85d..f6e36acb0f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6736,7 +6736,6 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver, bool revoke_label = false; bool revoke_namespace = false; bool revoke_nvme = false; - bool revoke_lockspace = false; bool revoke_nbdkit = false; VIR_DEBUG("src='%s' readonly=%d force_ro=%d force_rw=%d revoke=%d chain=%d", @@ -6755,17 +6754,11 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver, revoke_label = true; revoke_namespace = true; revoke_nvme = true; - revoke_lockspace = true; revoke_nbdkit = true; ret = 0; goto revoke; } - if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, src) < 0) - goto revoke; - - revoke_lockspace = true; - if (!(flags & QEMU_DOMAIN_STORAGE_SOURCE_ACCESS_MODIFY_ACCESS)) { if (qemuDomainStorageSourceAccessModifyNVMe(driver, vm, src, false) < 0) goto revoke; @@ -6826,11 +6819,6 @@ qemuDomainStorageSourceAccessModify(virQEMUDriver *driver, if (revoke_nvme) qemuDomainStorageSourceAccessModifyNVMe(driver, vm, src, true); - if (revoke_lockspace) { - if (virDomainLockImageDetach(driver->lockManager, vm, src) < 0) - VIR_WARN("Unable to release lock on %s", srcstr); - } - if (revoke_nbdkit) qemuNbdkitStopStorageSource(src, vm, chain); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 31726daf62..64226438fa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -612,6 +612,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, qemuDomainObjPrivate *priv = vm->privateData; virStorageSource *oldsrc = disk->src; qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); + bool locked = false; int rc; if (diskPriv->blockjob && qemuBlockJobIsRunning(diskPriv->blockjob)) { @@ -634,6 +635,15 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, if (qemuDomainStorageSourceChainAccessAllow(driver, vm, newsrc) < 0) goto rollback; + /* If the VM is paused or in another state we don't actually want to + * lock the image yet. It will be done when the VM is resumed */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, newsrc) < 0) + goto rollback; + + locked = true; + } + if (qemuHotplugAttachManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE) < 0) goto rollback; @@ -655,6 +665,9 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, rollback: ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, newsrc)); + if (locked) + ignore_value(virDomainLockImageDetach(driver->lockManager, vm, newsrc)); + qemuHotplugRemoveManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE); /* revert old image do the disk definition */ @@ -982,6 +995,7 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, bool releaseUSB = false; bool releaseVirtio = false; bool releaseSeclabel = false; + bool releaseLock = false; int ret = -1; if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { @@ -1083,6 +1097,15 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, releaseSeclabel = true; + /* If the VM is paused or in another state we don't actually want to + * lock the image yet. It will be done when the VM is resumed */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + if (virDomainLockImageAttach(driver->lockManager, cfg->uri, vm, disk->src) < 0) + goto cleanup; + + releaseLock = true; + } + if (qemuProcessPrepareHostStorageDisk(vm, disk) < 0) goto cleanup; @@ -1110,6 +1133,9 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, if (releaseSeclabel) ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src)); + if (releaseLock) + ignore_value(virDomainLockImageDetach(driver->lockManager, vm, disk->src)); + qemuHotplugRemoveManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE); } qemuDomainSecretDiskDestroy(disk); @@ -4895,9 +4921,15 @@ qemuDomainRemoveDiskDevice(virQEMUDriver *driver, qemuDomainReleaseDeviceAddress(vm, &disk->info); /* tear down disk security access */ - if (diskBackend) + if (diskBackend) { qemuDomainStorageSourceChainAccessRevoke(driver, vm, disk->src); + /* If the VM is paused or in another state the image is not locked anymore */ + if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { + ignore_value(virDomainLockImageDetach(driver->lockManager, vm, disk->src)); + } + } + qemuHotplugRemoveManagedPR(vm, disk->src, VIR_ASYNC_JOB_NONE); qemuNbdkitStopStorageSource(disk->src, vm, true); -- 2.54.0