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