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@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@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=5b... 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.
GVirConfigDomainOsSmBiosMode
VIR_ENUM_IMPL(virDomainSmbiosMode, VIR_DOMAIN_SMBIOS_LAST, "none", "emulate", "host", "sysinfo")
and GVirConfigDomainOsBootDevice.
VIR_ENUM_IMPL(virDomainBoot, VIR_DOMAIN_BOOT_LAST, "fd", "cdrom", "hd", "network")
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=e2c41e486018ee74f6a75c1f717... makes the enum case more compelling. Christophe