[libvirt] [libvirt-glib 1/4] Refactor gvir_config_object_delete_child

Christophe Fergeau

18 Jan 2012 18 Jan '12

3:50 p.m.

By changing gvir_config_xml_foreach_child to make it robust against current node deletion in the "foreach" callback, we can use gvir_config_object_foreach_child to implement gvir_config_object_delete_child --- libvirt-gconfig/libvirt-gconfig-helpers.c | 5 +++- libvirt-gconfig/libvirt-gconfig-object.c | 32 ++++++++++++++++++---------- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-helpers.c b/libvirt-gconfig/libvirt-gconfig-helpers.c index cc2e5cc..c406a49 100644 --- a/libvirt-gconfig/libvirt-gconfig-helpers.c +++ b/libvirt-gconfig/libvirt-gconfig-helpers.c @@ -169,14 +169,17 @@ void gvir_config_xml_foreach_child(xmlNodePtr node, g_return_if_fail(iter_func != NULL); - for (it = node->children; it != NULL; it = it->next) { + it = node->children; + while (it != NULL) { gboolean cont; + xmlNodePtr next = it->next; if (xmlIsBlankNode(it)) continue; cont = iter_func(it, opaque); if (!cont) break; + it = next; } } diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 5645490..7fb8b69 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -465,27 +465,35 @@ gvir_config_object_replace_child_with_attribute(GVirConfigObject *object, g_object_unref(G_OBJECT(child)); } +static gboolean maybe_unlink_node(xmlNodePtr node, const char *name) +{ + if (g_strcmp0((char *)node->name, name) == 0) { + xmlUnlinkNode(node); + xmlFreeNode(node); + + return TRUE; + } + + return FALSE; +} + +static gboolean remove_oneshot(xmlNodePtr node, gpointer opaque) +{ + return !maybe_unlink_node(node, opaque); +} + 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_CONFIG_IS_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); + gvir_config_object_foreach_child(object, NULL, remove_oneshot, + (gpointer)child_name); +} - 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); } -- 1.7.7.5

Show replies by date

Christophe Fergeau

18 Jan 18 Jan

3:50 p.m.

New subject: [libvirt] [libvirt-glib 2/4] Add gvir_config_object_delete_children

--- libvirt-gconfig/libvirt-gconfig-object-private.h | 1 + libvirt-gconfig/libvirt-gconfig-object.c | 13 +++++++++++++ 2 files changed, 14 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 6d01e26..922b0f3 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -72,6 +72,7 @@ void gvir_config_object_replace_child_with_attribute(GVirConfigObject *object, const char *attr_value); void gvir_config_object_delete_child(GVirConfigObject *object, const char *child_name); +void gvir_config_object_delete_children(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 7fb8b69..dbb63a5 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -493,9 +493,22 @@ gvir_config_object_delete_child(GVirConfigObject *object, (gpointer)child_name); } +static gboolean remove_always(xmlNodePtr node, gpointer opaque) +{ + maybe_unlink_node(node, opaque); + return TRUE; } +G_GNUC_INTERNAL void +gvir_config_object_delete_children(GVirConfigObject *object, const char *child_name) +{ + g_return_if_fail(GVIR_CONFIG_IS_OBJECT(object)); + g_return_if_fail(child_name != NULL); + + gvir_config_object_foreach_child(object, NULL, remove_always, + (gpointer)child_name); +} G_GNUC_INTERNAL void gvir_config_object_set_node_content(GVirConfigObject *object, -- 1.7.7.5

Daniel P. Berrange

19 Jan 19 Jan

5:14 p.m.

New subject: [libvirt] [libvirt-glib 2/4] Add gvir_config_object_delete_children

On Wed, Jan 18, 2012 at 04:50:42PM +0100, Christophe Fergeau wrote:

...

--- libvirt-gconfig/libvirt-gconfig-object-private.h | 1 + libvirt-gconfig/libvirt-gconfig-object.c | 13 +++++++++++++ 2 files changed, 14 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 :|

Christophe Fergeau

18 Jan 18 Jan

3:50 p.m.

New subject: [libvirt] [libvirt-glib 3/4] Split gvir_config_object_attach

Most of the time we want gvir_config_object_attach to replace existing nodes, but sometimes (for <devices> subnodes) we want it to append the new node and to keep the existing nodes with the same name. This commit solves this by adding 2 distinct helpers, _attach_add and _attach_replace. This should fix some unexpected behaviour of various _set_ functions which were appending new nodes instead of replacing the existing one. --- libvirt-gconfig/libvirt-gconfig-domain.c | 20 +++++++++++--------- libvirt-gconfig/libvirt-gconfig-object-private.h | 6 ++++-- libvirt-gconfig/libvirt-gconfig-object.c | 20 ++++++++++++++++++-- .../libvirt-gconfig-storage-pool-target.c | 4 ++-- libvirt-gconfig/libvirt-gconfig-storage-pool.c | 8 ++++---- .../libvirt-gconfig-storage-vol-target.c | 4 ++-- libvirt-gconfig/libvirt-gconfig-storage-vol.c | 8 ++++---- libvirt-gconfig/libvirt-gconfig.sym | 1 - 8 files changed, 45 insertions(+), 26 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain.c b/libvirt-gconfig/libvirt-gconfig-domain.c index fba1ee2..cf5aa17 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain.c +++ b/libvirt-gconfig/libvirt-gconfig-domain.c @@ -290,8 +290,8 @@ void gvir_config_domain_set_clock(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_CLOCK(klock)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(klock)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(klock)); } void gvir_config_domain_set_os(GVirConfigDomain *domain, @@ -300,8 +300,8 @@ void gvir_config_domain_set_os(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_OS(os)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(os)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(os)); } void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, @@ -310,8 +310,8 @@ void gvir_config_domain_set_seclabel(GVirConfigDomain *domain, g_return_if_fail(GVIR_CONFIG_IS_DOMAIN(domain)); g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_SECLABEL(seclabel)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), - GVIR_CONFIG_OBJECT(seclabel)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + GVIR_CONFIG_OBJECT(seclabel)); } void gvir_config_domain_set_lifecycle(GVirConfigDomain *domain, @@ -354,10 +354,12 @@ void gvir_config_domain_set_devices(GVirConfigDomain *domain, g_warn_if_reached(); continue; } - gvir_config_object_attach(devices_node, GVIR_CONFIG_OBJECT(it->data)); + gvir_config_object_attach_add(devices_node, + GVIR_CONFIG_OBJECT(it->data)); } - gvir_config_object_attach(GVIR_CONFIG_OBJECT(domain), devices_node); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(domain), + devices_node); g_object_unref(G_OBJECT(devices_node)); } @@ -372,7 +374,7 @@ void gvir_config_domain_add_device(GVirConfigDomain *domain, devices_node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(domain), "devices"); - gvir_config_object_attach(devices_node, GVIR_CONFIG_OBJECT(device)); + gvir_config_object_attach_add(devices_node, GVIR_CONFIG_OBJECT(device)); g_object_unref(G_OBJECT(devices_node)); } diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index 922b0f3..781e1a3 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -76,8 +76,10 @@ void gvir_config_object_delete_children(GVirConfigObject *object, const char *ch void gvir_config_object_set_child(GVirConfigObject *object, xmlNodePtr child); -void gvir_config_object_attach(GVirConfigObject *parent, - GVirConfigObject *child); +void gvir_config_object_attach_add(GVirConfigObject *parent, + GVirConfigObject *child); +void gvir_config_object_attach_replace(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, diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index dbb63a5..2e28208 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -803,12 +803,16 @@ gvir_config_object_set_attribute_with_type(GVirConfigObject *object, ...) va_end(args); } -G_GNUC_INTERNAL void -gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) +static void +gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child, gboolean replace) { g_return_if_fail(GVIR_CONFIG_IS_OBJECT(parent)); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(child)); + if (replace) { + gvir_config_object_delete_children(parent, + (char *)child->priv->node->name); + } xmlUnlinkNode(child->priv->node); xmlAddChild(parent->priv->node, child->priv->node); if (child->priv->doc != NULL) { @@ -821,6 +825,18 @@ gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) } G_GNUC_INTERNAL void +gvir_config_object_attach_replace(GVirConfigObject *parent, GVirConfigObject *child) +{ + gvir_config_object_attach(parent, child, TRUE); +} + +G_GNUC_INTERNAL void +gvir_config_object_attach_add(GVirConfigObject *parent, GVirConfigObject *child) +{ + gvir_config_object_attach(parent, child, FALSE); +} + +G_GNUC_INTERNAL void gvir_config_object_remove_attribute(GVirConfigObject *object, const char *attr_name) { diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c index 56a8c98..0d7f164 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool-target.c @@ -86,6 +86,6 @@ void gvir_config_storage_pool_target_set_permissions(GVirConfigStoragePoolTarget g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), + GVIR_CONFIG_OBJECT(perms)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-pool.c b/libvirt-gconfig/libvirt-gconfig-storage-pool.c index 3955251..c524c17 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-pool.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-pool.c @@ -137,8 +137,8 @@ void gvir_config_storage_pool_set_source(GVirConfigStoragePool *pool, g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_SOURCE(source)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(source)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), + GVIR_CONFIG_OBJECT(source)); } void gvir_config_storage_pool_set_target(GVirConfigStoragePool *pool, @@ -147,6 +147,6 @@ void gvir_config_storage_pool_set_target(GVirConfigStoragePool *pool, g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL(pool)); g_return_if_fail(GVIR_CONFIG_IS_STORAGE_POOL_TARGET(target)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(pool), - GVIR_CONFIG_OBJECT(target)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(pool), + GVIR_CONFIG_OBJECT(target)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c index d7dfca7..3786e2b 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol-target.c @@ -90,6 +90,6 @@ void gvir_config_storage_vol_target_set_permissions(GVirConfigStorageVolTarget * g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); g_return_if_fail(GVIR_CONFIG_IS_STORAGE_PERMISSIONS(perms)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(target), - GVIR_CONFIG_OBJECT(perms)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(target), + GVIR_CONFIG_OBJECT(perms)); } diff --git a/libvirt-gconfig/libvirt-gconfig-storage-vol.c b/libvirt-gconfig/libvirt-gconfig-storage-vol.c index 777efef..6281c02 100644 --- a/libvirt-gconfig/libvirt-gconfig-storage-vol.c +++ b/libvirt-gconfig/libvirt-gconfig-storage-vol.c @@ -107,8 +107,8 @@ void gvir_config_storage_vol_set_target(GVirConfigStorageVol *vol, g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_TARGET(target)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(target)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), + GVIR_CONFIG_OBJECT(target)); } void gvir_config_storage_vol_set_backing_store(GVirConfigStorageVol *vol, @@ -117,6 +117,6 @@ void gvir_config_storage_vol_set_backing_store(GVirConfigStorageVol *vol, g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL(vol)); g_return_if_fail(GVIR_CONFIG_IS_STORAGE_VOL_BACKING_STORE(backing_store)); - gvir_config_object_attach(GVIR_CONFIG_OBJECT(vol), - GVIR_CONFIG_OBJECT(backing_store)); + gvir_config_object_attach_replace(GVIR_CONFIG_OBJECT(vol), + GVIR_CONFIG_OBJECT(backing_store)); } diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 9bfe1d9..0fcbd13 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -239,7 +239,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_object_get_schema; 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; -- 1.7.7.5

Daniel P. Berrange

19 Jan 19 Jan

5:15 p.m.

New subject: [libvirt] [libvirt-glib 3/4] Split gvir_config_object_attach

On Wed, Jan 18, 2012 at 04:50:43PM +0100, Christophe Fergeau wrote:

...

Most of the time we want gvir_config_object_attach to replace existing nodes, but sometimes (for <devices> subnodes) we want it to append the new node and to keep the existing nodes with the same name. This commit solves this by adding 2 distinct helpers, _attach_add and _attach_replace. This should fix some unexpected behaviour of various _set_ functions which were appending new nodes instead of replacing the existing one. --- libvirt-gconfig/libvirt-gconfig-domain.c | 20 +++++++++++--------- libvirt-gconfig/libvirt-gconfig-object-private.h | 6 ++++-- libvirt-gconfig/libvirt-gconfig-object.c | 20 ++++++++++++++++++-- .../libvirt-gconfig-storage-pool-target.c | 4 ++-- libvirt-gconfig/libvirt-gconfig-storage-pool.c | 8 ++++---- .../libvirt-gconfig-storage-vol-target.c | 4 ++-- libvirt-gconfig/libvirt-gconfig-storage-vol.c | 8 ++++---- libvirt-gconfig/libvirt-gconfig.sym | 1 - 8 files changed, 45 insertions(+), 26 deletions(-)

Christophe Fergeau

18 Jan 18 Jan

3:50 p.m.

New subject: [libvirt] [libvirt-glib 4/4] Remove 2 unwanted exports

These 2 GVirObject helpers are declared in libvirt-gobject-private.h and so shouldn't be exported. --- libvirt-gconfig/libvirt-gconfig.sym | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 0fcbd13..efbfafb 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -239,8 +239,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_object_get_schema; gvir_config_object_to_xml; gvir_config_object_validate; - gvir_config_object_set_attribute; - gvir_config_object_set_attribute_with_type; gvir_config_secret_get_type; gvir_config_secret_new; -- 1.7.7.5

Daniel P. Berrange

19 Jan 19 Jan

5:15 p.m.

New subject: [libvirt] [libvirt-glib 4/4] Remove 2 unwanted exports

On Wed, Jan 18, 2012 at 04:50:44PM +0100, Christophe Fergeau wrote:

...

These 2 GVirObject helpers are declared in libvirt-gobject-private.h and so shouldn't be exported. --- libvirt-gconfig/libvirt-gconfig.sym | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 0fcbd13..efbfafb 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -239,8 +239,6 @@ LIBVIRT_GCONFIG_0.0.4 { gvir_config_object_get_schema; gvir_config_object_to_xml; gvir_config_object_validate; - gvir_config_object_set_attribute; - gvir_config_object_set_attribute_with_type;

gvir_config_secret_get_type; gvir_config_secret_new;

Daniel P. Berrange

5:14 p.m.

On Wed, Jan 18, 2012 at 04:50:41PM +0100, Christophe Fergeau wrote:

...

By changing gvir_config_xml_foreach_child to make it robust against current node deletion in the "foreach" callback, we can use gvir_config_object_foreach_child to implement gvir_config_object_delete_child --- libvirt-gconfig/libvirt-gconfig-helpers.c | 5 +++- libvirt-gconfig/libvirt-gconfig-object.c | 32 ++++++++++++++++++---------- 2 files changed, 24 insertions(+), 13 deletions(-)

4972

Age (days ago)

4973

Last active (days ago)

List overview

Download

7 comments

2 participants

participants (2)

Christophe Fergeau
Daniel P. Berrange

[libvirt] [libvirt-glib 1/4] Refactor gvir_config_object_delete_child

tags

participants (2)