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 <[email protected]> > --- > 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 <[email protected]>
