[libvirt] [libvirt-glib 1/2] Add gvir_gconfig_object_remove_attribute helper

--- libvirt-gconfig/libvirt-gconfig-object-private.h | 2 ++ libvirt-gconfig/libvirt-gconfig-object.c | 11 +++++++++++ 2 files changed, 13 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index ea06a67..38c129f 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -64,6 +64,8 @@ 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; +void gvir_config_object_remove_attribute(GVirConfigObject *object, + const char *attr_name); G_END_DECLS diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index a75db6e..52a2a0d 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -680,3 +680,14 @@ gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) child->priv->doc = g_object_ref(G_OBJECT(parent->priv->doc)); } } + +G_GNUC_INTERNAL void +gvir_config_object_remove_attribute(GVirConfigObject *object, + const char *attr_name) +{ + int status; + + do { + status = xmlUnsetProp(object->priv->node, (xmlChar *)attr_name); + } while (status == 0); +} -- 1.7.7.4

Zeeshan reported an issue where calling gvir_config_domain_disk_set_target_dev several times would result in duplicated 'dev' attributes in the resulting XML instead of overwriting the existing 'dev' attribute. Since there are no cases interesting to libvirt-gconfig where having several attributes with the same name is useful, when calling gvir_config_object_set_attribute*, we can just remove attributes with the same names as the one we are setting. --- libvirt-gconfig/libvirt-gconfig-object.c | 2 ++ libvirt-gconfig/tests/test-domain-create.c | 2 ++ 2 files changed, 4 insertions(+), 0 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index 52a2a0d..7a6d062 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -573,6 +573,7 @@ gvir_config_object_set_attribute(GVirConfigObject *object, ...) if (name == NULL) { break; } + gvir_config_object_remove_attribute(object, name); value = va_arg(args, const char *); if (value == NULL) { g_warn_if_reached(); @@ -603,6 +604,7 @@ gvir_config_object_set_attribute_with_type(GVirConfigObject *object, ...) if (name == NULL) { break; } + gvir_config_object_remove_attribute(object, name); attr_type = va_arg(args, GType); if (G_TYPE_IS_ENUM(attr_type)) { diff --git a/libvirt-gconfig/tests/test-domain-create.c b/libvirt-gconfig/tests/test-domain-create.c index 21c7664..c42deaf 100644 --- a/libvirt-gconfig/tests/test-domain-create.c +++ b/libvirt-gconfig/tests/test-domain-create.c @@ -98,6 +98,8 @@ int main(void) gvir_config_domain_disk_set_type(disk, GVIR_CONFIG_DOMAIN_DISK_FILE); gvir_config_domain_disk_set_guest_device_type(disk, GVIR_CONFIG_DOMAIN_DISK_GUEST_DEVICE_DISK); gvir_config_domain_disk_set_source(disk, "/tmp/foo/bar"); + gvir_config_domain_disk_set_driver_name(disk, "foo"); + gvir_config_domain_disk_set_driver_type(disk, "bar"); gvir_config_domain_disk_set_driver_name(disk, "qemu"); gvir_config_domain_disk_set_driver_type(disk, "qcow2"); gvir_config_domain_disk_set_target_bus(disk, GVIR_CONFIG_DOMAIN_DISK_BUS_IDE); -- 1.7.7.4

On Tue, Jan 3, 2012 at 11:08 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
Zeeshan reported an issue where calling gvir_config_domain_disk_set_target_dev several times would result in duplicated 'dev' attributes in the resulting XML instead of overwriting the existing 'dev' attribute. Since there are no cases interesting to libvirt-gconfig where having several attributes with the same name is useful, when calling gvir_config_object_set_attribute*, we can just remove attributes with the same names as the one we are setting.
And Zeeshan tested this and ACKs. :) -- Regards, Zeeshan Ali (Khattak) FSF member#5124

All that was missing to fix it was gvir_config_object_remove_attribute which now exists. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 3665d22..ef942f8 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -106,7 +106,8 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys node, "type", GVIR_TYPE_CONFIG_DOMAIN_FILESYS_DRIVER_TYPE, type, NULL); - /* else XXX delete existing attribute ? */ + else + gvir_config_object_remove_attribute(node, "type"); g_object_unref(G_OBJECT(node)); } -- 1.7.7.4

On Tue, Jan 3, 2012 at 11:37 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
All that was missing to fix it was gvir_config_object_remove_attribute which now exists.
ACK -- Regards, Zeeshan Ali (Khattak) FSF member#5124

On Tue, Jan 3, 2012 at 11:08 AM, Christophe Fergeau <cfergeau@redhat.com> wrote:
--- libvirt-gconfig/libvirt-gconfig-object-private.h | 2 ++ libvirt-gconfig/libvirt-gconfig-object.c | 11 +++++++++++ 2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-object-private.h b/libvirt-gconfig/libvirt-gconfig-object-private.h index ea06a67..38c129f 100644 --- a/libvirt-gconfig/libvirt-gconfig-object-private.h +++ b/libvirt-gconfig/libvirt-gconfig-object-private.h @@ -64,6 +64,8 @@ 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; +void gvir_config_object_remove_attribute(GVirConfigObject *object, + const char *attr_name);
G_END_DECLS
diff --git a/libvirt-gconfig/libvirt-gconfig-object.c b/libvirt-gconfig/libvirt-gconfig-object.c index a75db6e..52a2a0d 100644 --- a/libvirt-gconfig/libvirt-gconfig-object.c +++ b/libvirt-gconfig/libvirt-gconfig-object.c @@ -680,3 +680,14 @@ gvir_config_object_attach(GVirConfigObject *parent, GVirConfigObject *child) child->priv->doc = g_object_ref(G_OBJECT(parent->priv->doc)); } } + +G_GNUC_INTERNAL void +gvir_config_object_remove_attribute(GVirConfigObject *object, + const char *attr_name) +{ + int status; + + do { + status = xmlUnsetProp(object->priv->node, (xmlChar *)attr_name); + } while (status == 0); +}
Looks good so ACK. -- Regards, Zeeshan Ali (Khattak) FSF member#5124
participants (2)
-
Christophe Fergeau
-
Zeeshan Ali (Khattak)