[libvirt] [PATCH 07/12] libxl: Introduce libxlDriverConfig object
Jim Fehlig
jfehlig at suse.com
Tue Sep 3 23:16:02 UTC 2013
Michal Privoznik wrote:
> 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 at suse.com>
>> ---
>> 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?
>
Hmm, looking at this again I think adding the cleanup label was
overkill. I've changed the above two hunks to the following, which is a
bit simpler.
@@ -168,6 +177,7 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
virDomainDefPtr def = NULL;
libxlSavefileHeader hdr;
char *xml = NULL;
+ libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
if ((fd = virFileOpenAs(from, O_RDONLY, 0, -1, -1, 0)) < 0) {
virReportSystemError(-fd,
@@ -207,12 +217,13 @@ 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);
+ virObjectUnref(cfg);
*ret_def = def;
*ret_hdr = hdr;
@@ -221,6 +232,7 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
error:
VIR_FREE(xml);
+ virObjectUnref(cfg);
virDomainDefFree(def);
VIR_FORCE_CLOSE(fd);
return -1;
Regards,
Jim
More information about the libvir-list
mailing list