[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 REPOST 1/8] Add namespace callback hooks to domain_conf.



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 redhat com>

> +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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]