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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: pgpxQcoKtlwgC.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]