On Thu, 2014-02-20 at 10:12 +0200, Michael S. Tsirkin wrote:
> On Wed, Feb 19, 2014 at 03:20:54PM -0500, Bandan Das wrote:
> > The following patch depends on the value of rom_bar to
> > determine rom blacklist behavior. Existing code shouldn't
> > be affected by changing the default value of rom_bar since
> > all relevant decisions only rely on whether rom_bar is zero
> > or non-zero.
> >
> > Signed-off-by: Bandan Das <b...@redhat.com>
> > ---
> > hw/pci/pci.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 4e0701d..12c3e27 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -53,7 +53,12 @@ static void pci_bus_finalize(Object *obj);
> > static Property pci_props[] = {
> > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
> > DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
> > - DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 1),
> > + /*
> > + * 0 = disable
> > + * 1 = user requested on, force loading even if rom blacklisted
> > + * 2 = enabled but disables loading of blacklisted roms (default)
> > + */
> > + DEFINE_PROP_UINT32("rombar", PCIDevice, rom_bar, 2),
>
> How do users figure out this interface?
> Read code?
> Could we add a bit property rombarforce=on/off instead?
> Seems better.
>
> Maybe we should teach bool type visitors
> about 0 and 1 being legal values
> (call out to int visitor, then check value 0 or 1),
> then rombar can be changed to bit property too.
>
> Also, this will need QMP support right?
> IIUC rombar is not exposed in QMP ATM.
rombarforce seems very redundant for a user interface; rombar=1 "expose
the ROM BAR of the device", rombarforce=1 "yes, really expose the ROM
BAR of the device". Even if force implies rombar, I don't think that's
very easy to code in libvirt. I think we really just want to detect
unspecified versus specified, which probably means setting the default
value to something the user can't, or at least wouldn't, specify.
Thanks,
Alex
>
> > DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
> > QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
> > DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
> > --
> > 1.8.3.1
