Hi,
There's a typo in the subject "su*p*port"
On Wed, Nov 30, 2011 at 04:52:03PM +0000, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Create a new GVirConfigDomainFilesys object for dealing with
> the <filesystem> element.
>
> * libvirt-gconfig-domain-filesys.c, libvirt-gconfig-domain-filesys.h,
> Makefile.am, libvirt-gconfig.sym, libvirt-gconfig.h: New
> GVirConfigDomainFilesys object
> * libvirt-gconfig-object.c, libvirt-gconfig-object-private.h: Add
> gvir_config_object_delete_node for removing an XML node
> ---
> libvirt-gconfig/Makefile.am | 2 +
> libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 181 ++++++++++++++++++++++
> libvirt-gconfig/libvirt-gconfig-domain-filesys.h | 98 ++++++++++++
> libvirt-gconfig/libvirt-gconfig-object-private.h | 2 +
> libvirt-gconfig/libvirt-gconfig-object.c | 25 +++
> libvirt-gconfig/libvirt-gconfig.h | 1 +
> libvirt-gconfig/libvirt-gconfig.sym | 13 ++
> 7 files changed, 322 insertions(+), 0 deletions(-)
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-filesys.c
> create mode 100644 libvirt-gconfig/libvirt-gconfig-domain-filesys.h
>
> diff --git a/libvirt-gconfig/Makefile.am b/libvirt-gconfig/Makefile.am
> index 1879186..2c8c2a5 100644
> --- a/libvirt-gconfig/Makefile.am
> +++ b/libvirt-gconfig/Makefile.am
> @@ -16,6 +16,7 @@ GCONFIG_HEADER_FILES = \
> libvirt-gconfig-domain-clock.h \
> libvirt-gconfig-domain-device.h \
> libvirt-gconfig-domain-disk.h \
> + libvirt-gconfig-domain-filesys.h \
> libvirt-gconfig-domain-graphics.h \
> libvirt-gconfig-domain-graphics-spice.h \
> libvirt-gconfig-domain-input.h \
> @@ -45,6 +46,7 @@ GCONFIG_SOURCE_FILES = \
> libvirt-gconfig-domain-clock.c \
> libvirt-gconfig-domain-device.c \
> libvirt-gconfig-domain-disk.c \
> + libvirt-gconfig-domain-filesys.c \
> libvirt-gconfig-domain-graphics.c \
> libvirt-gconfig-domain-graphics-spice.c \
> libvirt-gconfig-domain-input.c \
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
> new file mode 100644
> index 0000000..6aa7f02
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c
I'd go with fs rather than filesys, but I'm fine with filesys if you feel
it's a better name.
> @@ -0,0 +1,181 @@
> +/*
> + * libvirt-gobject-config-domain-filesys.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@xxxxxxxxx>
Ah, I must have written this during my nap after lunch :)
> + */
> +
> +#include <config.h>
> +
> +#include <string.h>
> +
> +#include <libxml/tree.h>
> +
> +#include "libvirt-gconfig/libvirt-gconfig.h"
> +#include "libvirt-gconfig/libvirt-gconfig-helpers-private.h"
> +#include "libvirt-gconfig/libvirt-gconfig-object-private.h"
> +
> +#define GVIR_CONFIG_DOMAIN_FILESYS_GET_PRIVATE(obj) \
> + (G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_TYPE_CONFIG_DOMAIN_FILESYS, GVirConfigDomainFilesysPrivate))
> +
> +struct _GVirConfigDomainFilesysPrivate
> +{
> + GVirConfigDomainFilesysType type;
> +};
> +
> +G_DEFINE_TYPE(GVirConfigDomainFilesys, gvir_config_domain_filesys, GVIR_TYPE_CONFIG_DOMAIN_DEVICE);
> +
> +
> +static void gvir_config_domain_filesys_class_init(GVirConfigDomainFilesysClass *klass)
> +{
> + g_type_class_add_private(klass, sizeof(GVirConfigDomainFilesysPrivate));
> +}
> +
> +
> +static void gvir_config_domain_filesys_init(GVirConfigDomainFilesys *filesys)
> +{
> + g_debug("Init GVirConfigDomainFilesys=%p", filesys);
> +
> + filesys->priv = GVIR_CONFIG_DOMAIN_FILESYS_GET_PRIVATE(filesys);
> +}
> +
> +
> +GVirConfigDomainFilesys *gvir_config_domain_filesys_new(void)
> +{
> + GVirConfigObject *object;
> +
> + object = gvir_config_object_new(GVIR_TYPE_CONFIG_DOMAIN_FILESYS,
> + "filesystem", NULL);
> + return GVIR_CONFIG_DOMAIN_FILESYS(object);
> +}
> +
> +GVirConfigDomainFilesys *gvir_config_domain_filesys_new_from_xml(const gchar *xml,
> + GError **error)
> +{
> + GVirConfigObject *object;
> +
> + object = gvir_config_object_new_from_xml(GVIR_TYPE_CONFIG_DOMAIN_FILESYS,
> + "filesystem", NULL, xml, error);
> + return GVIR_CONFIG_DOMAIN_FILESYS(object);
> +}
> +
> +void gvir_config_domain_filesys_set_type(GVirConfigDomainFilesys *filesys,
> + GVirConfigDomainFilesysType type)
> +{
> + xmlNodePtr node;
> + const char *type_str;
> +
> + g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_FILESYS(filesys));
> +
> + node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(filesys));
> + g_return_if_fail(node != NULL);
> + type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_FILESYS_TYPE,
> + type);
> + g_return_if_fail(type_str != NULL);
> + xmlNewProp(node, (xmlChar*)"type", (xmlChar*)type_str);
> + filesys->priv->type = type;
gvir_config_object_set_attribute_with_type should work here
> +}
> +
> +void gvir_config_domain_filesys_set_access_type(GVirConfigDomainFilesys *filesys,
> + GVirConfigDomainFilesysAccessType type)
> +{
> + xmlNodePtr node;
> + const char *type_str;
> +
> + g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_FILESYS(filesys));
> +
> + node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(filesys));
> + g_return_if_fail(node != NULL);
> + type_str = gvir_config_genum_get_nick(GVIR_TYPE_CONFIG_DOMAIN_FILESYS_ACCESS_TYPE,
> + type);
> + g_return_if_fail(type_str != NULL);
> + xmlNewProp(node, (xmlChar*)"accesmode", (xmlChar*)type_str);
and here too
> +}
> +
> +void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys,
> + GVirConfigDomainFilesysDriverType type)
> +{
> + GVirConfigObject *node;
> +
> + g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_FILESYS(filesys));
> + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver");
> + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node));
> + if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT)
> + gvir_config_object_set_attribute_with_type(
> + node, "type",
> + GVIR_TYPE_CONFIG_DOMAIN_FILESYS_DRIVER_TYPE,
> + type, NULL);
> + /* else XXX delete existing attribute ? */
> + g_object_unref(G_OBJECT(node));
> +}
> +
> +void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys,
> + const char *source)
> +{
> + GVirConfigObject *source_node;
> + const char *attribute_name;
> +
> + g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_FILESYS(filesys));
> + source_node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(filesys),
> + "source");
> + g_return_if_fail(source_node != NULL);
> +
> + switch (filesys->priv->type) {
> + case GVIR_CONFIG_DOMAIN_FILESYS_MOUNT:
> + attribute_name = "dir";
> + break;
> + case GVIR_CONFIG_DOMAIN_FILESYS_FILE:
> + attribute_name = "file";
> + break;
> + case GVIR_CONFIG_DOMAIN_FILESYS_BLOCK:
> + attribute_name = "dev";
> + break;
> + case GVIR_CONFIG_DOMAIN_FILESYS_TEMPLATE:
> + attribute_name = "name";
> + break;
> + default:
> + g_return_if_reached();
> + }
> +
> + gvir_config_object_set_attribute(source_node,
> + attribute_name, source,
> + NULL);
> + g_object_unref(G_OBJECT(source_node));
> +}
> +
> +
> +void gvir_config_domain_filesys_set_target(GVirConfigDomainFilesys *filesys,
> + const char *path)
> +{
> + GVirConfigObject *node;
> +
> + g_return_if_fail(GVIR_IS_CONFIG_DOMAIN_FILESYS(filesys));
> + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "target");
> + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(node));
> + gvir_config_object_set_attribute(node, "dir", path, NULL);
> + g_object_unref(G_OBJECT(node));
> +}
> +
> +void gvir_config_domain_filesys_set_readonly(GVirConfigDomainFilesys *filesys,
> + gboolean readonly)
> +{
> + if (readonly)
> + gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(filesys), "readonly");
You need to unref _replace_child return value
> + else
> + gvir_config_object_delete_child(GVIR_CONFIG_OBJECT(filesys), "readonly");
> +}
> diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
> new file mode 100644
> index 0000000..bfb98df
> --- /dev/null
> +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h
> @@ -0,0 +1,98 @@
> +/*
> + * libvirt-gobject-domain-filesys.c: libvirt gobject integration
Nit, filesys.h
> + *
> + * 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@xxxxxxxxx>
I'm pleading non-guilty once again :)
> + */
> +
> +#if !defined(__LIBVIRT_GCONFIG_H__) && !defined(LIBVIRT_GCONFIG_BUILD)
> +#error "Only <libvirt-gconfig/libvirt-gconfig.h> can be included directly."
> +#endif
> +
> +#ifndef __LIBVIRT_GCONFIG_DOMAIN_FILESYS_H__
> +#define __LIBVIRT_GCONFIG_DOMAIN_FILESYS_H__
> +
> +G_BEGIN_DECLS
> +
> +#define GVIR_TYPE_CONFIG_DOMAIN_FILESYS (gvir_config_domain_filesys_get_type ())
> +#define GVIR_CONFIG_DOMAIN_FILESYS(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GVIR_TYPE_CONFIG_DOMAIN_FILESYS, GVirConfigDomainFilesys))
> +#define GVIR_CONFIG_DOMAIN_FILESYS_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), GVIR_TYPE_CONFIG_DOMAIN_FILESYS, GVirConfigDomainFilesysClass))
> +#define GVIR_IS_CONFIG_DOMAIN_FILESYS(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), GVIR_TYPE_CONFIG_DOMAIN_FILESYS))
> +#define GVIR_IS_CONFIG_DOMAIN_FILESYS_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), GVIR_TYPE_CONFIG_DOMAIN_FILESYS))
> +#define GVIR_CONFIG_DOMAIN_FILESYS_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), GVIR_TYPE_CONFIG_DOMAIN_FILESYS, GVirConfigDomainFilesysClass))
> +
> +typedef struct _GVirConfigDomainFilesys GVirConfigDomainFilesys;
> +typedef struct _GVirConfigDomainFilesysPrivate GVirConfigDomainFilesysPrivate;
> +typedef struct _GVirConfigDomainFilesysClass GVirConfigDomainFilesysClass;
> +
> +struct _GVirConfigDomainFilesys
> +{
> + GVirConfigDomainDevice parent;
> +
> + GVirConfigDomainFilesysPrivate *priv;
> +
> + /* Do not add fields to this struct */
> +};
> +
> +struct _GVirConfigDomainFilesysClass
> +{
> + GVirConfigDomainDeviceClass parent_class;
> +
> + gpointer padding[20];
> +};
> +
> +typedef enum {
> + GVIR_CONFIG_DOMAIN_FILESYS_MOUNT,
> + GVIR_CONFIG_DOMAIN_FILESYS_BLOCK,
> + GVIR_CONFIG_DOMAIN_FILESYS_FILE,
> + GVIR_CONFIG_DOMAIN_FILESYS_TEMPLATE,
> +} GVirConfigDomainFilesysType;
> +
> +typedef enum {
> + GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_PASSTHROUGH,
> + GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_MAPPED,
> + GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_SQUASHED,
> +} GVirConfigDomainFilesysAccessType;
> +
> +typedef enum {
> + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT,
> + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH,
> + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE,
> +} GVirConfigDomainFilesysDriverType;
> +
> +GType gvir_config_domain_filesys_get_type(void);
> +
> +GVirConfigDomainFilesys *gvir_config_domain_filesys_new(void);
> +GVirConfigDomainFilesys *gvir_config_domain_filesys_new_from_xml(const gchar *xml,
> + GError **error);
> +
> +void gvir_config_domain_filesys_set_type(GVirConfigDomainFilesys *filesys,
> + GVirConfigDomainFilesysType type);
> +void gvir_config_domain_filesys_set_access_type(GVirConfigDomainFilesys *filesys,
> + GVirConfigDomainFilesysAccessType type);
> +void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys,
> + GVirConfigDomainFilesysDriverType type);
> +void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys,
> + const char *source);
> +void gvir_config_domain_filesys_set_target(GVirConfigDomainFilesys *filesys,
> + const char *target);
> +void gvir_config_domain_filesys_set_readonly(GVirConfigDomainFilesys *filesys,
> + gboolean readonly);
> +G_END_DECLS
> +
> +#endif /* __LIBVIRT_GCONFIG_DOMAIN_FILESYS_H__ */
> diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h
> index f196be6..c564ac5 100644
> --- a/libvirt-gconfig/libvirt-gconfig-object-private.h
> +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h
> @@ -39,6 +39,8 @@ GVirConfigObject *gvir_config_object_add_child(GVirConfigObject *object,
> const char *child_name);
> GVirConfigObject *gvir_config_object_replace_child(GVirConfigObject *object,
> const char *child_name);
> +void gvir_config_object_delete_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 902e014..7fb1d4f 100644
> --- a/libvirt-gconfig/libvirt-gconfig-object.c
> +++ b/libvirt-gconfig/libvirt-gconfig-object.c
> @@ -380,6 +380,31 @@ gvir_config_object_replace_child(GVirConfigObject *object,
> NULL));
> }
>
> +
> +G_GNUC_INTERNAL void
> +gvir_config_object_delete_child(GVirConfigObject *object,
> + const char *child_name)
> +{
> + xmlNodePtr parent_node;
> + xmlNodePtr old_node;
> +
> + g_return_if_fail(GVIR_IS_CONFIG_OBJECT(object));
> + g_return_if_fail(child_name != NULL);
> +
> + parent_node = gvir_config_object_get_xml_node(GVIR_CONFIG_OBJECT(object));
> + g_return_if_fail (parent_node != NULL);
This is nitpicking but there's a space before (
> +
> + if (!(old_node = gvir_config_xml_get_element(parent_node, child_name, NULL)))
> + return;
> +
> + /* FIXME: should we make sure there are no multiple occurrences
> + * of this node?
> + */
> + xmlUnlinkNode(old_node);
> + xmlFreeNode(old_node);
> +}
I think we will get memory corruption if this API is combined with
_attach_child:
device_node = gvir_config_object_new("device");
fs_node = gvir_config_object_new("fs");
gvir_config_object_attach(device_node, fs_node);
gvir_config_object_delete_child(device_node, "fs");
g_object_unref(G_OBJECT(fs_node));
The xmlNodePtr held by fs_node will be freed twice, once by _delete_child
and when _finalize runs after the call to g_object_unref
Having each GVirConfigObject keep a list of its GVirConfigObject children
would make it possible to handle this case I think.
I'm fine with getting this function in even if there are known issues with
it.
> +
> +
> G_GNUC_INTERNAL void
> gvir_config_object_set_node_content(GVirConfigObject *object,
> const char *node_name,
> diff --git a/libvirt-gconfig/libvirt-gconfig.h b/libvirt-gconfig/libvirt-gconfig.h
> index ad9f59a..ed7623c 100644
> --- a/libvirt-gconfig/libvirt-gconfig.h
> +++ b/libvirt-gconfig/libvirt-gconfig.h
> @@ -33,6 +33,7 @@
> #include <libvirt-gconfig/libvirt-gconfig-domain-clock.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-device.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-disk.h>
> +#include <libvirt-gconfig/libvirt-gconfig-domain-filesys.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-graphics-spice.h>
> #include <libvirt-gconfig/libvirt-gconfig-domain-input.h>
> diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym
> index 9da3933..3ad7b1f 100644
> --- a/libvirt-gconfig/libvirt-gconfig.sym
> +++ b/libvirt-gconfig/libvirt-gconfig.sym
> @@ -47,6 +47,19 @@ LIBVIRT_GCONFIG_0.0.1 {
> gvir_config_domain_disk_set_target_dev;
> gvir_config_domain_disk_set_type;
>
> + gvir_config_domain_filesys_get_type;
> + gvir_config_domain_filesys_type_get_type;
> + gvir_config_domain_filesys_access_type_get_type;
> + gvir_config_domain_filesys_driver_type_get_type;
> + gvir_config_domain_filesys_new;
> + gvir_config_domain_filesys_new_from_xml;
> + gvir_config_domain_filesys_set_access_type;
> + gvir_config_domain_filesys_set_driver_type;
> + gvir_config_domain_filesys_set_readonly;
> + gvir_config_domain_filesys_set_source;
> + gvir_config_domain_filesys_set_target;
> + gvir_config_domain_filesys_set_type;
> +
> gvir_config_domain_graphics_get_type;
>
> gvir_config_domain_graphics_spice_get_type;
> --
> 1.7.6.4
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgpQBStnzfsxx.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list