[libvirt] [libvirt-glib 2/3] gconfig: Add GVirConfigDomainSnapshotDisk getters/setters

[Daniel P. Berrange]

On Thu, May 02, 2013 at 11:17:49PM +0200, Christophe Fergeau wrote:
> On Thu, May 02, 2013 at 11:40:56PM +0300, Zeeshan Ali (Khattak) wrote:
> > On Thu, May 2, 2013 at 6:36 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > > On Thu, May 02, 2013 at 06:22:14PM +0300, Zeeshan Ali (Khattak) wrote:
> > >> On Wed, May 1, 2013 at 10:59 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > >> > +
> > >> > +
> > >> > +const char *gvir_config_domain_snapshot_disk_get_driver_type(GVirConfigDomainSnapshotDisk *disk)
> > >>
> > >> Shouldn't 'driver_type' be an enum?
> > >
> > > libvirt is storing it internally as a string, see
> > > http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/conf/snapshot_conf.c;h=5b54a28a596aef0bb883dfe12ca5c2f300a48999;hb=HEAD#l130
> > > libvirt-gconfig generally follows what libvirt does when it comes to
> > > choosing between strings and enums.
> > 
> > I don't think that is true cause I see examples of enum setter/getters
> > that translate to strings in libvirt xml. e.g GVirConfigDomainOsType,
> 
> This one is indeed inconsistent, maybe it was added before danpb gave this
> rule of thumb for enum VS string, maybe the inconsistency was not noticed
> at review time.

Yep, that one should have been an enum too.

> > If we go with string option, I suggest we provide #defines for known
> > values and document here that these can be used.
> 
> Sounds good, though this could be done in an additional patch as this would
> also be useful for disk devices.
> 
> However, after a bit more research,
> http://libvirt.org/git/?p=libvirt.git;a=commit;h=e2c41e486018ee74f6a75c1f717622
> makes the enum case more compelling.

Yep, if the RNG is declaring an enum of values, we should expose it as
an enum in the API imho.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|