[06/18] conf: Add <hostdev model='virtio-{non-}transitional'/>

Message ID 14f1f820a928c6d5e4cca54d573f8857df9956df.1547746867.git.crobinso@redhat.com
State Superseded
Headers show
Series
  • qemu: virtio-{non-}transitional support
Related show

Commit Message

Cole Robinson Jan. 17, 2019, 5:52 p.m.
qemu vhost-scsi devices map to XML roughly like:

    <hostdev mode='subsystem' type='scsi_host'>
      <source protocol='vhost' wwpn=X/>
    </hostdev>

To support vhost-scsi-pci-{non-}traditional in qemu, we
need to to extend the SCSI Host hostdev XML to handle
model= value. This matches the XML model= format used
for mediated devices. This is just the domain_conf bits
and some XML test cases.

Use of virtio-X naming here does not match the hostdev
protocol=vhost nor does it match the qemu vhost-X device
naming, however it's more consistent with all other
model= names in this area, and also matches the
inconsistency of <vsock> devices which use model=virtio
but map to vhost-vsock on the qemu commandline

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 docs/formatdomain.html.in                     |  5 ++-
 docs/schemas/domaincommon.rng                 |  9 +++++
 src/conf/domain_conf.c                        | 37 ++++++++++++++++---
 src/conf/domain_conf.h                        | 12 ++++++
 src/libvirt_private.syms                      |  2 +
 .../virtio-non-transitional.x86_64-3.1.0.args |  3 ++
 ...virtio-non-transitional.x86_64-latest.args |  3 ++
 .../virtio-non-transitional.xml               |  3 ++
 .../virtio-transitional.x86_64-3.1.0.args     |  3 ++
 .../virtio-transitional.x86_64-latest.args    |  3 ++
 .../qemuxml2argvdata/virtio-transitional.xml  |  3 ++
 .../virtio-non-transitional.xml               |  9 +++++
 .../virtio-transitional.xml                   |  9 +++++
 13 files changed, 95 insertions(+), 6 deletions(-)

-- 
2.20.1

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

Comments

Andrea Bolognani Jan. 18, 2019, 3:06 p.m. | #1
On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
[...]
>  typedef struct _virDomainHostdevSubsysSCSIVHost virDomainHostdevSubsysSCSIVHost;

>  typedef virDomainHostdevSubsysSCSIVHost *virDomainHostdevSubsysSCSIVHostPtr;

>  struct _virDomainHostdevSubsysSCSIVHost {

>      int protocol; /* enum virDomainHostdevSubsysSCSIHostProtocolType */

>      char *wwpn;

> +    int model; /* enum virDomainHostdevSubsysSCSIVHostModelType */

>  };


There is no check for the value of model in
virDomainHostdevDefCheckABIStability(). Interestingly, the same
is true for mdevs, so that looks like an oversight that can be
corrected there as well.

Everything else looks reasonable, including the decision to use
model=virtio-* instead of model=vhost-* considering that neither
approach would be completely satisfactory anyway, so

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>


-- 
Andrea Bolognani / Red Hat / Virtualization

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

Patch

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 29e4bb1bda..36dce863ac 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4680,7 +4680,10 @@ 
           <dd><span class="since">since 2.5.0</span>For SCSI devices, user
             is responsible to make sure the device is not used by host. This
             <code>type</code> passes all LUNs presented by a single HBA to
-            the guest.
+            the guest. <span class="since">Since 5.1.0,</span> the
+            <code>model</code> attribute can be specified further
+            with "virtio-transitional", "virtio-non-transitional", or
+            "virtio" which matches the old behavior.
           </dd>
           <dt><code>mdev</code></dt>
           <dd>For mediated devices (<span class="since">Since 3.2.0</span>)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 425d7f851a..532a78ce32 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4626,6 +4626,15 @@ 
     <attribute name="type">
       <value>scsi_host</value>
     </attribute>
+    <optional>
+      <attribute name="model">
+        <choice>
+          <value>virtio</value>
+          <value>virtio-transitional</value>
+          <value>virtio-non-transitional</value>
+        </choice>
+      </attribute>
+    </optional>
     <element name="source">
       <choice>
         <group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9c68e41765..3776079e1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -714,6 +714,13 @@  VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIHostProtocol,
               "none",
               "vhost")
 
+VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIVHostModel,
+              VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_LAST,
+              "default",
+              "virtio",
+              "virtio-transitional",
+              "virtio-non-transitional")
+
 VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
               "storage",
               "misc",
@@ -7597,6 +7604,7 @@  virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
     int ret = -1;
     virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci;
     virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
+    virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def->source.subsys.u.scsi_host;
     virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev;
 
     /* @managed can be read from the xml document - it is always an
@@ -7679,14 +7687,26 @@  virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
         }
     }
 
-    if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
+    if (def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV &&
+        def->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
         if (model) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("'model' attribute in <hostdev> is only supported "
-                             "when type='mdev'"));
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("'model' attribute in <hostdev> is not supported "
+                             "for type='%s'"),
+                           virDomainHostdevSubsysTypeToString(def->source.subsys.type));
             goto cleanup;
         }
-    } else {
+    }
+
+    if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
+        if (model &&
+            ((scsihostsrc->model = virDomainHostdevSubsysSCSIVHostModelTypeFromString(model)) < 0)) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("unknown hostdev model '%s'"),
+                           model);
+            goto cleanup;
+        }
+    } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
         if (!model) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
                            _("Missing 'model' attribute in mediated device's "
@@ -27093,6 +27113,7 @@  virDomainHostdevDefFormat(virBufferPtr buf,
     const char *mode = virDomainHostdevModeTypeToString(def->mode);
     virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
     virDomainHostdevSubsysMediatedDevPtr mdevsrc = &def->source.subsys.u.mdev;
+    virDomainHostdevSubsysSCSIVHostPtr scsihostsrc = &def->source.subsys.u.scsi_host;
     const char *type;
 
     if (!mode) {
@@ -27143,6 +27164,12 @@  virDomainHostdevDefFormat(virBufferPtr buf,
                               virTristateBoolTypeToString(scsisrc->rawio));
         }
 
+        if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST &&
+            scsihostsrc->model) {
+            virBufferAsprintf(buf, " model='%s'",
+                              virDomainHostdevSubsysSCSIVHostModelTypeToString(scsihostsrc->model));
+        }
+
         if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV) {
             virBufferAsprintf(buf, " model='%s'",
                               virMediatedDeviceModelTypeToString(mdevsrc->model));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index e3f4273b55..36ab544dd3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -394,11 +394,23 @@  typedef enum {
 
 VIR_ENUM_DECL(virDomainHostdevSubsysSCSIHostProtocol)
 
+typedef enum {
+    VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_DEFAULT,
+    VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_VIRTIO,
+    VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_VIRTIO_TRANSITIONAL,
+    VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_VIRTIO_NON_TRANSITIONAL,
+
+    VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_LAST,
+} virDomainHostdevSubsysSCSIVHostModelType;
+
+VIR_ENUM_DECL(virDomainHostdevSubsysSCSIVHostModel)
+
 typedef struct _virDomainHostdevSubsysSCSIVHost virDomainHostdevSubsysSCSIVHost;
 typedef virDomainHostdevSubsysSCSIVHost *virDomainHostdevSubsysSCSIVHostPtr;
 struct _virDomainHostdevSubsysSCSIVHost {
     int protocol; /* enum virDomainHostdevSubsysSCSIHostProtocolType */
     char *wwpn;
+    int model; /* enum virDomainHostdevSubsysSCSIVHostModelType */
 };
 
 typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bd7e896654..60a193450b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -407,6 +407,8 @@  virDomainHostdevInsert;
 virDomainHostdevModeTypeToString;
 virDomainHostdevRemove;
 virDomainHostdevSubsysPCIBackendTypeToString;
+virDomainHostdevSubsysSCSIVHostModelTypeFromString;
+virDomainHostdevSubsysSCSIVHostModelTypeToString;
 virDomainHostdevSubsysTypeToString;
 virDomainHPTResizingTypeToString;
 virDomainHubTypeFromString;
diff --git a/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-3.1.0.args b/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-3.1.0.args
index 6eaf3086ef..2eaa509c04 100644
--- a/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-3.1.0.args
+++ b/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-3.1.0.args
@@ -27,12 +27,15 @@  file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 addr=0x1 \
 -device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
 -device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
+-device pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
 -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \
 -device virtio-blk-pci,disable-legacy=on,scsi=off,bus=pci.2,addr=0x0,\
 drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \
 -netdev user,id=hostnet0 \
 -device virtio-net-pci,disable-legacy=on,netdev=hostnet0,id=net0,\
 mac=00:11:22:33:44:55,bus=pci.1,addr=0x0 \
+-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0,\
+bus=pci.3,addr=0x0 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-latest.args b/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-latest.args
index 9a0eb9a1f3..82255909c4 100644
--- a/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-latest.args
@@ -27,12 +27,15 @@  file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 addr=0x1 \
 -device pcie-root-port,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \
 -device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 \
+-device pcie-root-port,port=0xb,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x3 \
 -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \
 -device virtio-blk-pci-non-transitional,scsi=off,bus=pci.2,addr=0x0,\
 drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \
 -netdev user,id=hostnet0 \
 -device virtio-net-pci-non-transitional,netdev=hostnet0,id=net0,\
 mac=00:11:22:33:44:55,bus=pci.1,addr=0x0 \
+-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0,\
+bus=pci.3,addr=0x0 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2argvdata/virtio-non-transitional.xml b/tests/qemuxml2argvdata/virtio-non-transitional.xml
index a1b35d1c07..32d2bdc638 100644
--- a/tests/qemuxml2argvdata/virtio-non-transitional.xml
+++ b/tests/qemuxml2argvdata/virtio-non-transitional.xml
@@ -16,6 +16,9 @@ 
       <mac address='00:11:22:33:44:55'/>
       <model type='virtio-non-transitional'/>
     </interface>
+    <hostdev mode='subsystem' type='scsi_host' managed='no' model='virtio-non-transitional'>
+      <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
+    </hostdev>
     <controller type='usb' index='0' model='none'/>
     <memballoon model='none'/>
   </devices>
diff --git a/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args b/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args
index 9b80cd893a..4e991d6187 100644
--- a/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args
+++ b/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args
@@ -27,12 +27,15 @@  file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 addr=0x1 \
 -device pcie-pci-bridge,id=pci.2,bus=pci.1,addr=0x0 \
 -device pcie-root-port,port=0x9,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x1 \
+-device pcie-root-port,port=0xa,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x2 \
 -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \
 -device virtio-blk-pci,scsi=off,bus=pci.2,addr=0x2,drive=drive-virtio-disk0,\
 id=virtio-disk0,bootindex=1 \
 -netdev user,id=hostnet0 \
 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.2,\
 addr=0x1 \
+-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0,\
+bus=pci.3,addr=0x0 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2argvdata/virtio-transitional.x86_64-latest.args b/tests/qemuxml2argvdata/virtio-transitional.x86_64-latest.args
index 46e139d492..dab25ba2e8 100644
--- a/tests/qemuxml2argvdata/virtio-transitional.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/virtio-transitional.x86_64-latest.args
@@ -27,12 +27,15 @@  file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
 addr=0x1 \
 -device pcie-pci-bridge,id=pci.2,bus=pci.1,addr=0x0 \
 -device pcie-root-port,port=0x9,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x1 \
+-device pcie-root-port,port=0xa,chassis=4,id=pci.4,bus=pcie.0,addr=0x1.0x2 \
 -drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \
 -device virtio-blk-pci-transitional,scsi=off,bus=pci.2,addr=0x2,\
 drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 \
 -netdev user,id=hostnet0 \
 -device virtio-net-pci-transitional,netdev=hostnet0,id=net0,\
 mac=00:11:22:33:44:55,bus=pci.2,addr=0x1 \
+-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0,\
+bus=pci.3,addr=0x0 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2argvdata/virtio-transitional.xml b/tests/qemuxml2argvdata/virtio-transitional.xml
index 18f665577a..eddc1ce9f5 100644
--- a/tests/qemuxml2argvdata/virtio-transitional.xml
+++ b/tests/qemuxml2argvdata/virtio-transitional.xml
@@ -16,6 +16,9 @@ 
       <mac address='00:11:22:33:44:55'/>
       <model type='virtio-transitional'/>
     </interface>
+    <hostdev mode='subsystem' type='scsi_host' managed='no' model='virtio-transitional'>
+      <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
+    </hostdev>
     <controller type='usb' index='0' model='none'/>
     <memballoon model='none'/>
   </devices>
diff --git a/tests/qemuxml2xmloutdata/virtio-non-transitional.xml b/tests/qemuxml2xmloutdata/virtio-non-transitional.xml
index f0dc7c0833..2af5195dfd 100644
--- a/tests/qemuxml2xmloutdata/virtio-non-transitional.xml
+++ b/tests/qemuxml2xmloutdata/virtio-non-transitional.xml
@@ -40,6 +40,11 @@ 
       <target chassis='3' port='0xa'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
     </controller>
+    <controller type='pci' index='4' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='4' port='0xb'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x3'/>
+    </controller>
     <interface type='user'>
       <mac address='00:11:22:33:44:55'/>
       <model type='virtio-non-transitional'/>
@@ -47,6 +52,10 @@ 
     </interface>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
+    <hostdev mode='subsystem' type='scsi_host' managed='no' model='virtio-non-transitional'>
+      <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
+      <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/>
+    </hostdev>
     <memballoon model='none'/>
   </devices>
 </domain>
diff --git a/tests/qemuxml2xmloutdata/virtio-transitional.xml b/tests/qemuxml2xmloutdata/virtio-transitional.xml
index f9729391a5..ce7b109845 100644
--- a/tests/qemuxml2xmloutdata/virtio-transitional.xml
+++ b/tests/qemuxml2xmloutdata/virtio-transitional.xml
@@ -39,6 +39,11 @@ 
       <target chassis='3' port='0x9'/>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
     </controller>
+    <controller type='pci' index='4' model='pcie-root-port'>
+      <model name='pcie-root-port'/>
+      <target chassis='4' port='0xa'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
     <interface type='user'>
       <mac address='00:11:22:33:44:55'/>
       <model type='virtio-transitional'/>
@@ -46,6 +51,10 @@ 
     </interface>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
+    <hostdev mode='subsystem' type='scsi_host' managed='no' model='virtio-transitional'>
+      <source protocol='vhost' wwpn='naa.5123456789abcde0'/>
+      <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/>
+    </hostdev>
     <memballoon model='none'/>
   </devices>
 </domain>