Re: [libvirt] [PATCHv2 01/21] virCaps: conf: start splitting out irrelevat data

On Wed, Mar 06, 2013 at 04:37:45PM +0100, Peter Krempa wrote: > The virCaps structure gathered a ton of irrelevant data over time that. > The original reason is that it was propagated to the XML parser > functions. > > This patch aims to create a new data structure virDomainXMLConf that > will contain immutable data that are used by the XML parser. This will > allow two things we need: > > 1) Get rid of the stuff from virCaps > > 2) Allow us to add callbacks to check and add driver specific stuff > after domain XML is parsed. > > This first attempt removes pointers to private data allocation functions > to this new structure and update all callers and function that require > them. > --- > src/conf/capabilities.h | 8 ++---- > src/conf/domain_conf.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ > src/conf/domain_conf.h | 27 +++++++++++++++++++ > 3 files changed, 99 insertions(+), 6 deletions(-) > > diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h > index cc01765..bf64296 100644 > --- a/src/conf/capabilities.h > +++ b/src/conf/capabilities.h > @@ -159,19 +159,15 @@ struct _virCaps { > size_t nguests; > size_t nguests_max; > virCapsGuestPtr *guests; > + > + /* deal with these later */ Better written as /* Move to virDomainXMLConf later */ > unsigned char macPrefix[VIR_MAC_PREFIX_BUFLEN]; > unsigned int emulatorRequired : 1; > const char *defaultDiskDriverName; > int defaultDiskDriverType; /* enum virStorageFileFormat */ > int (*defaultConsoleTargetType)(const char *ostype, virArch guestarch); > - void *(*privateDataAllocFunc)(void); > - void (*privateDataFreeFunc)(void *); > - int (*privateDataXMLFormat)(virBufferPtr, void *); > - int (*privateDataXMLParse)(xmlXPathContextPtr, void *); > bool hasWideScsiBus; > const char *defaultInitPath; > - > - virDomainXMLNamespace ns; > }; > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index f7c8af1..2b54aec 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -736,6 +736,76 @@ static int virDomainObjOnceInit(void) > > VIR_ONCE_GLOBAL_INIT(virDomainObj) > > + > +/* This structure holds various callbacks and data needed > + * while parsing and creating domain XMLs */ > +struct _virDomainXMLConf { > + virObject parent; > + > + /* domain private data management callbacks */ > + virDomainXMLPrivateDataAllocFunc privateDataAllocFunc; > + virDomainXMLPrivateDataFreeFunc privateDataFreeFunc; > + virDomainXMLPrivateDataFormatFunc privateDataXMLFormat; > + virDomainXMLPrivateDataParseFunc privateDataXMLParse; Why not just use 'virDomainXMLPrivateDataCallbacks' here instead of duplicating its contents ? > + > + /* XML namespace callbacks */ > + virDomainXMLNamespace ns; > + }; Good, I like that this struct is only in the .c file and not exposed in .h > +virDomainXMLConfPtr > +virDomainXMLConfNew(virDomainXMLPrivateDataCallbacksPtr priv, > + virDomainXMLNamespacePtr xmlns) > +{ > + virDomainXMLConfPtr xmlconf; > + > + if (virDomainXMLConfInitialize() < 0) > + return NULL; > + > + if (!(xmlconf = virObjectNew(virDomainXMLConfClass))) > + return NULL; > + > + if (priv) { > + xmlconf->privateDataAllocFunc = priv->alloc; > + xmlconf->privateDataFreeFunc = priv->free; > + xmlconf->privateDataXMLFormat = priv->format; > + xmlconf->privateDataXMLParse = priv->parse; Then you could just do if (priv) memcpy(xmlconf->privDataCallbacks, priv, sizeof(*priv));

> + } > + > + if (xmlns) > + xmlconf->ns = *xmlns;

> + > + return xmlconf; > +} > + > +virDomainXMLNamespace > +virDomainXMLConfGetNamespace(virDomainXMLConfPtr xmlconf) > +{ > + return xmlconf->ns; > +}