The libxlDomainMigrateBegin3Params API locks and ref counts the associated
virDomainObj but relies on the helper function libxlDomainMigrationBegin
to unref/unlock the object. libxlDomainMigrationBegin is also used by
libxlDomainMigratePerform3Params for p2p migration, but in that case the
lock/ref and unref/unlock are properly handled in the API entry point. So
p2p migrations suffer a double unref/unlock in the Perform API.
Remove the unref/unlock (virDomainObjEndAPI) from libxlDomainMigrationBegin
and adjust libxlDomainMigrateBegin3Params to properly unref/unlock
the virDomainObj on success and error paths.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---
src/libxl/libxl_driver.c | 24 +++++++++++++-----------
src/libxl/libxl_migration.c | 1 -
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index c3616a86d..eff13e5aa 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5876,6 +5876,7 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
{
const char *xmlin = NULL;
virDomainObjPtr vm = NULL;
+ char *xmlout = NULL;
#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
virReportUnsupportedError();
@@ -5895,25 +5896,26 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
return NULL;
if (STREQ_NULLABLE(vm->def->name, "Domain-0")) {
- virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Domain-0 cannot be migrated"));
- return NULL;
+ virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Domain-0 cannot be migrated"));
+ goto cleanup;
}
- if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) {
- virObjectUnlock(vm);
- return NULL;
- }
+ if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0)
+ goto cleanup;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("domain is not running"));
- virObjectUnlock(vm);
- return NULL;
+ goto cleanup;
}
- return libxlDomainMigrationBegin(domain->conn, vm, xmlin,
- cookieout, cookieoutlen);
+ xmlout = libxlDomainMigrationBegin(domain->conn, vm, xmlin,
+ cookieout, cookieoutlen);
+
+ cleanup:
+ virDomainObjEndAPI(&vm);
+ return xmlout;
}
static int
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index ccf2daed1..4b848c920 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -443,7 +443,6 @@ libxlDomainMigrationBegin(virConnectPtr conn,
cleanup:
libxlMigrationCookieFree(mig);
- virDomainObjEndAPI(&vm);
virDomainDefFree(tmpdef);
virObjectUnref(cfg);
return xml;
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 03/13/2018 01:26 PM, Jim Fehlig wrote: > The libxlDomainMigrateBegin3Params API locks and ref counts the associated > virDomainObj but relies on the helper function libxlDomainMigrationBegin > to unref/unlock the object. libxlDomainMigrationBegin is also used by > libxlDomainMigratePerform3Params for p2p migration, but in that case the > lock/ref and unref/unlock are properly handled in the API entry point. So > p2p migrations suffer a double unref/unlock in the Perform API. > > Remove the unref/unlock (virDomainObjEndAPI) from libxlDomainMigrationBegin > and adjust libxlDomainMigrateBegin3Params to properly unref/unlock > the virDomainObj on success and error paths. > > Signed-off-by: Jim Fehlig <jfehlig@suse.com> > --- > src/libxl/libxl_driver.c | 24 +++++++++++++----------- > src/libxl/libxl_migration.c | 1 - > 2 files changed, 13 insertions(+), 12 deletions(-) > Reviewed-by: John Ferlan <jferlan@redhat.com> John BTW: Outside the scope of this series; however, danpb went through the painstaking task of modifying names of qemu_migration API's such that it's easier to determine if the API was run on qemuMigrationSrc or qemuMigrationDst - made it so much easier to remember which was which w/r/t the various stages - you may want to consider doing that too for libxl! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/14/2018 06:43 AM, John Ferlan wrote: > > > On 03/13/2018 01:26 PM, Jim Fehlig wrote: >> The libxlDomainMigrateBegin3Params API locks and ref counts the associated >> virDomainObj but relies on the helper function libxlDomainMigrationBegin >> to unref/unlock the object. libxlDomainMigrationBegin is also used by >> libxlDomainMigratePerform3Params for p2p migration, but in that case the >> lock/ref and unref/unlock are properly handled in the API entry point. So >> p2p migrations suffer a double unref/unlock in the Perform API. >> >> Remove the unref/unlock (virDomainObjEndAPI) from libxlDomainMigrationBegin >> and adjust libxlDomainMigrateBegin3Params to properly unref/unlock >> the virDomainObj on success and error paths. >> >> Signed-off-by: Jim Fehlig <jfehlig@suse.com> >> --- >> src/libxl/libxl_driver.c | 24 +++++++++++++----------- >> src/libxl/libxl_migration.c | 1 - >> 2 files changed, 13 insertions(+), 12 deletions(-) >> > > Reviewed-by: John Ferlan <jferlan@redhat.com> Thanks for reviewing the series! > BTW: Outside the scope of this series; however, danpb went through the > painstaking task of modifying names of qemu_migration API's such that > it's easier to determine if the API was run on qemuMigrationSrc or > qemuMigrationDst - made it so much easier to remember which was which > w/r/t the various stages - you may want to consider doing that too for > libxl! Good point. I'm familiar with the code and still find myself thinking about which phase pertains to the src and which to the dest. Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.