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

[Christophe Fergeau]

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

> 
> > I don't know whether we should return TRUE or
> > FALSE as the default value as this depends on the QEMU version :(
> > "If no rom bar is specified, the qemu default will be used (older
> > versions of qemu used a default of "off", while newer qemus have a
> > default of "on")"
> 
> Maybe we can go with newer behaviour depending on whether or not rom
> file is set and leave a comment here about this?

I'm not sure these 2 attributes are related this way. We probably can
return the default used for new QEMU versions, and tweak it when needed.

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/20160512/e68134a7/attachment-0001.sig>