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

On Thu, May 12, 2016 at 1:28 PM, Christophe Fergeau <cfergeau(a)redhat.com&gt; 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.

> 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?