On Fri, Jan 20, 2012 at 11:11:57PM +0200, Zeeshan Ali (Khattak) wrote:
> From: "Zeeshan Ali (Khattak)" <zeeshanak@xxxxxxxxx>
>
> ---
> 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.
> +}
> +
> +static xmlNodePtr gvir_config_domain_get_metadata_node
> + (GVirConfigDomain *domain, gboolean create)
> +{
> + xmlNodePtr domain_node, metadata_node;
> +
> + domain_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(domain));
> + metadata_node = gvir_config_xml_get_element(domain_node, "metadata", NULL);
> + if (metadata_node == NULL && create)
> + metadata_node = xmlNewChild(domain_node, NULL, (xmlChar *)"metadata", NULL);
> +
> + return metadata_node;
> +}
> +
> +static xmlNodePtr gvir_config_domain_get_custom_xml_node
> + (GVirConfigDomain *domain, const gchar *ns_uri)
> +{
> + xmlNodePtr metadata_node, node;
> +
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL);
> +
> + metadata_node = gvir_config_domain_get_metadata_node(domain, FALSE);
This does the same as gvir_config_xml_element_get_element if I'm not
mistaken.
> + 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.
> +
> +gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain,
> + const gchar *xml,
> + const gchar *ns,
> + const gchar *ns_uri,
> + GError **error)
> +{
> + xmlNodePtr metadata_node, node, old_node;
> + xmlDocPtr doc;
> + xmlNsPtr namespace;
> +
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), FALSE);
> + g_return_val_if_fail(xml != NULL, FALSE);
> + g_return_val_if_fail(ns != NULL, FALSE);
> + g_return_val_if_fail(ns_uri != NULL, FALSE);
> + g_return_val_if_fail(error == NULL || *error == NULL, FALSE);
> +
> + metadata_node = gvir_config_domain_get_metadata_node(domain, TRUE);
> +
> + node = gvir_config_xml_parse(xml, NULL, error);
> + if (error != NULL && *error != NULL)
> + return FALSE;
> +
> + namespace = xmlNewNs(node, (xmlChar *)ns_uri, (xmlChar *)ns);
This can return NULL in case of failure
> + 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).
> +
> + xmlFreeDoc(doc);
> +
> + return TRUE;
> +}
> +
> +gchar *gvir_config_domain_get_custom_xml(GVirConfigDomain *domain,
> + const gchar *ns_uri,
> + GError **error)
> +{
> + xmlNodePtr metadata_node, node;
> + xmlBufferPtr xmlbuf;
> + gchar *xml = NULL;
> +
> + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL);
> +
> + metadata_node = gvir_config_domain_get_metadata_node(domain, FALSE);
This does the same as gvir_config_xml_element_get_element if I'm not
mistaken.
> + if (metadata_node == NULL)
> + return NULL;
> +
> + node = gvir_config_domain_get_custom_xml_node(domain, ns_uri);
> + if (node == NULL)
> + return NULL;
> +
> + xmlbuf = xmlBufferCreate();
> + if (xmlNodeDump(xmlbuf, metadata_node->doc, metadata_node, 0, 0) < 0)
> + gvir_config_set_error (error,
> + GVIR_CONFIG_OBJECT_ERROR,
> + 0,
> + "Failed to convert custom node '%s' to string",
> + node->name);
> + else
> + xml = g_strndup((gchar *)xmlBufferContent(xmlbuf), xmlBufferLength(xmlbuf));
> +
> + xmlBufferFree(xmlbuf);
> +
> + return xml;
> +}
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h
> index 6cdaec9..96ded07 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain.h
> +++ b/libvirt-gconfig/libvirt-gconfig-domain.h
> @@ -126,6 +126,14 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain *domain);
> void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain,
> GVirConfigDomainLifecycleEvent event,
> GVirConfigDomainLifecycleAction action);
> +gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain,
> + const gchar *xml,
> + const gchar *ns,
> + const gchar *ns_uri,
> + GError **error);
> +gchar *gvir_config_domain_get_custom_xml(GVirConfigDomain *domain,
> + const gchar *ns_uri,
> + GError **error);
>
> G_END_DECLS
>
> diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.c b/libvirt-gconfig/libvirt-gconfig-helpers.c
> index c406a49..140a4a0 100644
> --- a/libvirt-gconfig/libvirt-gconfig-helpers.c
> +++ b/libvirt-gconfig/libvirt-gconfig-helpers.c
> @@ -148,7 +148,9 @@ gvir_config_xml_parse(const char *xml, const char *root_node, GError **err)
> "Unable to parse configuration");
> return NULL;
> }
> - if ((!doc->children) || (strcmp((char *)doc->children->name, root_node) != 0)) {
> + if ((!doc->children) ||
> + (root_node != NULL &&
> + (strcmp((char *)doc->children->name, root_node) != 0))) {
I'm not sure if this is directly related to the patch or not, but you could
use g_strcmp0 here instead of adding this root_node check.
> g_set_error(err,
> GVIR_CONFIG_OBJECT_ERROR,
> 0,
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym
> index 88aef57..ab2c7bf 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -14,6 +14,8 @@ LIBVIRT_GCONFIG_0.0.4 {
> gvir_config_domain_new;
> gvir_config_domain_new_from_xml;
> gvir_config_domain_set_clock;
> + gvir_config_domain_set_custom_xml;
> + gvir_config_domain_get_custom_xml;
> gvir_config_domain_get_description;
> gvir_config_domain_set_description;
> gvir_config_domain_get_devices;
> --
> 1.7.7.5
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgpUqoJ8yeZ7F.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list