Re: [libvirt] [PATCH v4 4/6] Add GVirConfigDomainHostdevPci
On Thu, May 12, 2016 at 1:28 PM, Christophe Fergeau <cfergeau(a)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.
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?
--
Regards,
Zeeshan Ali (Khattak)