On 07/01/2010 02:59 PM, Chris Lalancette wrote: > This patch adds namespace XML parsers to be hooked into > the main domain parser. This allows for individual hypervisor > drivers to add per-namespace XML into the main domain XML. > > Changes since v1: > - Use a statically declared table for caps->ns, removing the need to > allocate/free it. > > Signed-off-by: Chris Lalancette <clalance@xxxxxxxxxx> > +typedef int (*virDomainDefNamespaceParse)(xmlDocPtr, xmlNodePtr, > + xmlXPathContextPtr, void **); > +typedef void (*virDomainDefNamespaceFree)(void *); > +typedef int (*virDomainDefNamespaceXMLFormat)(virBufferPtr, void *); > +typedef const char *(*virDomainDefNamespaceHref)(void); Should virDomainDefNamespaceHref take a void* argument... > + if (def->namespaceData && def->ns.href) > + virBufferVSprintf(&buf, " %s", (def->ns.href)()); and pass def->namespaceData through to that function? I'm a little bit nervous about callback functions that cannot be extended, and passing a (unused, for now) void* pointer gives us some room for growth without changing the API, if need be. > +++ b/src/conf/domain_conf.c > @@ -738,6 +738,9 @@ void virDomainDefFree(virDomainDefPtr def) > > virCPUDefFree(def->cpu); > > + if (def->namespaceData && def->ns.free) > + (def->ns.free)(def->namespaceData); Style nit - you can omit the first set of parenthesis: def->ns.free(def->namespaceData) I don't know which style is more prevalent in existing code though, but if you choose to elide the parenthesis, there are several places in this patch that are affected. > static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, > - xmlXPathContextPtr ctxt, int flags) > + xmlDocPtr xml, > + xmlNodePtr root, > + xmlXPathContextPtr ctxt, > + int flags) As long as we are changing this API, should we change flags to unsigned int? And should we add a virCheckFlags(VIR_DOMAIN_XML_INACTIVE..., NULL)? -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list