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