Re: [PATCHv2 10/11] Introduce QEMU_CAPS_DEVICE_VIRTIO_REVISION

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 09, 2016 at 11:08:10PM -0400, Laine Stump wrote:
On 08/08/2016 12:35 PM, Ján Tomko wrote:
Check whether the disable-legacy property is present on the following
devices:
   virtio-balloon-pci
   virtio-blk-pci
   virtio-scsi-pci
   virtio-net-pci
   virtio-gpu-pci

Assuming that if QEMU knows other virtio devices where this property
is applicable, it will have at least one of these devices.

Pretty cool idea to look for the presence of disable-legacy in several
virtio devices in case the binary has  been built without one of them.
When I was doing a similar check, I semi-randomly picked
"disable-modern", but only checked it on virtio-net. Although in
practice that will probably work just as well, I like your idea better.


I only added it to those devices where we look for other properties.

I'm curious though - did you have a reason for checking for
disable-legacy rather than disable-modern? Or was your choice also
semi-random?


I don't remember thinking about it, I just picked the first one.
But chronologically, virtio 0.9 was introduced first.


Added in QEMU by:
commit e266d421490e0ae83044bbebb209b2d3650c0ba6
     virtio-pci: add flags to enable/disable legacy/modern
---
  src/qemu/qemu_capabilities.c                            | 6 ++++++
  src/qemu/qemu_capabilities.h                            | 1 +
  tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml        | 1 +
  tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml        | 1 +
  tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 +
  tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 +
  tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml       | 1 +
  tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml        | 1 +
  tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml        | 1 +
  9 files changed, 14 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 43e3ea7..340691a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -340,6 +340,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
                "display", /* 230 */
                "intel-iommu",
                "smm",
+              "virtio-revision",

I had thought about naming my capability flag based on what I used it
for as well, but decided it would be better to make the name of the
capability as close to the actual option name as possible, to avoid
confusion and allow re-use.

Okay, this string needs to be stable across libvirt releases, so
matching it as close as possible makes sense,


Maybe an even better idea would be to make capabilities named
"virtio-disable-modern" and "virtio-disable-legacy" set directly based
on the presence of the options, then create a synthetic capability named
virtio-revision that would be set if both of those others were true (you
might find that you need to add more checks in the future, e.g. to
disable it for certain interim versions of qemu that had bugs in virtio-1.0)

but I don't see a point of adding two capabilites for it if we are only
using one. The second one can just be added later if needed.

Jan

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]