On Fri, 2016-05-13 at 13:51 -0400, Cole Robinson wrote:
> >
> > +/* Capabilities that we assume are always enabled
> > + * for QEMU >= 0.12.0 */
> > +static void
> > +virQEMUCapsInitHelpBasic(virQEMUCapsPtr qemuCaps)
> > +{
> > +
> > + /* Although very new versions of qemu advertise the presence of
> > + * the rombar option in the output of "qemu -device pci-assign,?",
> > + * this advertisement was added to the code long after the option
> > + * itself. According to qemu developers, though, rombar is
> > + * available in all qemu binaries from release 0.12 onward.
> > + * Setting the capability this way makes it available in more
> > + * cases where it might be needed, and shouldn't cause any false
> > + * positives (in the case that it did, qemu would produce an error
> > + * log and refuse to start, so it would be immediately obvious).
> > + */
> > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_PCI_ROMBAR);
> > +
> > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VIRTIO_BLK_SG_IO);
> > + virQEMUCapsSet(qemuCaps, QEMU_CAPS_CPU_HOST);
> > +}
>
> As Pavel said, the approach we've taken for other flags is to rename them to
> X_$NAME, and remove all code usages. Moving these flags to their own function
> confuses that approach, so I suggest either give the X_$NAME treatment to each
> of those flags, or just using this series to disable PCI_ROMBAR and leave the
> other flags for a later series.
I hadn't noticed that pattern, and it makes much more sense
than what I'm doing here - setting a capability unconditionally
and never checking for its presence seemed kinda off, glad you
guys caught it and suggested a better solution before I had a
chance to push :)
--
Andrea Bolognani
Software Engineer - Virtualization Team
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list