On Fri, Oct 07, 2011 at 11:41:00AM +0200, Christophe Fergeau wrote:
> This commit changes gvir_config_domain_new_from_xml not to operate
> on an existing object. This makes it possible to call it to create
> an appropriate xmlNodePtr object which can then be used to construct
> an object derived from GVirConfigObject. This will also makes it
> possible to remove GVirConfigObject::doc in a subsequent commit.
> ---
> libvirt-gconfig/libvirt-gconfig-domain.c | 16 +++++-----
> libvirt-gconfig/libvirt-gconfig-domain.h | 2 +-
> libvirt-gconfig/libvirt-gconfig-object.c | 41 +++++-----------------------
> libvirt-gconfig/libvirt-gconfig-object.h | 3 ++
> libvirt-gconfig/tests/test-domain-parse.c | 6 +++-
> libvirt-gobject/libvirt-gobject-domain.c | 2 +-
> 6 files changed, 26 insertions(+), 44 deletions(-)
>
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c
> index 00cab80..ffd707d 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain.c
> +++ b/libvirt-gconfig/libvirt-gconfig-domain.c
> @@ -112,10 +112,16 @@ static void gvir_config_domain_init(GVirConfigDomain *conn)
> }
>
>
> -GVirConfigDomain *gvir_config_domain_new_from_xml(const gchar *xml)
> +GVirConfigDomain *gvir_config_domain_new_from_xml(const gchar *xml,
> + GError **error)
> {
> + xmlNodePtr node;
> +
> + node = gvir_config_xml_parse(xml, "domain", error);
> + if ((error != NULL) && (*error != NULL))
> + return NULL;
> return GVIR_CONFIG_DOMAIN(g_object_new(GVIR_TYPE_CONFIG_DOMAIN,
> - "doc", xml,
> + "node", node,
> "schema", DATADIR "/libvirt/schemas/domain.rng",
> NULL));
> }
> @@ -132,10 +138,6 @@ GVirConfigDomain *gvir_config_domain_new(void)
> NULL));
> }
>
> -/* FIXME: do we add a GError ** to all getters in case there's an XML
> - * parsing error? Doesn't work with gobject properties
> - * => have a function to test if an error has occurred a la cairo?
> - */
> char *gvir_config_domain_get_name(GVirConfigDomain *domain)
> {
> xmlNodePtr node;
> @@ -145,7 +147,6 @@ char *gvir_config_domain_get_name(GVirConfigDomain *domain)
> return NULL;
>
> return gvir_config_xml_get_child_element_content_glib(node, "name");
> -
> }
>
> void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name)
> @@ -164,7 +165,6 @@ void gvir_config_domain_set_name(GVirConfigDomain *domain, const char *name)
> xmlFree(encoded_name);
>
> old_node = gvir_config_xml_get_element(parent_node, "name", NULL);
> -
> if (old_node) {
> old_node = xmlReplaceNode(old_node, new_node);
> xmlFreeNode(old_node);
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain.h b/libvirt-gconfig/libvirt-gconfig-domain.h
> index f6ceef1..b5ae050 100644
> --- a/libvirt-gconfig/libvirt-gconfig-domain.h
> +++ b/libvirt-gconfig/libvirt-gconfig-domain.h
> @@ -59,7 +59,7 @@ struct _GVirConfigDomainClass
>
> GType gvir_config_domain_get_type(void);
>
> -GVirConfigDomain *gvir_config_domain_new_from_xml(const gchar *xml);
> +GVirConfigDomain *gvir_config_domain_new_from_xml(const gchar *xml, GError **error);
> GVirConfigDomain *gvir_config_domain_new(void);
>
> char *gvir_config_domain_get_name(GVirConfigDomain *domain);
> diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c
> index b7829c9..bcb622a 100644
> --- a/libvirt-gconfig/libvirt-gconfig-object.c
> +++ b/libvirt-gconfig/libvirt-gconfig-object.c
> @@ -26,10 +26,10 @@
> #include <string.h>
>
> #include <libxml/relaxng.h>
> -#include <libxml/xmlerror.h>
>
> #include "libvirt-gconfig/libvirt-gconfig.h"
>
> +
> //extern gboolean debugFlag;
> gboolean debugFlag;
>
> @@ -67,6 +67,7 @@ static void gvir_xml_structured_error_nop(void *userData G_GNUC_UNUSED,
> {
> }
>
> +
> static void gvir_config_object_get_property(GObject *object,
> guint prop_id,
> GValue *value,
> @@ -93,7 +94,6 @@ static void gvir_config_object_get_property(GObject *object,
> }
> }
>
> -
> static void gvir_config_object_set_property(GObject *object,
> guint prop_id,
> const GValue *value,
> @@ -209,34 +209,6 @@ static void gvir_config_object_init(GVirConfigObject *conn)
> memset(priv, 0, sizeof(*priv));
> }
>
> -static void
> -gvir_config_object_parse(GVirConfigObject *config,
> - GError **err)
> -{
> - GVirConfigObjectPrivate *priv = config->priv;
> - xmlDocPtr doc;
> - if (priv->node)
> - return;
> -
> - if (!priv->doc) {
> - *err = g_error_new(GVIR_CONFIG_OBJECT_ERROR,
> - 0,
> - "%s",
> - "No XML document to parse");
> - return;
> - }
> -
> - doc = xmlParseMemory(priv->doc, strlen(priv->doc));
> - if (!doc) {
> - *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> - 0,
> - "%s",
> - "Unable to parse configuration");
> - }
> - priv->node = doc->children;
> -}
> -
> -
> void gvir_config_object_validate(GVirConfigObject *config,
> GError **err)
> {
> @@ -248,9 +220,13 @@ void gvir_config_object_validate(GVirConfigObject *config,
> xmlSetGenericErrorFunc(NULL, gvir_xml_generic_error_nop);
> xmlSetStructuredErrorFunc(NULL, gvir_xml_structured_error_nop);
>
> - gvir_config_object_parse(config, err);
> - if (*err)
> + if (!priv->node) {
> + *err = gvir_xml_error_new(GVIR_CONFIG_OBJECT_ERROR,
> + 0,
> + "%s",
> + "No XML document associated with this config object");
> return;
> + }
>
> rngParser = xmlRelaxNGNewParserCtxt(priv->schema);
> if (!rngParser) {
> @@ -333,6 +309,5 @@ const gchar *gvir_config_object_get_schema(GVirConfigObject *config)
> xmlNodePtr gvir_config_object_get_xml_node(GVirConfigObject *config,
> GError **error)
> {
> - gvir_config_object_parse(config, error);
> return config->priv->node;
> }
> diff --git a/libvirt-gconfig/libvirt-gconfig-object.h b/libvirt-gconfig/libvirt-gconfig-object.h
> index 40480ba..98a05cb 100644
> --- a/libvirt-gconfig/libvirt-gconfig-object.h
> +++ b/libvirt-gconfig/libvirt-gconfig-object.h
> @@ -68,6 +68,9 @@ const gchar *gvir_config_object_get_doc(GVirConfigObject *config);
> const gchar *gvir_config_object_get_schema(GVirConfigObject *config);
> xmlNodePtr gvir_config_object_get_xml_node(GVirConfigObject *config, GError **error);
>
> +/* FIXME: move to a libvirt-gconfig-helpers.h file? */
> +xmlNodePtr gvir_config_object_parse(const char *xml, const char *root_node, GError **err);
> +
> G_END_DECLS
>
> #endif /* __LIBVIRT_GCONFIG_OBJECT_H__ */
> diff --git a/libvirt-gconfig/tests/test-domain-parse.c b/libvirt-gconfig/tests/test-domain-parse.c
> index 3a36144..57b1875 100644
> --- a/libvirt-gconfig/tests/test-domain-parse.c
> +++ b/libvirt-gconfig/tests/test-domain-parse.c
> @@ -50,7 +50,11 @@ int main(int argc, char **argv)
>
> g_type_init();
>
> - domain = gvir_config_domain_new_from_xml(xml);
> + domain = gvir_config_domain_new_from_xml(xml, &error);
> + if (error != NULL) {
> + g_print("Couldn't parse %s: %s\n", argv[1], error->message);
> + return 3;
> + }
> g_assert(domain != NULL);
> name = gvir_config_domain_get_name(domain);
> g_assert(name != NULL);
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c
> index fd5f709..c5df290 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> @@ -432,7 +432,7 @@ GVirConfigDomain *gvir_domain_get_config(GVirDomain *dom,
> return NULL;
> }
>
> - GVirConfigDomain *conf = gvir_config_domain_new_from_xml(xml);
> + GVirConfigDomain *conf = gvir_config_domain_new_from_xml(xml, err);
> g_free(xml);
> if ((err != NULL) && (*err != NULL))
> return NULL;
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list