Re: [libvirt] [libvirt-glib] API to get/set custom metadata from/to domain config

On Fri, Jan 20, 2012 at 11:11:57PM +0200, Zeeshan Ali (Khattak) wrote: > From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org&gt; > > --- >  libvirt-gconfig/libvirt-gconfig-domain.c  |  115 +++++++++++++++++++++++++++++ >  libvirt-gconfig/libvirt-gconfig-domain.h  |    8 ++ >  libvirt-gconfig/libvirt-gconfig-helpers.c |    4 +- >  libvirt-gconfig/libvirt-gconfig.sym       |    2 + >  4 files changed, 128 insertions(+), 1 deletions(-) > > diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c > index 61af625..d1faabd 100644 > --- a/libvirt-gconfig/libvirt-gconfig-domain.c > +++ b/libvirt-gconfig/libvirt-gconfig-domain.c > @@ -449,3 +449,118 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain *domain) > >      return data.devices; >  } > + > +static void set_namespace_on_tree(xmlNodePtr node, xmlNsPtr namespace) > +{ > +    xmlNodePtr child; > + > +    xmlSetNs(node, namespace); > + > +    for (child = node->children; child != NULL; child = child->next) > +        set_namespace_on_tree(child, namespace); I wouldn't make this recursive, I'd just set the namespace on the root node in case users of the API want to use different namespaces in the xml they embed in the domain.

> +    if (metadata_node == NULL) > +        return NULL; > + > +    for (node = metadata_node->children; node != NULL; node = node->next) { > +        if (node->ns != NULL && > +            (g_strcmp0 ((gchar *)node->ns->href, ns_uri) == 0)) > +            break; > +    } > + > +    return node; > +} Hmm seeing this loop, gvir_config_object_foreach_child could even be used here with a small helper struct returning the value.

> +    set_namespace_on_tree(node, namespace); > +    doc = node->doc; > + > +    old_node = gvir_config_domain_get_custom_xml_node(domain, ns_uri); > +    if (old_node != NULL) > +        xmlReplaceNode(old_node, node); > +    else { > +        xmlUnlinkNode(node); > +        xmlAddChild(metadata_node, node); > +    } This duplicates one of the GVirConfigObject helpers. I've tried to restrict xmlNodePtr use to libvirt-gconfig-object.c/libvirt-gconfig-helpers.c. So this patch which adds a lot of xmlNodePtr use to libvirt-gconfig-domain.c makes me a bit uncomfortable. Here I think we could use gvir_config_object_new_from_xml and reuse the existing helpers (with the addition of gvir_config_object_set_namespace).