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

[Christophe Fergeau]

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.

> 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=e2c41e486018ee74f6a75c1f717622
makes the enum case more compelling.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130502/19a45f22/attachment-0001.sig>