Re: [PATCH v4 4/6] Add GVirConfigDomainHostdevPci

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx> 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

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]