On Mon, Jul 20, 2026 at 16:06:24 +0900, Mitsuru Kariya via Devel wrote:
qemuDomainChangeEjectableMedia() ran qemuDomainPrepareDiskSource() and the surrounding image setup (backing chain detection, storage access, managed PR) unconditionally. When ejecting to no media the new source is empty, and qemuDomainPrepareDiskSource() still assigned it node names that are never realized in QEMU, leaving a stale name in the disk source
This by itself is not a problem. In addition some of the functions don't do anything at all for empty source. But I agree that setting up all of this is pointless.
that a later detach would then try to blockdev-del.
This is not really fixed by this, you'd get a different error because in qemuBlockStorageSourceDetachPrepare data->storageNodeName would be NULL, but data->storageAttached is still set to true. qemuBlockStorageSourceChainDetach then would call qemuMonitorBlockdevDel with NULL argument, which would report an JSON formatting error as 'node-name' is mandatory. Thus the code was doing pointless stuff but this commit isn't fixing anything regarding to the previous error.
Wrap the image setup in a virStorageSourceIsEmpty() check, mirroring qemuDomainAttachDeviceDiskLiveInternal(), and only revoke storage access on rollback when it was actually granted, using a releaseSeclabel flag as the attach path already does.
Signed-off-by: Mitsuru Kariya <Mitsuru.Kariya@oss.nttdata.com> --- src/qemu/qemu_hotplug.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c2cd5496e0..b96ff24cbf 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 releaseSeclabel = false; int rc;
if (diskPriv->blockjob && qemuBlockJobIsRunning(diskPriv->blockjob)) { @@ -625,17 +626,21 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, if (virDomainDiskTranslateSourcePool(disk) < 0) goto rollback;
- if (qemuDomainDetermineDiskChain(driver, vm, disk, NULL) < 0) - goto rollback; + if (!virStorageSourceIsEmpty(newsrc)) { + if (qemuDomainDetermineDiskChain(driver, vm, disk, NULL) < 0) + goto rollback;
- if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) - goto rollback; + if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) + goto rollback;
- if (qemuDomainStorageSourceChainAccessAllow(driver, vm, newsrc) < 0) - goto rollback; + if (qemuDomainStorageSourceChainAccessAllow(driver, vm, newsrc) < 0) + goto rollback;
- if (qemuHotplugAttachManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE) < 0) - goto rollback; + releaseSeclabel = true; + + if (qemuHotplugAttachManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE) < 0) + goto rollback; + }
rc = qemuDomainChangeMediaBlockdev(vm, disk, oldsrc, newsrc, force);
@@ -653,7 +658,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriver *driver, return 0;
rollback: - ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, newsrc)); + if (releaseSeclabel) + ignore_value(qemuDomainStorageSourceChainAccessRevoke(driver, vm, newsrc));
qemuHotplugRemoveManagedPR(vm, newsrc, VIR_ASYNC_JOB_NONE);
I'll tweak the commit message slightly. Reviewed-by: Peter Krempa <pkrempa@redhat.com>