Re: [PATCHv2 10/11] Introduce QEMU_CAPS_DEVICE_VIRTIO_REVISION

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

 



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'm curious though - did you have a reason for checking for disable-legacy rather than disable-modern? Or was your choice also semi-random?


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.

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)

      );
@@ -1565,6 +1566,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
      { "deflate-on-oom", QEMU_CAPS_VIRTIO_BALLOON_AUTODEFLATE },
+    { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION },
  };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
@@ -1574,15 +1576,18 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
      { "event_idx", QEMU_CAPS_VIRTIO_BLK_EVENT_IDX },
      { "scsi", QEMU_CAPS_VIRTIO_BLK_SCSI },
      { "logical_block_size", QEMU_CAPS_BLOCKIO },
+    { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION },
  };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioNet[] = {
      { "tx", QEMU_CAPS_VIRTIO_TX_ALG },
      { "event_idx", QEMU_CAPS_VIRTIO_NET_EVENT_IDX },
+    { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION },
  };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioSCSI[] = {
      { "iothread", QEMU_CAPS_VIRTIO_SCSI_IOTHREAD },
+    { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION },
  };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsPCIAssign[] = {
@@ -1659,6 +1664,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = {
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = {
      { "virgl", QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL },
+    { "disable-legacy", QEMU_CAPS_DEVICE_VIRTIO_REVISION },
  };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsICH9[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d249e2e..15cf19d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -373,6 +373,7 @@ typedef enum {
      QEMU_CAPS_DISPLAY, /* -display */
      QEMU_CAPS_DEVICE_INTEL_IOMMU, /* -device intel-iommu */
      QEMU_CAPS_MACHINE_SMM_OPT, /* -machine xxx,smm=on/off/auto */
+    QEMU_CAPS_DEVICE_VIRTIO_REVISION, /* virtio-*pci.disable-{legacy,modern} */
QEMU_CAPS_LAST /* this must always be the last item */
  } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index 339ee1f..5ec9320 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -184,6 +184,7 @@
    <flag name='display'/>
    <flag name='intel-iommu'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2004000</version>
    <kvmVersion>0</kvmVersion>
    <package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
index c1a68d0..33c89ae 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -189,6 +189,7 @@
    <flag name='display'/>
    <flag name='intel-iommu'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2005000</version>
    <kvmVersion>0</kvmVersion>
    <package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index 85d7d3f..2eefaa0 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -158,6 +158,7 @@
    <flag name='tls-creds-x509'/>
    <flag name='display'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2005094</version>
    <kvmVersion>0</kvmVersion>
    <package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index deb1257..581ffaa 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -158,6 +158,7 @@
    <flag name='tls-creds-x509'/>
    <flag name='display'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2005094</version>
    <kvmVersion>0</kvmVersion>
    <package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index 2b7ea0e..8ed2c49 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
@@ -152,6 +152,7 @@
    <flag name='tls-creds-x509'/>
    <flag name='display'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2005094</version>
    <kvmVersion>0</kvmVersion>
    <package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
index 495c114..bf35867 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -195,6 +195,7 @@
    <flag name='display'/>
    <flag name='intel-iommu'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2006000</version>
    <kvmVersion>0</kvmVersion>
    <package></package>
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
index fafffa6..de2f3ee 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
@@ -192,6 +192,7 @@
    <flag name='display'/>
    <flag name='intel-iommu'/>
    <flag name='smm'/>
+  <flag name='virtio-revision'/>
    <version>2006091</version>
    <kvmVersion>0</kvmVersion>
    <package> (v2.7.0-rc1-52-g42e0d60)</package>


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