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

From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org> --- 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); +} + +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); + 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; +} + +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); + 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); + } + + 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); + 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))) { 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

On Fri, Jan 20, 2012 at 11:11:57PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Jan 23, 2012 at 1:01 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
On Fri, Jan 20, 2012 at 11:11:57PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- 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.
Ah good point but OTOH we should ensure that no node in the XML is using non/default namespace so maybe we should just set it for every node that doesn't have a namespace already?
+ 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.
Sure but IMHO code is simpler this way.
+ 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).
But we are not create a GVirConfigObject here at all. -- Regards, Zeeshan Ali (Khattak) FSF member#5124 P.S. Not replying to comments I agree with.

On Mon, Jan 23, 2012 at 04:53:40PM +0200, Zeeshan Ali (Khattak) wrote:
On Mon, Jan 23, 2012 at 1:01 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
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.
Ah good point but OTOH we should ensure that no node in the XML is using non/default namespace so maybe we should just set it for every node that doesn't have a namespace already?
As far as we are concerned, having the root node namespaced is all we care about, so I wouldn't touch the internal nodes unless we have a very good reason for that.
Hmm seeing this loop, gvir_config_object_foreach_child could even be used here with a small helper struct returning the value.
Sure but IMHO code is simpler this way. [..]
But we are not create a GVirConfigObject here at all.
See the upcoming patch, though it seems most of what we are doing here may move to libvirt...
P.S. Not replying to comments I agree with.
eh, this PS was well hidden, I'm only seeing it now :) Christophe
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Based on a patch from Zeeshan Ali (Khattak) <zeeshanak@gnome.org> --- libvirt-gconfig/libvirt-gconfig-domain.c | 60 +++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-domain.h | 7 +++ libvirt-gconfig/libvirt-gconfig-helpers-private.h | 1 + libvirt-gconfig/libvirt-gconfig-helpers.c | 23 ++++++++- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 ++ libvirt-gconfig/libvirt-gconfig-object.c | 32 +++++++++++ libvirt-gconfig/libvirt-gconfig.sym | 2 + 7 files changed, 128 insertions(+), 1 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index 61af625..d943099 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -449,3 +449,63 @@ GList *gvir_config_domain_get_devices(GVirConfigDomain *domain) return data.devices; } + +gboolean gvir_config_domain_set_custom_xml(GVirConfigDomain *domain, + const gchar *xml, + const gchar *ns, + const gchar *ns_uri, + GError **error) +{ + GVirConfigObject *metadata; + GVirConfigObject *custom_xml; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), FALSE); + g_return_val_if_fail(xml != NULL, FALSE); + g_return_val_if_fail(error == NULL || *error == NULL, FALSE); + + metadata = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), "metadata"); + + custom_xml = gvir_config_object_new_from_xml(GVIR_CONFIG_TYPE_OBJECT, NULL, NULL, xml, error); + if (error != NULL && *error != NULL) + return FALSE; + + gvir_config_object_set_namespace(custom_xml, ns, ns_uri, TRUE); + + gvir_config_object_attach_replace(metadata, custom_xml); + + return TRUE; +} + +struct LookupNamespacedNodeData { + const char *ns_uri; + xmlNodePtr node; +}; + +static gboolean lookup_namespaced_node(xmlNodePtr node, gpointer opaque) +{ + struct LookupNamespacedNodeData* data = opaque; + + if (node->ns == NULL) + return TRUE; + + if (g_strcmp0((char *)node->ns->href, data->ns_uri) == 0) { + data->node = node; + return FALSE; + } + + return TRUE; +} + +gchar *gvir_config_domain_get_custom_xml(GVirConfigDomain *domain, + const gchar *ns_uri) +{ + struct LookupNamespacedNodeData data = { NULL, NULL }; + + g_return_val_if_fail(GVIR_CONFIG_IS_DOMAIN(domain), NULL); + g_return_val_if_fail(ns_uri != NULL, NULL); + + data.ns_uri = ns_uri; + gvir_config_object_foreach_child(GVIR_CONFIG_OBJECT(domain), "metadata", + lookup_namespaced_node, &data); + return gvir_config_xml_node_to_string(data.node); +} diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h index 6cdaec9..769d2f0 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.h +++ b/libvirt-gconfig/libvirt-gconfig-domain.h @@ -126,6 +126,13 @@ 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); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-helpers-private.h b/libvirt-gconfig/libvirt-gconfig-helpers-private.h index 96ec02e..514aeb0 100644 --- a/libvirt-gconfig/libvirt-gconfig-helpers-private.h +++ b/libvirt-gconfig/libvirt-gconfig-helpers-private.h @@ -56,6 +56,7 @@ char *gvir_config_xml_get_child_element_content_glib (xmlNode *node, const char *child_name); xmlChar *gvir_config_xml_get_attribute_content(xmlNodePtr node, const char *attr_name); +char *gvir_config_xml_node_to_string(xmlNodePtr node); char *gvir_config_xml_get_attribute_content_glib(xmlNodePtr node, const char *attr_name); const char *gvir_config_genum_get_nick (GType enum_type, gint value); diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.c b/libvirt-gconfig/libvirt-gconfig-helpers.c index c406a49..6500439 100644 --- a/libvirt-gconfig/libvirt-gconfig-helpers.c +++ b/libvirt-gconfig/libvirt-gconfig-helpers.c @@ -148,7 +148,8 @@ 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) || + (g_strcmp0((char *)doc->children->name, root_node) != 0)) { g_set_error(err, GVIR_CONFIG_OBJECT_ERROR, 0, @@ -306,3 +307,23 @@ gvir_config_genum_get_value (GType enum_type, const char *nick, g_return_val_if_reached(default_value); } + +G_GNUC_INTERNAL char * +gvir_config_xml_node_to_string(xmlNodePtr node) +{ + xmlBufferPtr xmlbuf; + char *xml; + + if (node == NULL) + return NULL; + + xmlbuf = xmlBufferCreate(); + if (xmlNodeDump(xmlbuf, node->doc, node, 0, 0) < 0) + return NULL; + else + xml = g_strndup((gchar *)xmlBufferContent(xmlbuf), xmlBufferLength(xmlbuf)); + + xmlBufferFree(xmlbuf); + + return xml; +} diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 781e1a3..8fc9a08 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -90,6 +90,10 @@ void gvir_config_object_foreach_child(GVirConfigObject *object, const char *parent_name, GVirConfigXmlNodeIterator iter_func, gpointer opaque); +gboolean gvir_config_object_set_namespace(GVirConfigObject *object, + const char *ns, + const char *ns_uri, + gboolean recursive); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 2e28208..5449bd6 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -846,3 +846,35 @@ gvir_config_object_remove_attribute(GVirConfigObject *object, status = xmlUnsetProp(object->priv->node, (xmlChar *)attr_name); } while (status == 0); } + +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); +} + +G_GNUC_INTERNAL gboolean +gvir_config_object_set_namespace(GVirConfigObject *object, const char *ns, + const char *ns_uri, gboolean recursive) +{ + xmlNsPtr namespace; + + g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(object), FALSE); + g_return_val_if_fail(ns != NULL, FALSE); + g_return_val_if_fail(ns_uri != NULL, FALSE); + + namespace = xmlNewNs(object->priv->node, + (xmlChar *)ns_uri, (xmlChar *)ns); + if (namespace == NULL) + return FALSE; + if (recursive) { + set_namespace_on_tree(object->priv->node, namespace); + } else { + xmlSetNs(object->priv->node, namespace); + } + return TRUE; +} 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

On Mon, Jan 23, 2012 at 7:19 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Based on a patch from Zeeshan Ali (Khattak) <zeeshanak@gnome.org>
Looks good! I'll test it later. Some minor issues:
+ gvir_config_object_set_namespace(custom_xml, ns, ns_uri, TRUE);
You meant to pass 'FALSE' in the last arg?
+ g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(object), FALSE); + g_return_val_if_fail(ns != NULL, FALSE); + g_return_val_if_fail(ns_uri != NULL, FALSE); + + namespace = xmlNewNs(object->priv->node, + (xmlChar *)ns_uri, (xmlChar *)ns); + if (namespace == NULL) + return FALSE; + if (recursive) { + set_namespace_on_tree(object->priv->node, namespace); + } else { + xmlSetNs(object->priv->node, namespace); + }
You don't need the curly braces here. -- Regards, Zeeshan Ali (Khattak) FSF member#5124

On Mon, Jan 23, 2012 at 11:31:57PM +0200, Zeeshan Ali (Khattak) wrote:
On Mon, Jan 23, 2012 at 7:19 PM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Based on a patch from Zeeshan Ali (Khattak) <zeeshanak@gnome.org>
Looks good! I'll test it later
I've only compile-tested it, if it doesn't work, don't waste time on it, just tell me to test my stuff
+ gvir_config_object_set_namespace(custom_xml, ns, ns_uri, TRUE);
You meant to pass 'FALSE' in the last arg?
Dunno, depends on what we decide to do :) I prefer FALSE, your initial patch uses TRUE so I kept that for now.
+ g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(object), FALSE); + g_return_val_if_fail(ns != NULL, FALSE); + g_return_val_if_fail(ns_uri != NULL, FALSE); + + namespace = xmlNewNs(object->priv->node, + (xmlChar *)ns_uri, (xmlChar *)ns); + if (namespace == NULL) + return FALSE; + if (recursive) { + set_namespace_on_tree(object->priv->node, namespace); + } else { + xmlSetNs(object->priv->node, namespace); + }
You don't need the curly braces here.
I know, but I like them :) I don't mind dropping them Christophe

On Fri, Jan 20, 2012 at 11:11:57PM +0200, Zeeshan Ali (Khattak) wrote:
From: "Zeeshan Ali (Khattak)" <zeeshanak@gnome.org>
--- 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(-)
The API design gets my ACK, will defer to Christophe for the other code review points. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (3)
-
Christophe Fergeau
-
Daniel P. Berrange
-
Zeeshan Ali (Khattak)