On Thu, Dec 08, 2022 at 14:30:38 +0100, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrd...@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.
