On Tue, Jan 22, 2019 at 10:44:23AM -0200, Eduardo Habkost wrote:
> On Tue, Jan 22, 2019 at 12:45:59PM +0100, Andrea Bolognani wrote:
> > On Tue, 2019-01-22 at 09:31 +0000, Daniel P. Berrangé wrote:
> > > On Mon, Jan 21, 2019 at 05:41:47PM -0500, Cole Robinson wrote:
> > > > On 01/18/2019 07:33 AM, Andrea Bolognani wrote:
> > > > > On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
> > > > > > + if (has_tmodel) {
> > > > > > + if (virQEMUCapsGet(qemuCaps, tmodel_cap))
> > > > > > + virBufferAddLit(buf, "-transitional");
> > > > > > +
> > > > > > + /* No error for if -transitional is not supported: our address
> > > > > > + * allocation will force the device into plain PCI bus, which
> > > > > > + * is functionally identical to standard 'virtio-XXX' behavior
> > > > > > + */
> > > > > > + } else if (has_ntmodel) {
> > > > > > + if (virQEMUCapsGet(qemuCaps, ntmodel_cap)) {
> > > > > > + virBufferAddLit(buf, "-non-transitional");
> > > > > > + } else if (virQEMUCapsGet(qemuCaps,
> > > > > > + QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY)) {
> > > > > > + virBufferAddLit(buf, ",disable-legacy=on");
> > > > > > + } else {
> > > > > > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > > > > > + _("virtio non-transitional model not supported "
> > > > > > + "for this qemu"));
> > > > > > + return -1;
> > > > > > + }
> > > > > > + }
> > > > >
> > > > > Would it make sense to be more explicit here? Current versions of
> > > > > QEMU default to disable-modern=off,disable-legacy=off for virtio-pci
> > > > > devices plugged into conventional PCI slots, but unless I'm mistaken
> > > > > that was not always the case, so it would perhaps be preferrable to
> > > > > not rely on that behavior and always explicitly set both disable-*
> > > > > options when the new devices are not available; if the options
> > > > > themselves are not available, then we should error out.
> > > >
> > > > I don't know enough to say, CCing ehabkost and danpb for more eyes
> > >
> > > The QEMU code for the devices has
> > >
> > > - Original devs: disable-modern=off disable-legacy=auto
> > > - Transitional devs: disable-modern=off disable-legacy=off
> > > - Non-transitional devs: disable-modern=off disable-legacy=on
> > >
> > > IOW, in the case that -transitional is not available, we could
> > > set disable-legacy=off. Provided that we always place -transitional
> > > devices into PCI slots, never PCI-e slots, whether we set
> > > disable-legacy=off or not doesn't have any effect. None the less it
> > > would make sense to set it explicitly though as that would cause us
> > > to catch bugs if we mistakenly had a -transitional dev in a PCI-e
> > > slot.
> > >
> > > I don't see a need to set disable-modern at all, since it is
> > > defaulting to "off" in all cases.
> >
> > Wasn't there some old QEMU release where disable-modern didn't
> > default to off? I seem to remember that.
>
> Yes, disable-modern is "on" on 2.6 and older machine-types.
>
> >
> > More generally, I don't see a reason *not* to specify disable-modern
> > along with disable-legacy. Why be implicit when you can very easily
> > be explicit instead?
>
> Agreed.
Ok, I've no objection to an explicit disable-modern=off based on this
info.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
