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
pgpOcNzcq4UGD.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
