[libvirt] [PATCH v2] qemu: fix domain unlock/unref in qemuMigrationSrcPerform

Nikolay Shirokovskiy posted 1 patch 4 years, 12 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1554125281-155394-1-git-send-email-nshirokovskiy@virtuozzo.com
src/qemu/qemu_driver.c    | 21 +++++++++------------
src/qemu/qemu_migration.c |  2 --
2 files changed, 9 insertions(+), 14 deletions(-)
[libvirt] [PATCH v2] qemu: fix domain unlock/unref in qemuMigrationSrcPerform
Posted by Nikolay Shirokovskiy 4 years, 12 months ago
qemuMigrationSrcPerform callers expect it to call virDomainObjEndAPI
in any case so on error paths we miss the virDomainObjEndAPI call.
To fix this let's make qemuMigrationSrcPerform callers responsible
for the virDomainObjEndAPI call.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---

diff to v1 [1]:
- move virDomainObjEndAPI call to qemuMigrationSrcPerform callers

[1] https://www.redhat.com/archives/libvir-list/2019-April/msg00032.html

 src/qemu/qemu_driver.c    | 21 +++++++++------------
 src/qemu/qemu_migration.c |  2 --
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 62d8d97..37baa5c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12550,7 +12550,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
                          unsigned long resource)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     int ret = -1;
     const char *dconnuri = NULL;
     qemuMigrationParamsPtr migParams = NULL;
@@ -12571,10 +12571,8 @@ qemuDomainMigratePerform(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
 
-    if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0) {
-        virDomainObjEndAPI(&vm);
+    if (virDomainMigratePerformEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
-    }
 
     if (flags & VIR_MIGRATE_PEER2PEER)
         VIR_STEAL_PTR(dconnuri, uri);
@@ -12592,6 +12590,7 @@ qemuDomainMigratePerform(virDomainPtr dom,
                                   flags, dname, resource, false);
 
  cleanup:
+    virDomainObjEndAPI(&vm);
     qemuMigrationParamsFree(migParams);
     return ret;
 }
@@ -12988,7 +12987,7 @@ qemuDomainMigratePerform3(virDomainPtr dom,
                           unsigned long resource)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     qemuMigrationParamsPtr migParams = NULL;
     int ret = -1;
 
@@ -13001,10 +13000,8 @@ qemuDomainMigratePerform3(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
 
-    if (virDomainMigratePerform3EnsureACL(dom->conn, vm->def) < 0) {
-        virDomainObjEndAPI(&vm);
+    if (virDomainMigratePerform3EnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
-    }
 
     ret = qemuMigrationSrcPerform(driver, dom->conn, vm, xmlin, NULL,
                                   dconnuri, uri, NULL, NULL, 0, NULL, 0,
@@ -13014,6 +13011,7 @@ qemuDomainMigratePerform3(virDomainPtr dom,
                                   flags, dname, resource, true);
 
  cleanup:
+    virDomainObjEndAPI(&vm);
     qemuMigrationParamsFree(migParams);
     return ret;
 }
@@ -13030,7 +13028,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
                                 unsigned int flags)
 {
     virQEMUDriverPtr driver = dom->conn->privateData;
-    virDomainObjPtr vm;
+    virDomainObjPtr vm = NULL;
     const char *dom_xml = NULL;
     const char *persist_xml = NULL;
     const char *dname = NULL;
@@ -13088,10 +13086,8 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
 
-    if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0) {
-        virDomainObjEndAPI(&vm);
+    if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
-    }
 
     ret = qemuMigrationSrcPerform(driver, dom->conn, vm, dom_xml, persist_xml,
                                   dconnuri, uri, graphicsuri, listenAddress,
@@ -13100,6 +13096,7 @@ qemuDomainMigratePerform3Params(virDomainPtr dom,
                                   cookiein, cookieinlen, cookieout, cookieoutlen,
                                   flags, dname, bandwidth, true);
  cleanup:
+    virDomainObjEndAPI(&vm);
     qemuMigrationParamsFree(migParams);
     VIR_FREE(migrate_disks);
     return ret;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 419a729..bb43aff 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4686,7 +4686,6 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
     }
 
  cleanup:
-    virDomainObjEndAPI(&vm);
     virObjectEventStateQueue(driver->domainEventState, event);
     virObjectUnref(cfg);
     return ret;
@@ -4757,7 +4756,6 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
         qemuDomainRemoveInactiveJob(driver, vm);
 
  cleanup:
-    virDomainObjEndAPI(&vm);
     return ret;
 }
 
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] qemu: fix domain unlock/unref in qemuMigrationSrcPerform
Posted by Michal Privoznik 4 years, 12 months ago
On 4/1/19 3:28 PM, Nikolay Shirokovskiy wrote:
> qemuMigrationSrcPerform callers expect it to call virDomainObjEndAPI
> in any case so on error paths we miss the virDomainObjEndAPI call.
> To fix this let's make qemuMigrationSrcPerform callers responsible
> for the virDomainObjEndAPI call.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
> 
> diff to v1 [1]:
> - move virDomainObjEndAPI call to qemuMigrationSrcPerform callers
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-April/msg00032.html
> 
>   src/qemu/qemu_driver.c    | 21 +++++++++------------
>   src/qemu/qemu_migration.c |  2 --
>   2 files changed, 9 insertions(+), 14 deletions(-)

ACK. I'll let Jirka chime in to decide if this is worth merging into 
5.2.0 or wait for 5.3.0. Jirka?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list