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

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:08 AM, Christophe Fergeau <cfergeau(a)redhat.com&gt; wrote: Looks good so ACK. -- Regards, Zeeshan Ali (Khattak) FSF member#5124