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@xxxxxxxxxx> 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@xxxxxxxxxx> 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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list