[libvirt PATCH] qemu: Fix error propagation in qemuMigrationBegin

Jiri Denemark posted 1 patch 1 year, 11 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/ecf3d3c6f7ecf88269fdadee03eb2558554d70c0.1653303317.git.jdenemar@redhat.com
src/qemu/qemu_migration.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[libvirt PATCH] qemu: Fix error propagation in qemuMigrationBegin
Posted by Jiri Denemark 1 year, 11 months ago
Commit v8.3.0-152-g49ef0f95c6 removed explicit VIR_FREE from
qemuMigrationBegin, effectively reverting v1.2.14-57-g77ddd0bba2

The xml variable was used to hold the return value and thus had to be
unset when an error happened after xml was already non-NULL. Such code
may be quite confusing though and we usually avoid it by not storing
anything to a return variable until everything succeeded.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_migration.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 438f2bc999..38596fa4de 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2487,6 +2487,7 @@ qemuMigrationSrcBegin(virConnectPtr conn,
     virQEMUDriver *driver = conn->privateData;
     g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     g_autofree char *xml = NULL;
+    char *ret = NULL;
     virDomainAsyncJob asyncJob;
 
     if (cfg->migrateTLSForce &&
@@ -2538,9 +2539,11 @@ qemuMigrationSrcBegin(virConnectPtr conn,
         goto endjob;
     }
 
+    ret = g_steal_pointer(&xml);
+
  cleanup:
     virDomainObjEndAPI(&vm);
-    return g_steal_pointer(&xml);
+    return ret;
 
  endjob:
     if (flags & VIR_MIGRATE_CHANGE_PROTECTION)
-- 
2.35.1
Re: [libvirt PATCH] qemu: Fix error propagation in qemuMigrationBegin
Posted by Peter Krempa 1 year, 11 months ago
On Mon, May 23, 2022 at 12:55:17 +0200, Jiri Denemark wrote:
> Commit v8.3.0-152-g49ef0f95c6 removed explicit VIR_FREE from
> qemuMigrationBegin, effectively reverting v1.2.14-57-g77ddd0bba2
> 
> The xml variable was used to hold the return value and thus had to be
> unset when an error happened after xml was already non-NULL. Such code
> may be quite confusing though and we usually avoid it by not storing
> anything to a return variable until everything succeeded.
> 
> Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
> ---
>  src/qemu/qemu_migration.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Oops.

Reviewed-by: Peter Krempa <pkrempa@redhat.com>