Similar to commit 540c339a for the QEMU driver, rework reference counting in the libxl driver to make it more deterministic and the code a bit cleaner. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; --- I've been testing this patch on and off for a few weeks now using libvirt-tck domain tests, local test scripts, and some manual tests for good measure. I sent the patch to Anthony Perard (cc'd) nearly two weeks ago for testing in his OpenStack+Xen+libvirt CI loop, although I haven't received any feedback thus far. Also included Martin in the cc since he encouraged this patch https://www.redhat.com/archives/libvir-list/2015-April/msg00014.html src/libxl/libxl_domain.c | 32 ++---- src/libxl/libxl_domain.h | 5 +- src/libxl/libxl_driver.c | 274 ++++++++++++++++++-------------------------- src/libxl/libxl_migration.c | 6 +- 4 files changed, 128 insertions(+), 189 deletions(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 0652270..ce188ee 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -96,13 +96,12 @@ libxlDomainObjFreeJob(libxlDomainObjPrivatePtr priv) #define LIBXL_JOB_WAIT_TIME (1000ull * 30) /* - * obj must be locked before calling, libxlDriverPrivatePtr must NOT be locked + * obj must be locked before calling * * This must be called by anything that will change the VM state - * in any way + * in any way. * - * Upon successful return, the object will have its ref count increased, - * successful calls must be followed by EndJob eventually + * Successful calls must eventually result in a call to EndJob. */ int libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, @@ -117,8 +116,6 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, return -1; then = now + LIBXL_JOB_WAIT_TIME; - virObjectRef(obj); - while (priv->job.active) { VIR_DEBUG("Wait normal job condition for starting job: %s", libxlDomainJobTypeToString(job)); @@ -149,21 +146,16 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, virReportSystemError(errno, "%s", _("cannot acquire job mutex")); - virObjectUnref(obj); return -1; } /* - * obj must be locked before calling + * obj must be locked and have a reference before calling * * To be called after completing the work associated with the * earlier libxlDomainBeginJob() call - * - * Returns true if the remaining reference count on obj is - * non-zero, false if the reference count has dropped to zero - * and obj is disposed. */ -bool +void libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, virDomainObjPtr obj) { @@ -175,8 +167,6 @@ libxlDomainObjEndJob(libxlDriverPrivatePtr driver ATTRIBUTE_UNUSED, libxlDomainObjResetJob(priv); virCondSignal(&priv->job.cond); - - return virObjectUnref(obj); } static void * @@ -485,12 +475,11 @@ libxlDomainShutdownThread(void *opaque) } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + virObjectUnlock(vm); + virObjectUnref(vm); if (dom_event) libxlDomainEventQueue(driver, dom_event); libxl_event_free(cfg->ctx, ev); @@ -528,6 +517,7 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) VIR_INFO("Received event for unknown domain ID %d", event->domid); goto error; } + virObjectRef(vm); /* * Start a thread to handle shutdown. We don't want to be tying up @@ -558,8 +548,10 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) /* Cast away any const */ libxl_event_free(cfg->ctx, (libxl_event *)event); virObjectUnref(cfg); - if (vm) + if (vm) { virObjectUnlock(vm); + virObjectUnref(vm); + } VIR_FREE(shutdown_info); } diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h index 8c73cc4..714ed91 100644 --- a/src/libxl/libxl_domain.h +++ b/src/libxl/libxl_domain.h @@ -83,10 +83,9 @@ libxlDomainObjBeginJob(libxlDriverPrivatePtr driver, enum libxlDomainJob job) ATTRIBUTE_RETURN_CHECK; -bool +void libxlDomainObjEndJob(libxlDriverPrivatePtr driver, - virDomainObjPtr obj) - ATTRIBUTE_RETURN_CHECK; + virDomainObjPtr obj); void libxlDomainEventQueue(libxlDriverPrivatePtr driver, diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index a7be745..c0061b3 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -282,7 +282,7 @@ libxlDomObjFromDomain(virDomainPtr dom) libxlDriverPrivatePtr driver = dom->conn->privateData; char uuidstr[VIR_UUID_STRING_BUFLEN]; - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); + vm = virDomainObjListFindByUUIDRef(driver->domains, dom->uuid); if (!vm) { virUUIDFormat(dom->uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -294,6 +294,25 @@ libxlDomObjFromDomain(virDomainPtr dom) return vm; } +/* + * Finish working with a domain object in an API. This function + * clears whatever was left of a domain that was gathered using + * libxlDomObjFromDomain(). Currently that means only unlocking and + * decrementing the reference counter of that domain. And in order to + * make sure the caller does not access the domain, the pointer is + * cleared. + */ +static void +libxlDomObjEndAPI(virDomainObjPtr *vm) +{ + if (!*vm) + return; + + virObjectUnlock(*vm); + virObjectUnref(*vm); + *vm = NULL; +} + static int libxlAutostartDomain(virDomainObjPtr vm, void *opaque) @@ -303,6 +322,7 @@ libxlAutostartDomain(virDomainObjPtr vm, int ret = -1; virObjectLock(vm); + virObjectRef(vm); virResetLastError(); if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { @@ -322,8 +342,10 @@ libxlAutostartDomain(virDomainObjPtr vm, ret = 0; endjob: - if (libxlDomainObjEndJob(driver, vm)) - virObjectUnlock(vm); + libxlDomainObjEndJob(driver, vm); + + virObjectUnlock(vm); + virObjectUnref(vm); return ret; } @@ -908,19 +930,19 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE, NULL))) goto cleanup; + virObjectRef(vm); def = NULL; if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { - if (!vm->persistent) { + if (!vm->persistent) virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } goto cleanup; } if (libxlDomainStart(driver, vm, (flags & VIR_DOMAIN_START_PAUSED) != 0, -1) < 0) { - virDomainObjListRemove(driver->domains, vm); + if (!vm->persistent) + virDomainObjListRemove(driver->domains, vm); goto endjob; } @@ -929,13 +951,11 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, dom->id = vm->def->id; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: virDomainDefFree(def); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return dom; } @@ -1060,12 +1080,10 @@ libxlDomainSuspend(virDomainPtr dom) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1117,12 +1135,10 @@ libxlDomainResume(virDomainPtr dom) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1183,8 +1199,7 @@ libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) } cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1232,8 +1247,7 @@ libxlDomainReboot(virDomainPtr dom, unsigned int flags) } cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1281,12 +1295,10 @@ libxlDomainDestroyFlags(virDomainPtr dom, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1315,8 +1327,7 @@ libxlDomainGetOSType(virDomainPtr dom) goto cleanup; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return type; } @@ -1335,8 +1346,7 @@ libxlDomainGetMaxMemory(virDomainPtr dom) ret = virDomainDefGetMemoryActual(vm->def); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -1455,12 +1465,10 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1513,8 +1521,7 @@ libxlDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info) ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1540,8 +1547,7 @@ libxlDomainGetState(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -1638,7 +1644,6 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, libxlDriverPrivatePtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; - bool remove_dom = false; #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -1670,21 +1675,15 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, goto endjob; if (!vm->persistent) - remove_dom = true; + virDomainObjListRemove(driver->domains, vm); ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (remove_dom && vm) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -1735,10 +1734,8 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, def = NULL; if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { - if (!vm->persistent) { + if (!vm->persistent) virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } goto cleanup; } @@ -1746,15 +1743,13 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from, if (ret < 0 && !vm->persistent) virDomainObjListRemove(driver->domains, vm); - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: if (VIR_CLOSE(fd) < 0) virReportSystemError(errno, "%s", _("cannot close file")); virDomainDefFree(def); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -1772,7 +1767,6 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); virDomainObjPtr vm; virObjectEventPtr event = NULL; - bool remove_dom = false; bool paused = false; int ret = -1; @@ -1828,7 +1822,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_CRASHED); if (!vm->persistent) - remove_dom = true; + virDomainObjListRemove(driver->domains, vm); } ret = 0; @@ -1846,16 +1840,10 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags) } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (remove_dom && vm) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -1869,7 +1857,6 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) virDomainObjPtr vm = NULL; char *name = NULL; int ret = -1; - bool remove_dom = false; virCheckFlags(0, -1); @@ -1902,21 +1889,15 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) goto endjob; if (!vm->persistent) - remove_dom = true; + virDomainObjListRemove(driver->domains, vm); ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (remove_dom && vm) { - virDomainObjListRemove(driver->domains, vm); - vm = NULL; - } - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); VIR_FREE(name); return ret; } @@ -1960,8 +1941,7 @@ libxlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) ret = vm->hasManagedSave; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -1990,8 +1970,7 @@ libxlDomainManagedSaveRemove(virDomainPtr dom, unsigned int flags) cleanup: VIR_FREE(name); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -2119,13 +2098,11 @@ libxlDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, ret = virDomainSaveConfig(cfg->configDir, def); endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: VIR_FREE(bitmask); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -2187,8 +2164,7 @@ libxlDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags) ret = (flags & VIR_DOMAIN_VCPU_MAXIMUM) ? def->maxvcpus : def->vcpus; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -2270,12 +2246,10 @@ libxlDomainPinVcpuFlags(virDomainPtr dom, unsigned int vcpu, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virBitmapFree(pcpumap); virObjectUnref(cfg); return ret; @@ -2354,8 +2328,7 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, cleanup: virBitmapFree(allcpumap); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -2418,8 +2391,7 @@ libxlDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, int maxinfo, ret = maxinfo; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -2442,8 +2414,7 @@ libxlDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) virDomainDefFormatConvertXMLFlags(flags)); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -2618,12 +2589,10 @@ libxlDomainCreateWithFlags(virDomainPtr dom, dom->id = vm->def->id; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -2760,8 +2729,7 @@ libxlDomainUndefineFlags(virDomainPtr dom, cleanup: VIR_FREE(name); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); if (event) libxlDomainEventQueue(driver, event); virObjectUnref(cfg); @@ -3618,14 +3586,12 @@ libxlDomainAttachDeviceFlags(virDomainPtr dom, const char *xml, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -3726,14 +3692,12 @@ libxlDomainDetachDeviceFlags(virDomainPtr dom, const char *xml, } endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -3833,8 +3797,7 @@ libxlDomainUpdateDeviceFlags(virDomainPtr dom, const char *xml, cleanup: virDomainDefFree(vmdef); virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -3964,8 +3927,7 @@ libxlDomainGetAutostart(virDomainPtr dom, int *autostart) ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -4029,14 +3991,12 @@ libxlDomainSetAutostart(virDomainPtr dom, int autostart) ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4091,8 +4051,7 @@ libxlDomainGetSchedulerType(virDomainPtr dom, int *nparams) ignore_value(VIR_STRDUP(ret, name)); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4157,8 +4116,7 @@ libxlDomainGetSchedulerParametersFlags(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4241,12 +4199,10 @@ libxlDomainSetSchedulerParametersFlags(virDomainPtr dom, ret = 0; endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4316,8 +4272,7 @@ libxlDomainOpenConsole(virDomainPtr dom, } cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -4452,8 +4407,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom, VIR_FREE(nodeset); virBitmapFree(nodes); libxl_bitmap_dispose(&nodemap); - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); virObjectUnref(cfg); return ret; } @@ -4474,8 +4428,7 @@ libxlDomainIsActive(virDomainPtr dom) ret = virDomainObjIsActive(obj); cleanup: - if (obj) - virObjectUnlock(obj); + libxlDomObjEndAPI(&obj); return ret; } @@ -4494,8 +4447,7 @@ libxlDomainIsPersistent(virDomainPtr dom) ret = obj->persistent; cleanup: - if (obj) - virObjectUnlock(obj); + libxlDomObjEndAPI(&obj); return ret; } @@ -4514,8 +4466,7 @@ libxlDomainIsUpdated(virDomainPtr dom) ret = vm->updated; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -4780,6 +4731,7 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, { const char *xmlin = NULL; virDomainObjPtr vm = NULL; + char *ret = NULL; #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -4798,19 +4750,20 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain, if (!(vm = libxlDomObjFromDomain(domain))) return NULL; - 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); + ret = libxlDomainMigrationBegin(domain->conn, vm, xmlin); + + cleanup: + libxlDomObjEndAPI(&vm); + return ret; } static int @@ -4919,8 +4872,7 @@ libxlDomainMigratePerform3Params(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + libxlDomObjEndAPI(&vm); return ret; } @@ -4963,23 +4915,20 @@ libxlDomainMigrateFinish3Params(virConnectPtr dconn, NULLSTR(dname)); return NULL; } + virObjectRef(vm); - if (virDomainMigrateFinish3ParamsEnsureACL(dconn, vm->def) < 0) { - virDomainObjEndAPI(&vm); - return NULL; - } + if (virDomainMigrateFinish3ParamsEnsureACL(dconn, vm->def) < 0) + goto cleanup; - if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) { - virDomainObjEndAPI(&vm); - return NULL; - } + if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) + goto cleanup; ret = libxlDomainMigrationFinish(dconn, vm, flags, cancelled); - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); - virDomainObjEndAPI(&vm); + cleanup: + libxlDomObjEndAPI(&vm); return ret; } @@ -4995,6 +4944,7 @@ libxlDomainMigrateConfirm3Params(virDomainPtr domain, { libxlDriverPrivatePtr driver = domain->conn->privateData; virDomainObjPtr vm = NULL; + int ret = -1; #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME virReportUnsupportedError(); @@ -5008,12 +4958,14 @@ libxlDomainMigrateConfirm3Params(virDomainPtr domain, if (!(vm = libxlDomObjFromDomain(domain))) return -1; - if (virDomainMigrateConfirm3ParamsEnsureACL(domain->conn, vm->def) < 0) { - virObjectUnlock(vm); - return -1; - } + if (virDomainMigrateConfirm3ParamsEnsureACL(domain->conn, vm->def) < 0) + goto cleanup; - return libxlDomainMigrationConfirm(driver, vm, flags, cancelled); + ret = libxlDomainMigrationConfirm(driver, vm, flags, cancelled); + + cleanup: + libxlDomObjEndAPI(&vm); + return ret; } static int libxlNodeGetSecurityModel(virConnectPtr conn, diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 39e4a65..665b21a 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -257,13 +257,9 @@ libxlDomainMigrationBegin(virConnectPtr conn, xml = virDomainDefFormat(def, VIR_DOMAIN_DEF_FORMAT_SECURE); endjob: - if (!libxlDomainObjEndJob(driver, vm)) - vm = NULL; + libxlDomainObjEndJob(driver, vm); cleanup: - if (vm) - virObjectUnlock(vm); - virDomainDefFree(tmpdef); virObjectUnref(cfg); return xml; -- 2.3.7