[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [libvirt-glib 15/23] Only do XML parsing when creating config objects



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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]