Re: [libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object

On 30.08.2013 23:46, Jim Fehlig wrote: > The libxlDriverPrivate struct contains an variety of data with > varying access needs. Similar to the QEMU and LXC drivers, > move all the static config data into a dedicated libxlDriverConfig > object. The only locking requirement is to hold the driver lock > while obtaining an instance of libxlDriverConfig. Once a reference > is held on the config object, it can be used completely lockless > since it is immutable. > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > src/libxl/libxl_conf.c | 124 ++++++++++++++++++- > src/libxl/libxl_conf.h | 52 +++++--- > src/libxl/libxl_driver.c | 313 +++++++++++++++++++++++------------------------ > 3 files changed, 309 insertions(+), 180 deletions(-) >

> > @@ -168,6 +177,8 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from, > virDomainDefPtr def = NULL; > libxlSavefileHeader hdr; > char *xml = NULL; > + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); > + int ret = -1; > > if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) { > virReportSystemError(-fd, > @@ -207,23 +218,25 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from, > goto error; > } > > - if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, > + if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, > 1 << VIR_DOMAIN_VIRT_XEN, > VIR_DOMAIN_XML_INACTIVE))) > goto error; > [...] > > > - VIR_FREE(xml); > - > *ret_def = def; > *ret_hdr = hdr; > > - return fd; > + ret = fd; > + goto cleanup; > > error: > - VIR_FREE(xml); > virDomainDefFree(def); > VIR_FORCE_CLOSE(fd); > - return -1; > + > +cleanup: > + VIR_FREE(xml); > + virObjectUnref(cfg); > + return ret; > In libvirt we rather have the 'error' label below the 'cleanup'. It's more common to jump from the 'error' to 'cleanup' then. So can you please swap these two?