[libvirt] [glib PATCH] domain config: add API to set the filesystem image format
Add the gvir_config_domain_filesys_set_driver_format function to allow setting nbd driver type + image format for containers filesystems. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 30 +++++++++++++++++++++++- libvirt-gconfig/libvirt-gconfig-domain-filesys.h | 4 ++++ libvirt-gconfig/libvirt-gconfig.sym | 5 ++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 006a407..fffbe88 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_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"))) { + 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( @@ -137,6 +139,32 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys g_object_unref(G_OBJECT(node)); } +void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys *filesys, + GVirConfigDomainDiskFormat format) +{ + GVirConfigObject *node; + GVirConfigDomainFilesysDriverType type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); + if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"))) { + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + } + g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); + if (format != GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW) + type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD; + + gvir_config_object_set_attribute_with_type( + node, "type", + GVIR_CONFIG_TYPE_DOMAIN_FILESYS_DRIVER_TYPE, + type, NULL); + + gvir_config_object_set_attribute_with_type( + node, "format", + GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT, + format, NULL); + g_object_unref(G_OBJECT(node)); +} + void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys, const char *source) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h index 4f3973e..18c4069 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h @@ -75,6 +75,8 @@ typedef enum { GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD, } GVirConfigDomainFilesysDriverType; GType gvir_config_domain_filesys_get_type(void); @@ -89,6 +91,8 @@ 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_driver_format(GVirConfigDomainFilesys *filesys, + GVirConfigDomainDiskFormat format); void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys, const char *source); void gvir_config_domain_filesys_set_ram_usage(GVirConfigDomainFilesys *filesys, diff --git a/libvirt-gconfig/libvirt-gconfig.sym b/libvirt-gconfig/libvirt-gconfig.sym index 407a52f..6ce1511 100644 --- a/libvirt-gconfig/libvirt-gconfig.sym +++ b/libvirt-gconfig/libvirt-gconfig.sym @@ -719,4 +719,9 @@ global: gvir_config_storage_vol_target_set_compat; } LIBVIRT_GCONFIG_0.1.9; +LIBVIRT_GCONFIG_0.2.1 { +global: + gvir_config_domain_filesys_set_driver_format; +} LIBVIRT_GCONFIG_0.2.0; + # .... define new API here using predicted next version number .... -- 2.1.4
Hey, On Mon, Jun 15, 2015 at 03:37:12PM +0200, Cédric Bosdonnat wrote:
Add the gvir_config_domain_filesys_set_driver_format function to allow setting nbd driver type + image format for containers filesystems. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 30 +++++++++++++++++++++++- libvirt-gconfig/libvirt-gconfig-domain-filesys.h | 4 ++++ libvirt-gconfig/libvirt-gconfig.sym | 5 ++++ 3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 006a407..fffbe88 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_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"))) { + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + }
I believe you could use gvir_config_object_replace_child() here? This should be split in a different commit.
g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT) gvir_config_object_set_attribute_with_type( @@ -137,6 +139,32 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys g_object_unref(G_OBJECT(node)); }
+void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys *filesys, + GVirConfigDomainDiskFormat format) +{ + GVirConfigObject *node; + GVirConfigDomainFilesysDriverType type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); + if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"))) { + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + }
_replace_child() ?
+ g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); + if (format != GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW) + type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD; + + gvir_config_object_set_attribute_with_type( + node, "type", + GVIR_CONFIG_TYPE_DOMAIN_FILESYS_DRIVER_TYPE, + type, NULL); + + gvir_config_object_set_attribute_with_type( + node, "format", + GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT, + format, NULL);
These 2 calls can probably be grouped in a single one? I haven't looked if there are other similar situations in libvirt-gconfig, but silently overwriting a preexisting "type" attribute with something different when setting the format does not seem very nice to the library user.
+ g_object_unref(G_OBJECT(node)); +} + void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys, const char *source) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h index 4f3973e..18c4069 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h @@ -75,6 +75,8 @@ typedef enum { GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD, } GVirConfigDomainFilesysDriverType;
Different commit? Could you add some small test case for the filesys node to tests/test/gconfig.c while you are at it? Christophe
On Mon, 2015-06-15 at 17:12 +0200, Christophe Fergeau wrote:
Hey,
On Mon, Jun 15, 2015 at 03:37:12PM +0200, Cédric Bosdonnat wrote:
Add the gvir_config_domain_filesys_set_driver_format function to allow setting nbd driver type + image format for containers filesystems. --- libvirt-gconfig/libvirt-gconfig-domain-filesys.c | 30 +++++++++++++++++++++++- libvirt-gconfig/libvirt-gconfig-domain-filesys.h | 4 ++++ libvirt-gconfig/libvirt-gconfig.sym | 5 ++++ 3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.c b/libvirt-gconfig/libvirt-gconfig-domain-filesys.c index 006a407..fffbe88 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_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"))) { + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + }
I believe you could use gvir_config_object_replace_child() here? This should be split in a different commit.
g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); if (type != GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT) gvir_config_object_set_attribute_with_type( @@ -137,6 +139,32 @@ void gvir_config_domain_filesys_set_driver_type(GVirConfigDomainFilesys *filesys g_object_unref(G_OBJECT(node)); }
+void gvir_config_domain_filesys_set_driver_format(GVirConfigDomainFilesys *filesys, + GVirConfigDomainDiskFormat format) +{ + GVirConfigObject *node; + GVirConfigDomainFilesysDriverType type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP; + + g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_FILESYS(filesys)); + if (!(node = gvir_config_object_get_child(GVIR_CONFIG_OBJECT(filesys), "driver"))) { + node = gvir_config_object_add_child(GVIR_CONFIG_OBJECT(filesys), "driver"); + }
_replace_child() ?
+ g_return_if_fail(GVIR_CONFIG_IS_OBJECT(node)); + if (format != GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW) + type = GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD; + + gvir_config_object_set_attribute_with_type( + node, "type", + GVIR_CONFIG_TYPE_DOMAIN_FILESYS_DRIVER_TYPE, + type, NULL); + + gvir_config_object_set_attribute_with_type( + node, "format", + GVIR_CONFIG_TYPE_DOMAIN_DISK_FORMAT, + format, NULL);
These 2 calls can probably be grouped in a single one?
I haven't looked if there are other similar situations in libvirt-gconfig, but silently overwriting a preexisting "type" attribute with something different when setting the format does not seem very nice to the library user.
What should we do in that case? Plainly return an error or just a warning?
+ g_object_unref(G_OBJECT(node)); +} + void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys, const char *source) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h index 4f3973e..18c4069 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h @@ -75,6 +75,8 @@ typedef enum { GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD, } GVirConfigDomainFilesysDriverType;
Different commit?
Could you add some small test case for the filesys node to tests/test/gconfig.c while you are at it?
I'll do it... but it really seems that there are a lot of untested areas there ;) -- Cedric
On Mon, Jun 15, 2015 at 05:36:49PM +0200, Cedric Bosdonnat wrote:
On Mon, 2015-06-15 at 17:12 +0200, Christophe Fergeau wrote:
I haven't looked if there are other similar situations in libvirt-gconfig, but silently overwriting a preexisting "type" attribute with something different when setting the format does not seem very nice to the library user.
What should we do in that case? Plainly return an error or just a warning?
That's a good question :) What I'm wondering is if libvirt-glib should make these kind of checks itself, of if it should allow to build non-sensical XML in some cases, and let libvirt errors out when it's handed this XML. This is already the case here anyway, as even if we are building a QEMU VM, we can call that API. So maybe don't try to be smart in this new API entry point, and only set the "format" attribute when gvir_config_domain_filesys_set_driver_format is called ?
+ g_object_unref(G_OBJECT(node)); +} + void gvir_config_domain_filesys_set_source(GVirConfigDomainFilesys *filesys, const char *source) { diff --git a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h index 4f3973e..18c4069 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-filesys.h +++ b/libvirt-gconfig/libvirt-gconfig-domain-filesys.h @@ -75,6 +75,8 @@ typedef enum { GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_DEFAULT, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_PATH, GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_HANDLE, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_LOOP, + GVIR_CONFIG_DOMAIN_FILESYS_DRIVER_NBD, } GVirConfigDomainFilesysDriverType;
Different commit?
Could you add some small test case for the filesys node to tests/test/gconfig.c while you are at it?
I'll do it... but it really seems that there are a lot of untested areas there ;)
Fine with me if you don't want to go that extra mile ;) Christophe
participants (3)
-
Cedric Bosdonnat -
Christophe Fergeau -
Cédric Bosdonnat