[PATCH 0/3] qemu: block copy fixes
Three patches fixing various bits in block copy job setup. Peter Krempa (3): qemu: domain: Unexport 'qemuDomainPrepareStorageSourceBlockdevNodename' qemuDomainBlockCopyCommon: Reorder setup of 'mirror' data qemuDomainBlockCopyCommon: Don't revoke access to file twice on failure src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ----- src/qemu/qemu_driver.c | 18 ++++++++---------- 3 files changed, 9 insertions(+), 16 deletions(-) -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> The function is referenced only from within qemu_domain.c Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c3ca4b3040..63ea7db33a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9694,7 +9694,7 @@ qemuDomainPrepareStorageSourceFDs(virStorageSource *src, } -int +static int qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, virStorageSource *src, const char *nodenameprefix, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 70e1fb187f..07b2151f6b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -757,11 +757,6 @@ int qemuDomainStorageSourceAccessAllow(virQEMUDriver *driver, bool newSource, bool chainTop); -int qemuDomainPrepareStorageSourceBlockdevNodename(virDomainDiskDef *disk, - virStorageSource *src, - const char *nodenameprefix, - qemuDomainObjPrivate *priv, - virQEMUDriverConfig *cfg); int qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDef *disk, virStorageSource *src, qemuDomainObjPrivate *priv, -- 2.49.0
From: Peter Krempa <pkrempa@redhat.com> While exploring an idea that modified the setup of the mirror I've noticed that the code setting up the 'discard' field in the block copy job happens after setup of the stroage source, while normally e.g. in qemuDomainPrepareStorageSource() it happens before. Reorder it despite not having an effect currently. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ce949dd07..f657751057 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14384,13 +14384,6 @@ qemuDomainBlockCopyCommon(virDomainObj *vm, * as read-write for the duration of the copy job */ mirror->readonly = false; - /* we must initialize XML-provided chain prior to detecting to keep semantics - * with VM startup */ - for (n = mirror; virStorageSourceIsBacking(n); n = n->backingStore) { - if (qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0) - goto endjob; - } - /* 'qemuDomainPrepareStorageSourceBlockdev' calls * 'qemuDomainPrepareDiskSourceData' which propagates 'detect_zeroes' * into the topmost virStorage source of the disk chain. @@ -14401,6 +14394,13 @@ qemuDomainBlockCopyCommon(virDomainObj *vm, mirror->detect_zeroes = disk->detect_zeroes; mirror->discard_no_unref = disk->discard_no_unref; + /* we must initialize XML-provided chain prior to detecting to keep semantics + * with VM startup */ + for (n = mirror; virStorageSourceIsBacking(n); n = n->backingStore) { + if (qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0) + goto endjob; + } + /* If reusing an external image that includes a backing file but the user * did not enumerate the chain in the XML we need to detect the chain */ if (mirror_reuse && -- 2.49.0
On a Thursday in 2025, Peter Krempa via Devel wrote:
From: Peter Krempa <pkrempa@redhat.com>
While exploring an idea that modified the setup of the mirror I've noticed that the code setting up the 'discard' field in the block copy job happens after setup of the stroage source, while normally e.g. in
*storage
qemuDomainPrepareStorageSource() it happens before.
Reorder it despite not having an effect currently.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
From: Peter Krempa <pkrempa@redhat.com> If the copy job fails to start up when calling the 'blockdev-mirror' command the code would call qemuDomainStorageSourceChainAccessRevoke() twice; once right after the monitor call and the second time in the 'endjob' section. Remove the one directly after the monitor call and let the common cleanup handle it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f657751057..2afe6fdc83 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14497,10 +14497,8 @@ qemuDomainBlockCopyCommon(virDomainObj *vm, virDomainAuditDisk(vm, NULL, mirror, "mirror", ret >= 0); qemuDomainObjExitMonitor(vm); - if (ret < 0) { - qemuDomainStorageSourceChainAccessRevoke(driver, vm, mirror); + if (ret < 0) goto endjob; - } /* Update vm in place to match changes. */ need_unlink = false; -- 2.49.0
On a Thursday in 2025, Peter Krempa via Devel wrote:
Three patches fixing various bits in block copy job setup.
Peter Krempa (3): qemu: domain: Unexport 'qemuDomainPrepareStorageSourceBlockdevNodename' qemuDomainBlockCopyCommon: Reorder setup of 'mirror' data qemuDomainBlockCopyCommon: Don't revoke access to file twice on failure
src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain.h | 5 ----- src/qemu/qemu_driver.c | 18 ++++++++---------- 3 files changed, 9 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa