On Wed, 2016-07-27 at 17:40 +0100, Joao Martins wrote:
This commit also factorizes code between the error and normal ends. Also noticed in the rest of the patches, but I think you forgot to include the SoB tag (Signed-off-by).
I never include it and it's not mandatory here in the libvirt community.
--- src/libxl/libxl_driver.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index cb501cf..5008c00 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -373,11 +373,13 @@ libxlReconnectDomain(virDomainObjPtr vm, uint8_t *data = NULL; virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; unsigned int hostdev_flags = VIR_HOSTDEV_SP_PCI; + int ret = -1;
#ifdef LIBXL_HAVE_PVUSB hostdev_flags |= VIR_HOSTDEV_SP_USB; #endif
+ virObjectRef(vm); virObjectLock(vm);
libxl_dominfo_init(&d_info); @@ -385,18 +387,18 @@ libxlReconnectDomain(virDomainObjPtr vm, /* Does domain still exist? */ rc = libxl_domain_info(cfg->ctx, &d_info, vm->def->id); if (rc == ERROR_INVAL) { - goto out; + goto error; } else if (rc != 0) { VIR_DEBUG("libxl_domain_info failed (code %d), ignoring domain %d", rc, vm->def->id); - goto out; + goto error; }
/* Is this a domain that was under libvirt control? */ if (libxl_userdata_retrieve(cfg->ctx, vm->def->id, "libvirt-xml", &data, &len)) { VIR_DEBUG("libxl_userdata_retrieve failed, ignoring domain %d", vm->def->id); - goto out; + goto error; }
/* Update domid in case it changed (e.g. reboot) while we were gone? */ @@ -405,7 +407,7 @@ libxlReconnectDomain(virDomainObjPtr vm, /* Update hostdev state */ if (virHostdevUpdateActiveDomainDevices(hostdev_mgr, LIBXL_DRIVER_NAME, vm->def, hostdev_flags) < 0) - goto out; + goto error;
if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback) driver->inhibitCallback(true, driver->inhibitOpaque); @@ -413,21 +415,23 @@ libxlReconnectDomain(virDomainObjPtr vm, /* Enable domain death events */ libxl_evenable_domain_death(cfg->ctx, vm->def->id, 0, &priv->deathW); After this patch it will always returns an error. Although patch 3 of this series does add the "ret = 0" here. I think it should a part of this patch.
Indeed, I failed splitting the change correctly, nice catch! -- Cedric
+ cleanup: libxl_dominfo_dispose(&d_info); virObjectUnlock(vm); + virObjectUnref(vm); virObjectUnref(cfg); - return 0; + return ret;
- out: - libxl_dominfo_dispose(&d_info); + error: libxlDomainCleanup(driver, vm); - if (!vm->persistent) + if (!vm->persistent) { virDomainObjListRemoveLocked(driver->domains, vm); - else - virObjectUnlock(vm); - virObjectUnref(cfg);
- return -1; + /* virDomainObjListRemoveLocked leaves the object unlocked, + * lock it again to factorize more code. */ + virObjectLock(vm); + } + goto cleanup; }
static void
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list