Re: [libvirt] [PATCH 2/3] libxl: fix segfault in libxlReconnectDomain

> 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).

> --- > 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.

> > + 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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list