On 03/07/2013 11:50 AM, Daniel P. Berrange wrote:
> 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));
Or use struct assignment:
xmlconf->privDataCallbacks = *priv;
(I have an innate dislike of memcpy when it can be avoided, since it
eliminates type checking).
>
>> + }
>> +
>> + if (xmlns)
>> + xmlconf->ns = *xmlns;
Yeah, like you did there.
>> +
>> + return xmlconf;
>> +}
>> +
>> +virDomainXMLNamespace
>> +virDomainXMLConfGetNamespace(virDomainXMLConfPtr xmlconf)
>> +{
>> + return xmlconf->ns;
>> +}
You're returning a struct there, rather than a pointer to a struct. Do
you really want to restrict yourself that way?
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list