On Thu, Dec 08, 2022 at 14:30:38 +0100, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_block.c | 179 +++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_block.h | 9 +++ src/qemu/qemu_driver.c | 162 +------------------------------------ 3 files changed, 189 insertions(+), 161 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8a6f601b29..4cca7555f3 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -3196,3 +3196,182 @@ qemuBlockExportAddNBD(virDomainObj *vm,
[...]
+ if (rc < 0) + goto error; + + if (mirror) { + disk->mirror = g_steal_pointer(&mirror); + disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; + } + qemuBlockJobStarted(job, vm);
[1]
+ + return 0; + + error: + if (clean_access) { + virErrorPtr orig_err; + virErrorPreserveLast(&orig_err); + /* Revert access to read-only, if possible. */ + qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, + true, false, false); + if (top_parent && top_parent != disk->src) + qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, + true, false, false); + + virErrorRestore(&orig_err); + } + qemuBlockJobStartupFinalize(vm, job);
[2]
+ + return -1; +}
[...]
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d509582719..2f05da3d8c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
[...]
- - ret = qemuMonitorBlockCommit(priv->mon, - qemuDomainDiskGetTopNodename(disk), - job->name, - topSource->nodeformat, - baseSource->nodeformat, - backingPath, speed); - - qemuDomainObjExitMonitor(vm); - - if (ret < 0) - goto endjob; - - if (mirror) { - disk->mirror = g_steal_pointer(&mirror); - disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT; - } - qemuBlockJobStarted(job, vm); + ret = qemuBlockCommit(vm, disk, baseSource, topSource, top_parent, bandwidth, flags);
endjob: - if (ret < 0 && clean_access) { - virErrorPtr orig_err; - virErrorPreserveLast(&orig_err); - /* Revert access to read-only, if possible. */ - qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, - true, false, false); - if (top_parent && top_parent != disk->src) - qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, - true, false, false); - - virErrorRestore(&orig_err); - } - qemuBlockJobStartupFinalize(vm, job);
So before this patch qemuBlockJobStartupFinalize() gets called both on error and success code paths. After this patch [1] the success code path doesn't call it any more, just the error code path [2]. Thus the reference to 'job' is leaked in the new code. The new function must also call qemuBlockJobStartupFinalize() in both cases.