[libvirt] [glib PATCH] domain config: add API to set the filesystem image format

Christophe Fergeau cfergeau at redhat.com
Mon Jun 15 15:47:02 UTC 2015


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150615/bb3f0c64/attachment-0001.sig>


More information about the libvir-list mailing list