[libvirt] [PATCH v4 4/6] Add GVirConfigDomainHostdevPci

[Christophe Fergeau]

On Wed, Jul 06, 2016 at 10:11:18PM +0100, Zeeshan Ali (Khattak) wrote:
> On Thu, May 12, 2016 at 1:28 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > Looks I never answered this one.
> >
> > On Tue, Apr 26, 2016 at 05:04:30PM +0100, Zeeshan Ali (Khattak) wrote:
> >> >> +const gchar *gvir_config_domain_hostdev_pci_get_rom_file(GVirConfigDomainHostdevPci *hostdev)
> >> >> +{
> >> >> +    return gvir_config_object_get_attribute(GVIR_CONFIG_OBJECT(hostdev), "rom", "file");
> >> >
> >> >> +}
> >> >> +
> >> >> +gboolean gvir_config_domain_hostdev_pci_get_rom_bar(GVirConfigDomainHostdevPci *hostdev)
> >> >> +{
> >> >> +    return gvir_config_object_get_attribute_boolean(GVIR_CONFIG_OBJECT(hostdev),
> >> >> +                                                    "rom", "bar", FALSE);
> >> >
> >> > I'd prefer to handle on/off parsing here rather than moving it to
> >> > get_attribute_boolean().
> >>
> >> Why? Quick look through libvirt XML docs, shows that on/off is used in
> >> other places too.
> >
> > libvirt treats "on"/"off" and "yes"/"no" as different types,
> > virTristateSwitch and virTristateBool.
> 
> Giving them different names don't really make them different. on/off
> and yes/no, both have two states and are essentially booleans.

Yes, but treating them as boolean means it's complicated to do the right
thing when converting them to strings as you cannot guess whether to use
"on"/"off" or "yes"/"no"

> > This patch would treat the 2 as "boolean", and only in the parsing case
> > as it obviously cannot guess what is the right behaviour when converting
> > to string.
> > So I'd rather we don't start to treat both as booleans.
> 
> So you're suggesting that I inline the implementation and not make this generic?

Either that, or introduce an enum type for "on"/"off" and use that ?

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