[libvirt] [libvirt-glib 01/13] Add GVirConfigXmlDoc type

This class derives from GObject and wraps an xmlDocPtr. It will be useful to refcount xmlDoc so that it can be shared between multiple GVirConfigObject instances. --- libvirt-gconfig/Makefile.am | 4 +- libvirt-gconfig/libvirt-gconfig-xml-doc.c | 137 +++++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-xml-doc.h | 66 ++++++++++++++ 3 files changed, 206 insertions(+), 1 deletions(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-xml-doc.c create mode 100644 libvirt-gconfig/libvirt-gconfig-xml-doc.h diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am index e91c601..8276d07 100644 --- a/libvirt-gconfig/Makefile.am +++ b/libvirt-gconfig/Makefile.am @@ -35,6 +35,7 @@ GCONFIG_HEADER_FILES = \ noinst_HEADERS = \ libvirt-gconfig-helpers-private.h \ libvirt-gconfig-object-private.h + libvirt-gconfig-xml-doc.h GCONFIG_SOURCE_FILES = \ libvirt-gconfig-object.c \ libvirt-gconfig-capabilities.c \ @@ -59,7 +60,8 @@ GCONFIG_SOURCE_FILES = \ libvirt-gconfig-node-device.c \ libvirt-gconfig-secret.c \ libvirt-gconfig-storage-pool.c \ - libvirt-gconfig-storage-vol.c + libvirt-gconfig-storage-vol.c \ + libvirt-gconfig-xml-doc.c libvirt_gconfig_1_0_ladir = $(includedir)/libvirt-gconfig-1.0/libvirt-gconfig libvirt_gconfig_1_0_la_HEADERS = \ diff --git a/libvirt-gconfig/libvirt-gconfig-xml-doc.c b/libvirt-gconfig/libvirt-gconfig-xml-doc.c new file mode 100644 index 0000000..7906158 --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-xml-doc.c @@ -0,0 +1,137 @@ +/* + * libvirt-gobject-config-xml-node.c: libvirt glib integration + * + * Copyright (C) 2011 Red Hat + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Christophe Fergeau <cfergeau@gmail.com> + */ + +#include <config.h> + +#include <string.h> + +#include <libxml/tree.h> + +#include "libvirt-gconfig/libvirt-gconfig.h" +#include "libvirt-gconfig/libvirt-gconfig-xml-doc.h" + +extern gboolean debugFlag; + +#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag)) g_debug(fmt, ## __VA_ARGS__); } while (0) + +#define GVIR_CONFIG_XML_DOC_GET_PRIVATE(obj) \ + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_CONFIG_XML_DOC, GVirConfigXmlDocPrivate)) + +struct _GVirConfigXmlDocPrivate +{ + xmlDocPtr doc; +}; + +G_DEFINE_TYPE(GVirConfigXmlDoc, gvir_config_xml_doc, G_TYPE_OBJECT); + +enum { + PROP_0, + PROP_DOC +}; + +static void gvir_config_xml_doc_get_property(GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ + GVirConfigXmlDoc *xml_doc = GVIR_CONFIG_XML_DOC(object); + + switch (prop_id) { + case PROP_DOC: + g_value_set_pointer(value, xml_doc->priv->doc); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + } +} + +static void gvir_config_xml_doc_set_property(GObject *object, + guint prop_id, + const GValue *value, + GParamSpec *pspec) +{ + GVirConfigXmlDoc *doc = GVIR_CONFIG_XML_DOC(object); + + switch (prop_id) { + case PROP_DOC: + doc->priv->doc = g_value_get_pointer(value); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); + } +} + +static void gvir_config_xml_doc_finalize(GObject *object) +{ + GVirConfigXmlDoc *doc = GVIR_CONFIG_XML_DOC(object); + GVirConfigXmlDocPrivate *priv = doc->priv; + + DEBUG("Finalize GVirConfigXmlDoc=%p", doc); + + if (priv->doc) + xmlFreeDoc(priv->doc); + + G_OBJECT_CLASS(gvir_config_xml_doc_parent_class)->finalize(object); +} + +static void gvir_config_xml_doc_class_init(GVirConfigXmlDocClass *klass) +{ + GObjectClass *object_class = G_OBJECT_CLASS(klass); + + g_type_class_add_private(klass, sizeof(GVirConfigXmlDocPrivate)); + + object_class->finalize = gvir_config_xml_doc_finalize; + object_class->get_property = gvir_config_xml_doc_get_property; + object_class->set_property = gvir_config_xml_doc_set_property; + + g_object_class_install_property(object_class, + PROP_DOC, + g_param_spec_pointer("doc", + "XML Doc", + "The XML doc this config object corresponds to", + G_PARAM_READWRITE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS)); +} + + +static void gvir_config_xml_doc_init(GVirConfigXmlDoc *xml_doc) +{ + GVirConfigXmlDocPrivate *priv; + + DEBUG("Init GVirConfigXmlDoc=%p", xml_doc); + + priv = xml_doc->priv = GVIR_CONFIG_XML_DOC_GET_PRIVATE(xml_doc); + + memset(priv, 0, sizeof(*priv)); +} + + +GVirConfigXmlDoc *gvir_config_xml_doc_new(xmlDocPtr doc) +{ + if (doc == NULL) { + doc = xmlNewDoc((xmlChar *)"1.0"); + } + return GVIR_CONFIG_XML_DOC(g_object_new(GVIR_TYPE_CONFIG_XML_DOC, + "doc", doc, + NULL)); +} diff --git a/libvirt-gconfig/libvirt-gconfig-xml-doc.h b/libvirt-gconfig/libvirt-gconfig-xml-doc.h new file mode 100644 index 0000000..079de1e --- /dev/null +++ b/libvirt-gconfig/libvirt-gconfig-xml-doc.h @@ -0,0 +1,66 @@ +/* + * libvirt-gobject-xml-doc.h: libvirt gobject integration + * + * Copyright (C) 2011 Red Hat + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Christophe Fergeau <cfergeau@gmail.com> + */ + +#if !defined(__LIBVIRT_GCONFIG_H__) && !defined(LIBVIRT_GCONFIG_BUILD) +#error "Only <libvirt-gconfig/libvirt-gconfig.h> can be included directly." +#endif + +#ifndef __LIBVIRT_GCONFIG_XML_DOC_H__ +#define __LIBVIRT_GCONFIG_XML_DOC_H__ + +G_BEGIN_DECLS + +#define GVIR_TYPE_CONFIG_XML_DOC (gvir_config_xml_doc_get_type ()) +#define GVIR_CONFIG_XML_DOC(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_TYPE_CONFIG_XML_DOC, GVirConfigXmlDoc)) +#define GVIR_CONFIG_XML_DOC_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_TYPE_CONFIG_XML_DOC, GVirConfigXmlDocClass)) +#define GVIR_IS_CONFIG_XML_DOC(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_TYPE_CONFIG_XML_DOC)) +#define GVIR_IS_CONFIG_XML_DOC_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_TYPE_CONFIG_XML_DOC)) +#define GVIR_CONFIG_XML_DOC_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_TYPE_CONFIG_XML_DOC, GVirConfigXmlDocClass)) + +typedef struct _GVirConfigXmlDoc GVirConfigXmlDoc; +typedef struct _GVirConfigXmlDocPrivate GVirConfigXmlDocPrivate; +typedef struct _GVirConfigXmlDocClass GVirConfigXmlDocClass; + +struct _GVirConfigXmlDoc +{ + GObject parent; + + GVirConfigXmlDocPrivate *priv; + + /* Do not add fields to this struct */ +}; + +struct _GVirConfigXmlDocClass +{ + GObjectClass parent_class; + + gpointer padding[20]; +}; + + +GType gvir_config_xml_doc_get_type(void); + +GVirConfigXmlDoc *gvir_config_xml_doc_new(xmlDocPtr doc); + +G_END_DECLS + +#endif /* __LIBVIRT_GCONFIG_XML_DOC_H__ */ -- 1.7.7.3

It will be used to refcount the xml data and make sure we only free it when all references have went away. --- libvirt-gconfig/libvirt-gconfig-object.c | 63 ++++++++++++++++++------------ 1 files changed, 38 insertions(+), 25 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index edbaf57..de760dd 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -30,6 +30,7 @@ #include "libvirt-gconfig/libvirt-gconfig.h" #include "libvirt-gconfig/libvirt-gconfig-helpers-private.h" #include "libvirt-gconfig/libvirt-gconfig-object-private.h" +#include "libvirt-gconfig/libvirt-gconfig-xml-doc.h" //extern gboolean debugFlag; @@ -44,6 +45,7 @@ struct _GVirConfigObjectPrivate { gchar *schema; + GVirConfigXmlDoc *doc; xmlNodePtr node; }; @@ -52,7 +54,8 @@ G_DEFINE_ABSTRACT_TYPE(GVirConfigObject, gvir_config_object, G_TYPE_OBJECT); enum { PROP_0, PROP_SCHEMA, - PROP_NODE + PROP_NODE, + PROP_DOC }; @@ -73,8 +76,8 @@ static void gvir_config_object_get_property(GObject *object, GValue *value, GParamSpec *pspec) { - GVirConfigObject *conn = GVIR_CONFIG_OBJECT(object); - GVirConfigObjectPrivate *priv = conn->priv; + GVirConfigObject *obj = GVIR_CONFIG_OBJECT(object); + GVirConfigObjectPrivate *priv = obj->priv; switch (prop_id) { case PROP_SCHEMA: @@ -82,7 +85,11 @@ static void gvir_config_object_get_property(GObject *object, break; case PROP_NODE: - g_value_set_pointer(value, gvir_config_object_get_xml_node(conn)); + g_value_set_pointer(value, gvir_config_object_get_xml_node(obj)); + break; + + case PROP_DOC: + g_value_set_object(value, obj->priv->doc); break; default: @@ -95,8 +102,8 @@ static void gvir_config_object_set_property(GObject *object, const GValue *value, GParamSpec *pspec) { - GVirConfigObject *conn = GVIR_CONFIG_OBJECT(object); - GVirConfigObjectPrivate *priv = conn->priv; + GVirConfigObject *obj = GVIR_CONFIG_OBJECT(object); + GVirConfigObjectPrivate *priv = obj->priv; switch (prop_id) { case PROP_SCHEMA: @@ -104,17 +111,13 @@ static void gvir_config_object_set_property(GObject *object, priv->schema = g_value_dup_string(value); break; - case PROP_NODE: { - xmlNodePtr node; - node = g_value_get_pointer(value); - if ((priv->node != NULL) - && (priv->node->doc != NULL) - && (priv->node->doc != node->doc)) { - xmlFreeDoc(priv->node->doc); - } - priv->node = node; + case PROP_NODE: + priv->node =g_value_get_pointer(value); + break; + + case PROP_DOC: + obj->priv->doc = g_value_dup_object(value); break; - } default: G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); @@ -131,12 +134,14 @@ static void gvir_config_object_finalize(GObject *object) g_free(priv->schema); - /* FIXME: all objects describing a given XML document will share the - * same document so we can't destroy it here like this, we need some - * refcounting to know when to destroy it. - */ - if (priv->node) - xmlFreeDoc(priv->node->doc); + if (priv->doc != NULL) { + g_object_unref(G_OBJECT(priv->doc)); + priv->node = NULL; /* node belongs to doc, make sure not to free it */ + } + if (priv->node != NULL) { + g_assert(priv->node->doc == NULL); + xmlFreeNode(priv->node); + } G_OBJECT_CLASS(gvir_config_object_parent_class)->finalize(object); } @@ -159,9 +164,7 @@ static void gvir_config_object_class_init(GVirConfigObjectClass *klass) G_PARAM_READABLE | G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | - G_PARAM_STATIC_NAME | - G_PARAM_STATIC_NICK | - G_PARAM_STATIC_BLURB)); + G_PARAM_STATIC_STRINGS)); g_object_class_install_property(object_class, PROP_NODE, @@ -172,6 +175,16 @@ static void gvir_config_object_class_init(GVirConfigObjectClass *klass) G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS)); + g_object_class_install_property(object_class, + PROP_DOC, + g_param_spec_object("doc", + "XML Doc", + "The XML doc this config object corresponds to", + GVIR_TYPE_CONFIG_XML_DOC, + G_PARAM_READWRITE | + G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS)); + g_type_class_add_private(klass, sizeof(GVirConfigObjectPrivate)); } -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:08PM +0100, Christophe Fergeau wrote:
It will be used to refcount the xml data and make sure we only free it when all references have went away. --- libvirt-gconfig/libvirt-gconfig-object.c | 63 ++++++++++++++++++------------ 1 files changed, 38 insertions(+), 25 deletions(-)
ACK 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 :|

Now that we have a refcounted GVirConfigXmlDoc type, make sure we use it when creating objects deriving from GVirConfigObject. --- libvirt-gconfig/libvirt-gconfig-object.c | 40 +++++++++++++++++++++--------- 1 files changed, 28 insertions(+), 12 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index de760dd..6cca49d 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -440,29 +440,45 @@ GVirConfigObject *gvir_config_object_new_from_xml(GType type, const gchar *xml, GError **error) { + GVirConfigObject *object; + GVirConfigXmlDoc *doc; xmlNodePtr node; node = gvir_config_xml_parse(xml, root_name, error); if ((error != NULL) && (*error != NULL)) return NULL; - return GVIR_CONFIG_OBJECT(g_object_new(type, - "node", node, - "schema", schema, - NULL)); + doc = gvir_config_xml_doc_new(node->doc); + object = GVIR_CONFIG_OBJECT(g_object_new(type, + "doc", doc, + "node", node, + "schema", schema, + NULL)); + g_object_unref(G_OBJECT(doc)); + + return object; } GVirConfigObject *gvir_config_object_new(GType type, const char *root_name, const char *schema) { - xmlDocPtr doc; + GVirConfigObject *object; + GVirConfigXmlDoc *doc; + xmlDocPtr xml_doc; xmlNodePtr node; - doc = xmlNewDoc((xmlChar *)"1.0"); - node = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); - xmlDocSetRootElement(doc, node); - return GVIR_CONFIG_OBJECT(g_object_new(type, - "node", node, - "schema", schema, - NULL)); + doc = gvir_config_xml_doc_new(NULL); + g_object_get(G_OBJECT(doc), "doc", &xml_doc, NULL); + g_assert(xml_doc != NULL); + node = xmlNewDocNode(xml_doc, NULL, (xmlChar *)root_name, NULL); + xmlDocSetRootElement(xml_doc, node); + object = GVIR_CONFIG_OBJECT(g_object_new(type, + "doc", doc, + "node", node, + "schema", schema, + NULL)); + + g_object_unref(G_OBJECT(doc)); + + return object; } -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:09PM +0100, Christophe Fergeau wrote:
Now that we have a refcounted GVirConfigXmlDoc type, make sure we use it when creating objects deriving from GVirConfigObject. --- libvirt-gconfig/libvirt-gconfig-object.c | 40 +++++++++++++++++++++--------- 1 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index de760dd..6cca49d 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -440,29 +440,45 @@ GVirConfigObject *gvir_config_object_new_from_xml(GType type, const gchar *xml, GError **error) { + GVirConfigObject *object; + GVirConfigXmlDoc *doc; xmlNodePtr node;
node = gvir_config_xml_parse(xml, root_name, error); if ((error != NULL) && (*error != NULL)) return NULL; - return GVIR_CONFIG_OBJECT(g_object_new(type, - "node", node, - "schema", schema, - NULL)); + doc = gvir_config_xml_doc_new(node->doc); + object = GVIR_CONFIG_OBJECT(g_object_new(type, + "doc", doc, + "node", node, + "schema", schema, + NULL)); + g_object_unref(G_OBJECT(doc)); + + return object; }
GVirConfigObject *gvir_config_object_new(GType type, const char *root_name, const char *schema) { - xmlDocPtr doc; + GVirConfigObject *object; + GVirConfigXmlDoc *doc; + xmlDocPtr xml_doc; xmlNodePtr node;
- doc = xmlNewDoc((xmlChar *)"1.0"); - node = xmlNewDocNode(doc, NULL, (xmlChar *)root_name, NULL); - xmlDocSetRootElement(doc, node); - return GVIR_CONFIG_OBJECT(g_object_new(type, - "node", node, - "schema", schema, - NULL)); + doc = gvir_config_xml_doc_new(NULL); + g_object_get(G_OBJECT(doc), "doc", &xml_doc, NULL); + g_assert(xml_doc != NULL); + node = xmlNewDocNode(xml_doc, NULL, (xmlChar *)root_name, NULL); + xmlDocSetRootElement(xml_doc, node); + object = GVIR_CONFIG_OBJECT(g_object_new(type, + "doc", doc, + "node", node, + "schema", schema, + NULL)); + + g_object_unref(G_OBJECT(doc)); + + return object;
ACK 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 :|

Since we want to use GVirConfigObject as a simple wrapper over xmlNodePtr, we need to be able to create instances of this type for "arbitrary" xml nodes. --- libvirt-gconfig/libvirt-gconfig-object.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 6cca49d..b10ffcb 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -49,7 +49,7 @@ struct _GVirConfigObjectPrivate xmlNodePtr node; }; -G_DEFINE_ABSTRACT_TYPE(GVirConfigObject, gvir_config_object, G_TYPE_OBJECT); +G_DEFINE_TYPE(GVirConfigObject, gvir_config_object, G_TYPE_OBJECT); enum { PROP_0, -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:10PM +0100, Christophe Fergeau wrote:
Since we want to use GVirConfigObject as a simple wrapper over xmlNodePtr, we need to be able to create instances of this type for "arbitrary" xml nodes. --- libvirt-gconfig/libvirt-gconfig-object.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 6cca49d..b10ffcb 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -49,7 +49,7 @@ struct _GVirConfigObjectPrivate xmlNodePtr node; };
-G_DEFINE_ABSTRACT_TYPE(GVirConfigObject, gvir_config_object, G_TYPE_OBJECT); +G_DEFINE_TYPE(GVirConfigObject, gvir_config_object, G_TYPE_OBJECT);
enum { PROP_0,
ACK 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 :|

It's needed to be able to keep track of references on the underlying GVirConfigXmlDoc when building a bigger xml tree from smaller separate xml nodes. --- libvirt-gconfig/libvirt-gconfig-object-private.h | 2 ++ libvirt-gconfig/libvirt-gconfig-object.c | 17 +++++++++++++++++ libvirt-gconfig/libvirt-gconfig.sym | 1 + 3 files changed, 20 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 0cedaef..9703150 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -42,6 +42,8 @@ xmlNodePtr gvir_config_object_replace_child(GVirConfigObject *object, void gvir_config_object_set_child(GVirConfigObject *object, xmlNodePtr child); +void gvir_config_object_attach(GVirConfigObject *parent, + GVirConfigObject *child); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index b10ffcb..ed10726 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -482,3 +482,20 @@ GVirConfigObject *gvir_config_object_new(GType type, return object; } + +G_GNUC_INTERNAL void +gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) +{ + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(parent)); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(child)); + + xmlUnlinkNode(child->priv->node); + xmlAddChild(parent->priv->node, child->priv->node); + if (child->priv->doc != NULL) { + g_object_unref(G_OBJECT(child->priv->doc)); + child->priv->doc = NULL; + } + if (parent->priv->doc != NULL) { + child->priv->doc = g_object_ref(G_OBJECT(parent->priv->doc)); + } +} diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ae97a92..ec37cdb 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -117,6 +117,7 @@ LIBVIRT_GOBJECT_0.0.1 { gvir_config_object_get_schema; gvir_config_object_to_xml; gvir_config_object_validate; + gvir_config_object_attach; gvir_config_secret_get_type; gvir_config_secret_new; -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:11PM +0100, Christophe Fergeau wrote:
It's needed to be able to keep track of references on the underlying GVirConfigXmlDoc when building a bigger xml tree from smaller separate xml nodes. --- libvirt-gconfig/libvirt-gconfig-object-private.h | 2 ++ libvirt-gconfig/libvirt-gconfig-object.c | 17 +++++++++++++++++ libvirt-gconfig/libvirt-gconfig.sym | 1 + 3 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 0cedaef..9703150 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -42,6 +42,8 @@ xmlNodePtr gvir_config_object_replace_child(GVirConfigObject *object, void gvir_config_object_set_child(GVirConfigObject *object, xmlNodePtr child);
+void gvir_config_object_attach(GVirConfigObject *parent, + GVirConfigObject *child);
G_END_DECLS
diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index b10ffcb..ed10726 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -482,3 +482,20 @@ GVirConfigObject *gvir_config_object_new(GType type,
return object; } + +G_GNUC_INTERNAL void +gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) +{ + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(parent)); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(child)); + + xmlUnlinkNode(child->priv->node); + xmlAddChild(parent->priv->node, child->priv->node); + if (child->priv->doc != NULL) { + g_object_unref(G_OBJECT(child->priv->doc)); + child->priv->doc = NULL; + } + if (parent->priv->doc != NULL) { + child->priv->doc = g_object_ref(G_OBJECT(parent->priv->doc)); + } +} diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ae97a92..ec37cdb 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -117,6 +117,7 @@ LIBVIRT_GOBJECT_0.0.1 { gvir_config_object_get_schema; gvir_config_object_to_xml; gvir_config_object_validate; + gvir_config_object_attach;
gvir_config_secret_get_type; gvir_config_secret_new;
ACK 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 :|

This allows us to properly refcount the underlying xmlDocPtr (and the associated xmlNodes) to avoid trying to free twice the same xmlNodes when disposing of GVirConfigObject referencing the same nodes. --- libvirt-gconfig/libvirt-gconfig-domain.c | 31 ++++++++++++++--------------- 1 files changed, 15 insertions(+), 16 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index 2c21219..d9cb2e9 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -266,25 +266,21 @@ void gvir_config_domain_set_features(GVirConfigDomain *domain, void gvir_config_domain_set_clock(GVirConfigDomain *domain, GVirConfigDomainClock *klock) { - xmlNodePtr clock_node; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN(domain)); g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_CLOCK(klock)); - clock_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock)); - gvir_config_object_set_child(GVIR_CONFIG_OBJECT(domain), clock_node); + gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(klock)); } void gvir_config_domain_set_os(GVirConfigDomain *domain, GVirConfigDomainOs *os) { - xmlNodePtr os_node; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN(domain)); g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_OS(os)); - os_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(os)); - gvir_config_object_set_child(GVIR_CONFIG_OBJECT(domain), os_node); + gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(os)); } /** @@ -294,18 +290,21 @@ void gvir_config_domain_set_os(GVirConfigDomain *domain, void gvir_config_domain_set_devices(GVirConfigDomain *domain, GList *devices) { - xmlNodePtr devices_node; + GVirConfigObject *devices_node; GList *it; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN(domain)); - devices_node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(domain), - "devices"); + devices_node = gvir_config_object_new(GVIR_TYPE_CONFIG_OBJECT, + "devices", NULL); for (it = devices; it != NULL; it = it->next) { - GVirConfigDomainDevice *device = GVIR_CONFIG_DOMAIN_DEVICE(it->data); - xmlNodePtr node; - - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(device)); - xmlAddChild(devices_node, node); + if (!GVIR_IS_CONFIG_DOMAIN_DEVICE(it->data)) { + g_warn_if_reached(); + continue; + } + gvir_config_object_attach(devices_node, GVIR_CONFIG_OBJECT(it->data)); } + + gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), devices_node); + g_object_unref(G_OBJECT(devices_node)); } -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:12PM +0100, Christophe Fergeau wrote:
This allows us to properly refcount the underlying xmlDocPtr (and the associated xmlNodes) to avoid trying to free twice the same xmlNodes when disposing of GVirConfigObject referencing the same nodes. --- libvirt-gconfig/libvirt-gconfig-domain.c | 31 ++++++++++++++--------------- 1 files changed, 15 insertions(+), 16 deletions(-)
ACK 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 :|

--- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 + libvirt-gconfig/libvirt-gconfig-object.c | 110 ++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig.sym | 2 + 3 files changed, 116 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 9703150..071d45c 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -44,6 +44,10 @@ void gvir_config_object_set_child(GVirConfigObject *object, void gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child); +void gvir_config_object_set_attribute(GVirConfigObject *object, + ...) G_GNUC_NULL_TERMINATED; +void gvir_config_object_set_attribute_with_type(GVirConfigObject *object, + ...) G_GNUC_NULL_TERMINATED; G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index ed10726..67507d8 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -484,6 +484,116 @@ GVirConfigObject *gvir_config_object_new(GType type, } G_GNUC_INTERNAL void +gvir_config_object_set_attribute(GVirConfigObject *object, ...) +{ + xmlDocPtr doc; + va_list args; + + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(object)); + + g_object_get(G_OBJECT(object->priv->doc), "doc", &doc, NULL); + va_start(args, object); + while (TRUE) { + const char *name; + const char *value; + xmlChar *encoded_value; + + name = va_arg(args, const char *); + if (name == NULL) { + break; + } + value = va_arg(args, const char *); + if (value == NULL) { + g_warn_if_reached(); + break; + } + encoded_value = xmlEncodeEntitiesReentrant(doc, (xmlChar*)value); + xmlNewProp(object->priv->node, (xmlChar *)name, encoded_value); + xmlFree(encoded_value); + } + va_end(args); +} + +G_GNUC_INTERNAL void +gvir_config_object_set_attribute_with_type(GVirConfigObject *object, ...) +{ + va_list args; + + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(object)); + + va_start(args, object); + while (TRUE) { + const char *name; + GType attr_type; + char *str; + + + name = va_arg(args, const char *); + if (name == NULL) { + break; + } + + attr_type = va_arg(args, GType); + if (G_TYPE_IS_ENUM(attr_type)) { + int val; + const char *enum_str; + val = va_arg(args, int); + enum_str = gvir_config_genum_get_nick(attr_type, val); + if (enum_str != NULL) { + str = g_strdup(enum_str); + } else { + str = NULL; + } + } else switch (attr_type) { + case G_TYPE_UINT64: { + guint64 val; + val = va_arg(args, guint64); + str = g_strdup_printf("%"G_GUINT64_FORMAT, val); + break; + } + case G_TYPE_UINT: { + guint val; + val = va_arg(args, guint); + str = g_strdup_printf("%u", val); + break; + } + case G_TYPE_INT: { + gint val; + val = va_arg(args, gint); + str = g_strdup_printf("%d", val); + break; + } + case G_TYPE_STRING: { + xmlDocPtr doc; + xmlChar *enc_str; + + str = va_arg(args, char *); + g_object_get(G_OBJECT(object->priv->doc), "doc", &doc, NULL); + enc_str = xmlEncodeEntitiesReentrant(doc, (xmlChar*)str); + str = g_strdup((char *)enc_str); + xmlFree(enc_str); + break; + } + case G_TYPE_BOOLEAN: { + gboolean val; + val = va_arg(args, gboolean); + str = g_strdup_printf("%s", val?"yes":"no"); + break; + } + default: + g_warning("Unhandled type: %s", g_type_name(attr_type)); + g_assert_not_reached(); + } + + if (str != NULL) { + xmlNewProp(object->priv->node, (xmlChar *)name, (xmlChar *)str); + g_free(str); + } + } + va_end(args); +} + +G_GNUC_INTERNAL void gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) { g_return_if_fail(GVIR_IS_CONFIG_OBJECT(parent)); diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index ec37cdb..1ad7fc4 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -118,6 +118,8 @@ LIBVIRT_GOBJECT_0.0.1 { gvir_config_object_to_xml; gvir_config_object_validate; gvir_config_object_attach; + gvir_config_object_set_attribute; + gvir_config_object_set_attribute_with_type; gvir_config_secret_get_type; gvir_config_secret_new; -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:13PM +0100, Christophe Fergeau wrote:
--- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 + libvirt-gconfig/libvirt-gconfig-object.c | 110 ++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig.sym | 2 + 3 files changed, 116 insertions(+), 0 deletions(-)
ACK 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 :|

--- libvirt-gconfig/libvirt-gconfig-domain-clock.c | 42 +++++-------------- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 36 ++++------------- .../libvirt-gconfig-domain-graphics-spice.c | 12 ++---- libvirt-gconfig/libvirt-gconfig-domain-input.c | 26 ++++-------- 4 files changed, 32 insertions(+), 84 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-clock.c b/libvirt-gconfig/libvirt-gconfig-domain-clock.c index b28856c..c78141a 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-clock.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-clock.c @@ -81,52 +81,32 @@ GVirConfigDomainClock *gvir_config_domain_clock_new_from_xml(const gchar *xml, void gvir_config_domain_clock_set_offset(GVirConfigDomainClock *klock, GVirConfigDomainClockOffset offset) { - xmlNodePtr node; - const char *offset_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_CLOCK(klock)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock)); - g_return_if_fail(node != NULL); - offset_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_CLOCK_OFFSET, - offset); - g_return_if_fail(offset_str != NULL); - xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)offset_str); + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(klock), + "offset", + GVIR_TYPE_CONFIG_DOMAIN_CLOCK_OFFSET, + offset, + NULL); } void gvir_config_domain_clock_set_timezone(GVirConfigDomainClock *klock, const char *tz) { - xmlNodePtr node; - xmlChar *encoded_tz; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_CLOCK(klock)); g_return_if_fail(tz != NULL); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock)); - if (node == NULL) - return; - - xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)"timezone"); - encoded_tz = xmlEncodeEntitiesReentrant(node->doc, (xmlChar*)tz); - xmlNewProp(node, (xmlChar*)"timezone", encoded_tz); - xmlFree(encoded_tz); + gvir_config_object_set_attribute(GVIR_CONFIG_OBJECT(klock), + "timezone", tz, NULL); } void gvir_config_domain_clock_set_variable_offset(GVirConfigDomainClock *klock, gint seconds) { - xmlNodePtr node; - char *offset_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_CLOCK(klock)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(klock)); - if (node == NULL) - return; - - xmlNewProp(node, (xmlChar*)"offset", (xmlChar*)"variable"); - offset_str = g_strdup_printf("%d", seconds); - xmlNewProp(node, (xmlChar*)"adjustment", (xmlChar*)offset_str); - g_free(offset_str); + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(klock), + "offset", G_TYPE_STRING, "variable", + "adjustment", G_TYPE_INT, seconds, + NULL); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c b/libvirt-gconfig/libvirt-gconfig-domain-disk.c index 8a9d6e6..dfdb381 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c @@ -81,50 +81,32 @@ GVirConfigDomainDisk *gvir_config_domain_disk_new_from_xml(const gchar *xml, void gvir_config_domain_disk_set_type(GVirConfigDomainDisk *disk, GVirConfigDomainDiskType type) { - xmlNodePtr node; - const char *type_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(disk)); - g_return_if_fail(node != NULL); - type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_DISK_TYPE, - type); - g_return_if_fail(type_str != NULL); - xmlNewProp(node, (xmlChar*)"type", (xmlChar*)type_str); + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(disk), "type", + GVIR_TYPE_CONFIG_DOMAIN_DISK_TYPE, + type, NULL); disk->priv->type = type; } void gvir_config_domain_disk_set_guest_device_type(GVirConfigDomainDisk *disk, GVirConfigDomainDiskGuestDeviceType type) { - xmlNodePtr node; - const char *type_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(disk)); - g_return_if_fail(node != NULL); - type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_DISK_GUEST_DEVICE_TYPE, - type); - g_return_if_fail(type_str != NULL); - xmlNewProp(node, (xmlChar*)"device", (xmlChar*)type_str); + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(disk), "device", + GVIR_TYPE_CONFIG_DOMAIN_DISK_GUEST_DEVICE_TYPE, + type, NULL); } void gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk, GVirConfigDomainDiskSnapshotType type) { - xmlNodePtr node; - const char *type_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(disk)); - g_return_if_fail(node != NULL); - type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_DISK_SNAPSHOT_TYPE, - type); - g_return_if_fail(type_str != NULL); - xmlNewProp(node, (xmlChar*)"snapshot", (xmlChar*)type_str); + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(disk), "snapshot", + GVIR_TYPE_CONFIG_DOMAIN_DISK_SNAPSHOT_TYPE, + type, NULL); } void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk, diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c index e5c2ff8..a5d0775 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c @@ -90,13 +90,9 @@ GVirConfigDomainGraphicsSpice *gvir_config_domain_graphics_spice_new_from_xml(co void gvir_config_domain_graphics_spice_set_port(GVirConfigDomainGraphicsSpice *graphics, unsigned int port) { - xmlNodePtr node; - char *port_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_GRAPHICS_SPICE(graphics)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(graphics)); - g_return_if_fail(node != NULL); - port_str = g_strdup_printf("%u", port); - xmlNewProp(node, (xmlChar*)"port", (xmlChar*)port_str); - g_free(port_str); + + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(graphics), + "port", G_TYPE_UINT, port, + NULL); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-input.c b/libvirt-gconfig/libvirt-gconfig-domain-input.c index ad11472..83bc26d 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-input.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-input.c @@ -81,29 +81,19 @@ GVirConfigDomainInput *gvir_config_domain_input_new_from_xml(const gchar *xml, void gvir_config_domain_input_set_device_type(GVirConfigDomainInput *input, GVirConfigDomainInputDeviceType type) { - xmlNodePtr node; - const char *type_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_INPUT(input)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(input)); - g_return_if_fail(node != NULL); - type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_INPUT_DEVICE_TYPE, - type); - g_return_if_fail(type_str != NULL); - xmlNewProp(node, (xmlChar*)"type", (xmlChar*)type_str); + + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(input), "type", + GVIR_TYPE_CONFIG_DOMAIN_INPUT_DEVICE_TYPE, + type, NULL); } void gvir_config_domain_input_set_bus(GVirConfigDomainInput *input, GVirConfigDomainInputBus bus) { - xmlNodePtr node; - const char *bus_str; - g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_INPUT(input)); - node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(input)); - g_return_if_fail(node != NULL); - bus_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_INPUT_BUS, - bus); - g_return_if_fail(bus_str != NULL); - xmlNewProp(node, (xmlChar*)"bus", (xmlChar*)bus_str); + + gvir_config_object_set_attribute_with_type(GVIR_CONFIG_OBJECT(input), "bus", + GVIR_TYPE_CONFIG_DOMAIN_INPUT_BUS, + bus, NULL); } -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:14PM +0100, Christophe Fergeau wrote:
--- libvirt-gconfig/libvirt-gconfig-domain-clock.c | 42 +++++-------------- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 36 ++++------------- .../libvirt-gconfig-domain-graphics-spice.c | 12 ++---- libvirt-gconfig/libvirt-gconfig-domain-input.c | 26 ++++-------- 4 files changed, 32 insertions(+), 84 deletions(-)
ACK 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 :|

Now that we have gvir_config_object_set_attribute, we can use the gvir_config_object_new{_from_xml} helpers. --- .../libvirt-gconfig-domain-interface-network.c | 29 ++++++++----------- 1 files changed, 12 insertions(+), 17 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c b/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c index e559df4..9ccf41a 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c @@ -61,30 +61,25 @@ static void gvir_config_domain_interface_network_init(GVirConfigDomainInterfaceN GVirConfigDomainInterfaceNetwork *gvir_config_domain_interface_network_new(void) { - xmlDocPtr doc; - xmlNodePtr node; - - doc = xmlNewDoc((xmlChar *)"1.0"); - node= xmlNewDocNode(doc, NULL, (xmlChar *)"interface", NULL); - xmlNewProp(doc->children, (xmlChar*)"type", (xmlChar*)"network"); - xmlDocSetRootElement(doc, node); - return GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(g_object_new(GVIR_TYPE_CONFIG_DOMAIN_INTERFACE_NETWORK, - "node", node, - NULL)); + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN_INTERFACE_NETWORK, + "interface", NULL); + gvir_config_object_set_attribute(object, "type", "network", NULL); + return GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(object); } GVirConfigDomainInterfaceNetwork *gvir_config_domain_interface_network_new_from_xml(const gchar *xml, GError **error) { - xmlNodePtr node; + GVirConfigObject *object; - node = gvir_config_xml_parse(xml, "interface", error); - if ((error != NULL) && (*error != NULL)) + object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_DOMAIN_INTERFACE_NETWORK, + "interface", NULL, xml, error); + if (object == NULL) return NULL; - xmlNewProp(node, (xmlChar*)"type", (xmlChar*)"network"); - return GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(g_object_new(GVIR_TYPE_CONFIG_DOMAIN_INTERFACE_NETWORK, - "node", node, - NULL)); + gvir_config_object_set_attribute(object, "type", "network", NULL); + return GVIR_CONFIG_DOMAIN_INTERFACE_NETWORK(object); } void gvir_config_domain_interface_network_set_source(GVirConfigDomainInterfaceNetwork *interface, -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:15PM +0100, Christophe Fergeau wrote:
Now that we have gvir_config_object_set_attribute, we can use the gvir_config_object_new{_from_xml} helpers. --- .../libvirt-gconfig-domain-interface-network.c | 29 ++++++++----------- 1 files changed, 12 insertions(+), 17 deletions(-)
ACK 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 :|

Now that we have gvir_config_object_set_attribute, we can use the gvir_config_object_new{_from_xml} helpers. --- .../libvirt-gconfig-domain-graphics-spice.c | 34 +++++++++----------- 1 files changed, 15 insertions(+), 19 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c index a5d0775..bea85e7 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.c @@ -61,30 +61,26 @@ static void gvir_config_domain_graphics_spice_init(GVirConfigDomainGraphicsSpice GVirConfigDomainGraphicsSpice *gvir_config_domain_graphics_spice_new(void) { - xmlDocPtr doc; - xmlNodePtr node; - - doc = xmlNewDoc((xmlChar *)"1.0"); - node= xmlNewDocNode(doc, NULL, (xmlChar *)"graphics", NULL); - xmlNewProp(doc->children, (xmlChar*)"type", (xmlChar*)"spice"); - xmlDocSetRootElement(doc, node); - return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE(g_object_new(GVIR_TYPE_CONFIG_DOMAIN_GRAPHICS_SPICE, - "node", node, - NULL)); + GVirConfigObject *object; + + object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN_GRAPHICS_SPICE, + "graphics", NULL); + gvir_config_object_set_attribute(object, "type", "spice", NULL); + return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE(object); } -GVirConfigDomainGraphicsSpice *gvir_config_domain_graphics_spice_new_from_xml(const gchar *xml, - GError **error) +GVirConfigDomainGraphicsSpice * +gvir_config_domain_graphics_spice_new_from_xml(const gchar *xml, + GError **error) { - xmlNodePtr node; + GVirConfigObject *object; - node = gvir_config_xml_parse(xml, "graphics", error); - if ((error != NULL) && (*error != NULL)) + object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_DOMAIN_GRAPHICS_SPICE, + "graphics", NULL, xml, error); + if (object == NULL) return NULL; - xmlNewProp(node, (xmlChar*)"type", (xmlChar*)"spice"); - return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE(g_object_new(GVIR_TYPE_CONFIG_DOMAIN_GRAPHICS_SPICE, - "node", node, - NULL)); + gvir_config_object_set_attribute(object, "type", "spice", NULL); + return GVIR_CONFIG_DOMAIN_GRAPHICS_SPICE(object); } void gvir_config_domain_graphics_spice_set_port(GVirConfigDomainGraphicsSpice *graphics, -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:16PM +0100, Christophe Fergeau wrote:
Now that we have gvir_config_object_set_attribute, we can use the gvir_config_object_new{_from_xml} helpers. --- .../libvirt-gconfig-domain-graphics-spice.c | 34 +++++++++----------- 1 files changed, 15 insertions(+), 19 deletions(-)
ACK 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 :|

This makes it possible to use the other GVirConfigObject helpers with the new child, and hides libxml from one more place in our API. --- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 29 ++++++++++++---------- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 +- libvirt-gconfig/libvirt-gconfig-object.c | 13 +++++++-- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c b/libvirt-gconfig/libvirt-gconfig-domain-disk.c index dfdb381..0c42384 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c @@ -142,44 +142,47 @@ void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk, void gvir_config_domain_disk_set_driver_name(GVirConfigDomainDisk *disk, const char *driver_name) { - xmlNodePtr node; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(disk), "driver"); - g_return_if_fail(node != NULL); - xmlNewProp(node, (xmlChar*)"name", (xmlChar*)driver_name); - + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute(node, "name", driver_name, NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_disk_set_driver_type(GVirConfigDomainDisk *disk, const char *driver_type) { - xmlNodePtr node; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(disk), "driver"); - g_return_if_fail(node != NULL); - xmlNewProp(node, (xmlChar*)"type", (xmlChar*)driver_type); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute(node, "type", driver_type, NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_disk_set_target_bus(GVirConfigDomainDisk *disk, const char *bus) { - xmlNodePtr node; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(disk), "target"); - g_return_if_fail(node != NULL); - xmlNewProp(node, (xmlChar*)"bus", (xmlChar*)bus); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute(node, "bus", bus, NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_disk_set_target_dev(GVirConfigDomainDisk *disk, const char *dev) { - xmlNodePtr node; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(disk), "target"); - g_return_if_fail(node != NULL); - xmlNewProp(node, (xmlChar*)"dev", (xmlChar*)dev); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute(node, "dev", dev, NULL); + g_object_unref(G_OBJECT(node)); } diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 071d45c..8f9b943 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -35,8 +35,8 @@ void gvir_config_object_set_node_content(GVirConfigObject *object, void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, const char *node_name, guint64 value); -xmlNodePtr gvir_config_object_add_child(GVirConfigObject *object, - const char *child_name); +GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object, + const char *child_name); xmlNodePtr gvir_config_object_replace_child(GVirConfigObject *object, const char *child_name); void gvir_config_object_set_child(GVirConfigObject *object, diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 67507d8..70d0608 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -341,7 +341,7 @@ gvir_config_object_set_child(GVirConfigObject *object, xmlNodePtr child) gvir_config_object_set_child_internal(object, child, TRUE); } -G_GNUC_INTERNAL xmlNodePtr +G_GNUC_INTERNAL GVirConfigObject * gvir_config_object_add_child(GVirConfigObject *object, const char *child_name) { @@ -349,16 +349,23 @@ gvir_config_object_add_child(GVirConfigObject *object, xmlNodePtr old_node; g_return_val_if_fail(GVIR_IS_CONFIG_OBJECT(object), NULL); + g_return_val_if_fail(child_name != NULL, NULL); new_node = xmlNewDocNode(NULL, NULL, (xmlChar *)child_name, NULL); old_node = gvir_config_object_set_child_internal(object, new_node, FALSE); if (old_node != NULL) { xmlFreeNode(new_node); - return old_node; + return GVIR_CONFIG_OBJECT(g_object_new(GVIR_TYPE_CONFIG_OBJECT, + "doc", object->priv->doc, + "node", old_node, + NULL)); } - return new_node; + return GVIR_CONFIG_OBJECT(g_object_new(GVIR_TYPE_CONFIG_OBJECT, + "doc", object->priv->doc, + "node", new_node, + NULL)); } G_GNUC_INTERNAL xmlNodePtr -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:17PM +0100, Christophe Fergeau wrote:
This makes it possible to use the other GVirConfigObject helpers with the new child, and hides libxml from one more place in our API. --- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 29 ++++++++++++---------- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 +- libvirt-gconfig/libvirt-gconfig-object.c | 13 +++++++-- 3 files changed, 28 insertions(+), 18 deletions(-)
ACK 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 :|

This makes it possible to use the other GVirConfigObject helpers with the new child, and hides libxml from one more place in our API. --- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 9 +++- .../libvirt-gconfig-domain-interface-network.c | 7 ++- libvirt-gconfig/libvirt-gconfig-domain-os.c | 46 +++++++++---------- libvirt-gconfig/libvirt-gconfig-domain-video.c | 14 +++--- libvirt-gconfig/libvirt-gconfig-domain.c | 12 +++-- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 +- libvirt-gconfig/libvirt-gconfig-object.c | 14 ++++-- 7 files changed, 57 insertions(+), 49 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-disk.c b/libvirt-gconfig/libvirt-gconfig-domain-disk.c index 0c42384..53a80f3 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-disk.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-disk.c @@ -112,13 +112,13 @@ void gvir_config_domain_disk_set_snapshot_type(GVirConfigDomainDisk *disk, void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk, const char *source) { - xmlNodePtr source_node; + GVirConfigObject *source_node; const char *attribute_name; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_DISK(disk)); source_node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(disk), "source"); - g_return_if_fail(source_node != NULL); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(source_node)); switch (disk->priv->type) { case GVIR_CONFIG_DOMAIN_DISK_FILE: @@ -136,7 +136,10 @@ void gvir_config_domain_disk_set_source(GVirConfigDomainDisk *disk, default: g_return_if_reached(); } - xmlNewProp(source_node, (xmlChar*)attribute_name, (xmlChar*)source); + gvir_config_object_set_attribute(source_node, + attribute_name, source, + NULL); + g_object_unref(G_OBJECT(source_node)); } void gvir_config_domain_disk_set_driver_name(GVirConfigDomainDisk *disk, diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c b/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c index 9ccf41a..d9e00fe 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface-network.c @@ -85,12 +85,13 @@ GVirConfigDomainInterfaceNetwork *gvir_config_domain_interface_network_new_from_ void gvir_config_domain_interface_network_set_source(GVirConfigDomainInterfaceNetwork *interface, const char *source) { - xmlNodePtr source_node; + GVirConfigObject *source_node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_INTERFACE_NETWORK(interface)); source_node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(interface), "source"); - g_return_if_fail(source_node != NULL); - xmlNewProp(source_node, (xmlChar*)"network", (xmlChar*)source); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(source_node)); + gvir_config_object_set_attribute(source_node, "network", source, NULL); + g_object_unref(G_OBJECT(source_node)); } diff --git a/libvirt-gconfig/libvirt-gconfig-domain-os.c b/libvirt-gconfig/libvirt-gconfig-domain-os.c index f1802ae..9110d6a 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-os.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-os.c @@ -79,16 +79,15 @@ GVirConfigDomainOs *gvir_config_domain_os_new_from_xml(const gchar *xml, GError void gvir_config_domain_os_set_os_type(GVirConfigDomainOs *os, GVirConfigDomainOsType type) { - xmlNodePtr node; const char *type_str; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_OS(os)); - node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(os), "type"); - g_return_if_fail(node != NULL); type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_OS_TYPE, type); g_return_if_fail(type_str != NULL); - xmlNodeSetContent(node, (xmlChar*)type_str); + + gvir_config_object_set_node_content(GVIR_CONFIG_OBJECT(os), + "type", type_str); } void gvir_config_domain_os_set_loader(GVirConfigDomainOs *os, @@ -101,47 +100,46 @@ void gvir_config_domain_os_set_loader(GVirConfigDomainOs *os, void gvir_config_domain_os_enable_boot_menu(GVirConfigDomainOs *os, gboolean enable) { - xmlNodePtr node; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_OS(os)); node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(os), "bootmenu"); - g_return_if_fail(node != NULL); - if (enable) - xmlNewProp(node, (xmlChar*)"enable", (xmlChar*)"yes"); - else - xmlNewProp(node, (xmlChar*)"enable", (xmlChar*)"no"); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute_with_type(node, "enable", + G_TYPE_BOOLEAN, enable, + NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_os_bios_enable_serial(GVirConfigDomainOs *os, gboolean enable) { - xmlNodePtr node; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_OS(os)); node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(os), "bios"); - g_return_if_fail(node != NULL); - if (enable) - xmlNewProp(node, (xmlChar*)"useserial", (xmlChar*)"yes"); - else - xmlNewProp(node, (xmlChar*)"useserial", (xmlChar*)"no"); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute_with_type(node, "useserial", + G_TYPE_BOOLEAN, enable, + NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_os_set_smbios_mode(GVirConfigDomainOs *os, - GVirConfigDomainOsSmBiosMode mode) + GVirConfigDomainOsSmBiosMode mode) { - xmlNodePtr node; - const char *mode_str; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_OS(os)); node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(os), "smbios"); - g_return_if_fail(node != NULL); - mode_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_OS_SM_BIOS_MODE, - mode); - if (mode_str != NULL) - xmlNewProp(node, (xmlChar*)"mode", (xmlChar*)mode_str); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute_with_type(node, "mode", + GVIR_TYPE_CONFIG_DOMAIN_OS_SM_BIOS_MODE, + mode, NULL); + g_object_unref(G_OBJECT(node)); } /** diff --git a/libvirt-gconfig/libvirt-gconfig-domain-video.c b/libvirt-gconfig/libvirt-gconfig-domain-video.c index 8c6c9fb..7276e02 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-video.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-video.c @@ -81,17 +81,17 @@ GVirConfigDomainVideo *gvir_config_domain_video_new_from_xml(const gchar *xml, void gvir_config_domain_video_set_model(GVirConfigDomainVideo *video, GVirConfigDomainVideoModel model) { - xmlNodePtr node; - const char *model_str; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_VIDEO(video)); node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(video), "model"); - g_return_if_fail(node != NULL); - model_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_VIDEO_MODEL, - model); - g_return_if_fail(model_str != NULL); - xmlNewProp(node, (xmlChar*)"type", (xmlChar*)model_str); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node)); + gvir_config_object_set_attribute_with_type(node, "type", + GVIR_TYPE_CONFIG_DOMAIN_VIDEO_MODEL, + model, + NULL); + g_object_unref(G_OBJECT(node)); } void gvir_config_domain_video_set_vram(GVirConfigDomainVideo *video, diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index d9cb2e9..84b7401 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -249,17 +249,19 @@ GStrv gvir_config_domain_get_features(GVirConfigDomain *domain) void gvir_config_domain_set_features(GVirConfigDomain *domain, const GStrv features) { - xmlNodePtr features_node; + GVirConfigObject *features_node; GStrv it; features_node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(domain), "features"); + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(features_node)); for (it = features; *it != NULL; it++) { - xmlNodePtr node; - - node = xmlNewDocNode(features_node->doc, NULL, (xmlChar *)*it, NULL); - xmlAddChild(features_node, node); + GVirConfigObject *feature; + feature = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(features_node), + *it); + g_object_unref(G_OBJECT(feature)); } + g_object_unref(G_OBJECT(features_node)); g_object_notify(G_OBJECT(domain), "features"); } diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 8f9b943..f196be6 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -37,8 +37,8 @@ void gvir_config_object_set_node_content_uint64(GVirConfigObject *object, guint64 value); GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object, const char *child_name); -xmlNodePtr gvir_config_object_replace_child(GVirConfigObject *object, - const char *child_name); +GVirConfigObject *gvir_config_object_replace_child(GVirConfigObject *object, + const char *child_name); void gvir_config_object_set_child(GVirConfigObject *object, xmlNodePtr child); diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 70d0608..72203be 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -368,7 +368,7 @@ gvir_config_object_add_child(GVirConfigObject *object, NULL)); } -G_GNUC_INTERNAL xmlNodePtr +G_GNUC_INTERNAL GVirConfigObject * gvir_config_object_replace_child(GVirConfigObject *object, const char *child_name) { @@ -380,7 +380,10 @@ gvir_config_object_replace_child(GVirConfigObject *object, new_node = xmlNewDocNode(NULL, NULL, (xmlChar *)child_name, NULL); gvir_config_object_set_child_internal(object, new_node, TRUE); - return new_node; + return GVIR_CONFIG_OBJECT(g_object_new(GVIR_TYPE_CONFIG_OBJECT, + "doc", object->priv->doc, + "node", new_node, + NULL)); } G_GNUC_INTERNAL void @@ -388,8 +391,8 @@ gvir_config_object_set_node_content(GVirConfigObject *object, const char *node_name, const char *value) { - xmlNodePtr node; xmlChar *encoded_data; + GVirConfigObject *node; g_return_if_fail(GVIR_IS_CONFIG_OBJECT(object)); g_return_if_fail(node_name != NULL); @@ -397,10 +400,11 @@ gvir_config_object_set_node_content(GVirConfigObject *object, node = gvir_config_object_replace_child(object, node_name); g_return_if_fail(node != NULL); - encoded_data = xmlEncodeEntitiesReentrant(node->doc, + encoded_data = xmlEncodeEntitiesReentrant(node->priv->node->doc, (xmlChar *)value); - xmlNodeSetContent(node, encoded_data); + xmlNodeSetContent(node->priv->node, encoded_data); xmlFree(encoded_data); + g_object_unref(G_OBJECT(node)); } G_GNUC_INTERNAL void -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:18PM +0100, Christophe Fergeau wrote:
This makes it possible to use the other GVirConfigObject helpers with the new child, and hides libxml from one more place in our API. --- libvirt-gconfig/libvirt-gconfig-domain-disk.c | 9 +++- .../libvirt-gconfig-domain-interface-network.c | 7 ++- libvirt-gconfig/libvirt-gconfig-domain-os.c | 46 +++++++++---------- libvirt-gconfig/libvirt-gconfig-domain-video.c | 14 +++--- libvirt-gconfig/libvirt-gconfig-domain.c | 12 +++-- libvirt-gconfig/libvirt-gconfig-object-private.h | 4 +- libvirt-gconfig/libvirt-gconfig-object.c | 14 ++++-- 7 files changed, 57 insertions(+), 49 deletions(-)
ACK 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 :|

Before the introduction of GVirConfigXmlDoc, we couldn't destroy the objects deriving from GVirConfigObject without memory corruption. Now that we have GVirConfigXmlDoc, we can make sure the memory we allocate for our various GVirConfigObject instances are properly cleaned up. --- libvirt-gconfig/tests/test-domain-create.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/tests/test-domain-create.c b/libvirt-gconfig/tests/test-domain-create.c index bade013..9491f36 100644 --- a/libvirt-gconfig/tests/test-domain-create.c +++ b/libvirt-gconfig/tests/test-domain-create.c @@ -66,6 +66,7 @@ int main(void) klock = gvir_config_domain_clock_new(); gvir_config_domain_clock_set_offset(klock, GVIR_CONFIG_DOMAIN_CLOCK_UTC); gvir_config_domain_set_clock(domain, klock); + g_object_unref(G_OBJECT(klock)); /* os node */ GVirConfigDomainOs *os; @@ -82,6 +83,7 @@ int main(void) g_list_free(devices); devices = NULL; gvir_config_domain_set_os(domain, os); + g_object_unref(G_OBJECT(os)); /* disk node */ GVirConfigDomainDisk *disk; @@ -130,6 +132,7 @@ int main(void) gvir_config_domain_set_devices(domain, devices); + g_list_foreach(devices, (GFunc)g_object_unref, NULL); g_list_free(devices); devices = NULL; -- 1.7.7.3

On Mon, Nov 28, 2011 at 04:32:19PM +0100, Christophe Fergeau wrote:
Before the introduction of GVirConfigXmlDoc, we couldn't destroy the objects deriving from GVirConfigObject without memory corruption. Now that we have GVirConfigXmlDoc, we can make sure the memory we allocate for our various GVirConfigObject instances are properly cleaned up. --- libvirt-gconfig/tests/test-domain-create.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
ACK 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 :|

On Mon, Nov 28, 2011 at 04:32:07PM +0100, Christophe Fergeau wrote:
This class derives from GObject and wraps an xmlDocPtr. It will be useful to refcount xmlDoc so that it can be shared between multiple GVirConfigObject instances. --- libvirt-gconfig/Makefile.am | 4 +- libvirt-gconfig/libvirt-gconfig-xml-doc.c | 137 +++++++++++++++++++++++++++++ libvirt-gconfig/libvirt-gconfig-xml-doc.h | 66 ++++++++++++++ 3 files changed, 206 insertions(+), 1 deletions(-) create mode 100644 libvirt-gconfig/libvirt-gconfig-xml-doc.c create mode 100644 libvirt-gconfig/libvirt-gconfig-xml-doc.h
ACK
+extern gboolean debugFlag; + +#define DEBUG(fmt, ...) do { if (G_UNLIKELY(debugFlag)) g_debug(fmt, ## __VA_ARGS__); } while (0)
This can be deleted before pushing to GIT
+ + DEBUG("Finalize GVirConfigXmlDoc=%p", doc);
..just call g_debug() directly (Likewise if it occurs in any other patches in this series). Regards, 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 (2)
-
Christophe Fergeau
-
Daniel P. Berrange