[libvirt] [glib] Don't overwrite filesystem type when writing format
When setting filesystem driver format first and type, only the type remained and vice-versa. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 8 ++++++-- tests/test-gconfig.c | 2 +- tests/xml/gconfig-domain-device-filesys.xml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 9b73af5..4e33d5f 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -125,7 +125,9 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys GVirConfigObject *node; g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); - node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!node) + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT) gvir_config_object_set_attribute_with_type( @@ -143,7 +145,9 @@ void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys *files GVirConfigObject *node; g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); - node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!node) + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); gvir_config_object_set_attribute_with_type( diff --git a/tests/test-gconfig.c b/tests/test-gconfig.c index bd2daa6..0eec53e 100644 --- a/tests/test-gconfig.c +++ b/tests/test-gconfig.c @@ -368,7 +368,7 @@ static void test_domain_device_filesystem(void) fs = gvir_config_domain_filesys_new(); gvir_config_domain_filesys_set_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_FILE); gvir_config_domain_filesys_set_access_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_ACCESS_MAPPED); - gvir_config_domain_filesys_set_driver_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT); + gvir_config_domain_filesys_set_driver_type(fs, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD); gvir_config_domain_filesys_set_driver_format(fs, GVIR_CONFIG_DOMAIN_DISK_FORMAT_QCOW2); gvir_config_domain_filesys_set_source(fs, "/path/to/source"); gvir_config_domain_filesys_set_target(fs, "/path/to/target1"); diff --git a/tests/xml/gconfig-domain-device-filesys.xml b/tests/xml/gconfig-domain-device-filesys.xml index 30152d2..a831c02 100644 --- a/tests/xml/gconfig-domain-device-filesys.xml +++ b/tests/xml/gconfig-domain-device-filesys.xml @@ -1,7 +1,7 @@ <domain> <devices> <filesystem type="file" accessmode="mapped"> - <driver format="qcow2"/> + <driver type="nbd" format="qcow2"/> <source file="/path/to/source"/> <target dir="/path/to/target1"/> <readonly/> -- 2.1.4
Hey, On Tue, Jul 07, 2015 at 04:26:02PM +0200, Cédric Bosdonnat wrote:
When setting filesystem driver format first and type, only the type remained and vice-versa. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 8 ++++++-- tests/test-gconfig.c | 2 +- tests/xml/gconfig-domain-device-filesys.xml | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 9b73af5..4e33d5f 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c @@ -125,7 +125,9 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys GVirConfigObject *node;
g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); - node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!node) + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver");
This is unintuitive, but after looking at the code and a quick test, I think _add_child() will create the node if it does not exist, and return the existing node if it already exists. So I think this could become: + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver");
g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT) gvir_config_object_set_attribute_with_type( @@ -143,7 +145,9 @@ void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys *files GVirConfigObject *node;
g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); - node = gvir_config_object_replace_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!node) + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver");
and same here. Christophe
On Tue, 2015-07-07 at 17:31 +0200, Christophe Fergeau wrote:
This is unintuitive, but after looking at the code and a quick test, I think _add_child() will create the node if it does not exist, and return the existing node if it already exists. So I think this could become:
+ node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver");
Waooh, indeed, not intuitive at all ;) I'll use that one then. -- Cedric
On Tue, Jul 07, 2015 at 06:24:38PM +0200, Cedric Bosdonnat wrote:
On Tue, 2015-07-07 at 17:31 +0200, Christophe Fergeau wrote:
This is unintuitive, but after looking at the code and a quick test, I think _add_child() will create the node if it does not exist, and return the existing node if it already exists. So I think this could become:
+ node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver");
Waooh, indeed, not intuitive at all ;) I'll use that one then.
We could try to change the name to something better, but I can't really think of a good name :( Christophe
participants (3)
-
Cedric Bosdonnat -
Christophe Fergeau -
Cédric Bosdonnat