If a mirror job fails to start in -blockdev mode we'd not unplug the backing files we added first because the code on the error path checked the wrong value. 'rc' is used as status of the code which added the images, but the state of the 'block(dev)-mirror' call is stored in 'ret' at that point. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0a1dc414ae..3218dc0e23 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18413,7 +18413,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, qemuBlockJobStarted(job, vm); endjob: - if (rc < 0 && + if (ret < 0 && virDomainObjIsActive(vm) && (data || crdata)) { qemuDomainObjEnterMonitor(driver, vm); -- 2.24.1

There are more places which require getting the topmost nodename to be passed to qemu. Separate it out into a new function. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_domain.c | 38 ++++++++++++++++++++++++++------------ src/qemu/qemu_domain.h | 4 ++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 38addc7b61..4604b6c993 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11530,6 +11530,31 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver, } +/** + * qemuDomainDiskGetTopNodename: + * + * @disk: disk definition object + * + * Returns the pointer to the node-name of the topmost layer used by @disk as + * backend. Currently returns the nodename of the copy-on-read filter if enabled + * or the nodename of the top image's format driver. Empty disks return NULL. + * This must be used only when VIR_QEMU_CAPS_BLOCKDEV is enabled. + */ +const char * +qemuDomainDiskGetTopNodename(virDomainDiskDefPtr disk) +{ + qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); + + if (virStorageSourceIsEmpty(disk->src)) + return NULL; + + if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) + return priv->nodeCopyOnRead; + + return disk->src->nodeformat; +} + + /** * qemuDomainDiskGetBackendAlias: * @disk: disk definition @@ -11549,8 +11574,6 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps, char **backendAlias) { - qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk); - const char *nodename = NULL; *backendAlias = NULL; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) { @@ -11560,16 +11583,7 @@ qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk, return 0; } - if (virStorageSourceIsEmpty(disk->src)) - return 0; - - if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON) - nodename = priv->nodeCopyOnRead; - else - nodename = disk->src->nodeformat; - - *backendAlias = g_strdup(nodename); - + *backendAlias = g_strdup(qemuDomainDiskGetTopNodename(disk)); return 0; } diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 21ece23177..c8bf621c61 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -874,6 +874,10 @@ int qemuDomainStorageFileInit(virQEMUDriverPtr driver, virStorageSourcePtr parent); char *qemuDomainStorageAlias(const char *device, int depth); +const char * +qemuDomainDiskGetTopNodename(virDomainDiskDefPtr disk) + ATTRIBUTE_NONNULL(1); + int qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps, char **backendAlias) -- 2.24.1

When using blockdev configurations the 'device' argument of 'blockdev-mirror' must correspond to the topmost node in the block node graph. Libvirt didn't do this properly in case when 'copy_on_read' option was enabled on the disk. Use qemuDomainDiskGetTopNodename to fix it for the blockdev-mirror calls in qemuDomainBlockCopy and the non-shared-storage migration. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_migration.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3218dc0e23..9e2a94306c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18386,7 +18386,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (blockdev) { ret = qemuMonitorBlockdevMirror(priv->mon, job->name, true, - disk->src->nodeformat, + qemuDomainDiskGetTopNodename(disk), mirror->nodeformat, bandwidth, granularity, buf_size, mirror_shallow); } else { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 29d228a8d9..d7814208a2 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -931,7 +931,7 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) { jobname = diskAlias; - sourcename = disk->src->nodeformat; + sourcename = qemuDomainDiskGetTopNodename(disk); persistjob = true; } else { jobname = NULL; -- 2.24.1

When using blockdev configurations the 'device' argument of 'blockdev-commit' must correspond to the topmost node in the block node graph. Libvirt didn't do this properly in case when 'copy_on_read' option was enabled on the disk. Use qemuDomainDiskGetTopNodename to fix it when calling block-commit. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9e2a94306c..0b23c747ce 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18813,7 +18813,7 @@ qemuDomainBlockCommit(virDomainPtr dom, jobname = job->name; nodetop = topSource->nodeformat; nodebase = baseSource->nodeformat; - device = disk->src->nodeformat; + device = qemuDomainDiskGetTopNodename(disk); if (!backingPath && top_parent && !(backingPath = qemuBlockGetBackingStoreString(baseSource))) goto endjob; -- 2.24.1