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