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