diff mbox series

[07/18] qemu: Support hostdev model=virtio-{non-}transitional

Message ID 7a29cd9ffb8aa6df22e21965f38cf6949ad53cd4.1547746867.git.crobinso@redhat.com
State New
Headers show
Series qemu: virtio-{non-}transitional support | expand

Commit Message

Cole Robinson Jan. 17, 2019, 5:52 p.m. UTC 
Add <hostdev> protocol=vhost model handling for virtio transitional
devices. Ex:

  <hostdev mode='subsystem' type='scsi_host' model='virtio-transitional'>
    <source protocol='vhost' wwpn=X/>
  </hostdev>

* "virtio-transitional" maps to qemu "vhost-scsi-pci-transitional"
* "virtio-non-transitional" maps to qemu "vhost-scsi-pci-non-transitional"

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

---
 src/qemu/qemu_capabilities.c                     |  4 ++++
 src/qemu/qemu_capabilities.h                     |  2 ++
 src/qemu/qemu_command.c                          | 16 +++++++++++-----
 src/qemu/qemu_domain_address.c                   |  8 +++++---
 tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml |  2 ++
 .../virtio-non-transitional.x86_64-3.1.0.args    |  4 ++--
 .../virtio-non-transitional.x86_64-latest.args   |  4 ++--
 .../virtio-transitional.x86_64-3.1.0.args        |  3 +--
 .../virtio-transitional.x86_64-latest.args       |  5 ++---
 tests/qemuxml2xmloutdata/virtio-transitional.xml |  7 +------
 10 files changed, 32 insertions(+), 23 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:54 p.m. UTC | #1 
On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:
[...]
> @@ -1118,6 +1120,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {

>      {"virtio-blk-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL},

>      {"virtio-net-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL},

>      {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL},

> +    {"vhost-scsi-pci-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL},

> +    {"vhost-scsi-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL},

>  };


Usual comment about capabilities.

[...]
> @@ -5038,10 +5043,11 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def,

>          goto cleanup;

>      }

>  

> -    if (ARCH_IS_S390(def->os.arch))

> -        virBufferAddLit(&buf, "vhost-scsi-ccw");

> -    else

> -        virBufferAddLit(&buf, "vhost-scsi-pci");

> +    if (qemuBuildVirtioTransitional(&buf, "vhost-scsi", qemuCaps,

> +                                    dev->info->type,

> +                                    hostsrc->model, NULL,

> +                                    VIR_DOMAIN_DEVICE_HOSTDEV) < 0)

> +        goto cleanup;


This changes quite a bit: instead of basing the choice of device
upon the architecture and supporting -ccw and -pci only, now we
base it upon address type and potentially support -device as well
as-s390.

I'm assuming the latter is not going to be a problem because there
are probably checks along the way preventing us to get there in
the first place, but we should definitely make sure we address the
former correctly, most likely by setting the address type based on
the architecture in postParse().

Either way, once you've made sure the change is actually safe or
added code that makes it so, you should switch from the open-coded
version to qemuBuildVirtioDevStr(), and only later in a separate
commit switch from that to qemuBuildVirtioTransitional().

>  

>      virBufferAsprintf(&buf, ",wwpn=%s,vhostfd=%s,id=%s",

>                        hostsrc->wwpn,

> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c

> index 654567c500..c334ba441f 100644

> --- a/src/qemu/qemu_domain_address.c

> +++ b/src/qemu/qemu_domain_address.c

> @@ -785,11 +785,13 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,

>              return pcieFlags;

>  

>          /* according to pbonzini, from the guest PoV vhost-scsi devices

> -         * are the same as virtio-scsi, so they should use virtioFlags

> -         * (same as virtio-scsi) to determine Express vs. legacy placement

> +         * are the same as virtio-scsi, so they should follor virtio logic

>           */


s/follor/follow/

Everything else looks fine.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Cole Robinson Jan. 21, 2019, 11:33 p.m. UTC | #2 
On 01/18/2019 10:54 AM, Andrea Bolognani wrote:
> On Thu, 2019-01-17 at 12:52 -0500, Cole Robinson wrote:

> [...]

>> @@ -1118,6 +1120,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {

>>      {"virtio-blk-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL},

>>      {"virtio-net-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL},

>>      {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL},

>> +    {"vhost-scsi-pci-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL},

>> +    {"vhost-scsi-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL},

>>  };

> 

> Usual comment about capabilities.

> 

> [...]

>> @@ -5038,10 +5043,11 @@ qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def,

>>          goto cleanup;

>>      }

>>  

>> -    if (ARCH_IS_S390(def->os.arch))

>> -        virBufferAddLit(&buf, "vhost-scsi-ccw");

>> -    else

>> -        virBufferAddLit(&buf, "vhost-scsi-pci");

>> +    if (qemuBuildVirtioTransitional(&buf, "vhost-scsi", qemuCaps,

>> +                                    dev->info->type,

>> +                                    hostsrc->model, NULL,

>> +                                    VIR_DOMAIN_DEVICE_HOSTDEV) < 0)

>> +        goto cleanup;

> 

> This changes quite a bit: instead of basing the choice of device

> upon the architecture and supporting -ccw and -pci only, now we

> base it upon address type and potentially support -device as well

> as-s390.

> 

> I'm assuming the latter is not going to be a problem because there

> are probably checks along the way preventing us to get there in

> the first place, but we should definitely make sure we address the

> former correctly, most likely by setting the address type based on

> the architecture in postParse().

> 

> Either way, once you've made sure the change is actually safe or

> added code that makes it so, you should switch from the open-coded

> version to qemuBuildVirtioDevStr(), and only later in a separate

> commit switch from that to qemuBuildVirtioTransitional().


If I rework BuildVirtioTransitional like I mentioned in a previous
response, we don't need to touch this code. Then it can be cleaned up in
a separate patch after this series

Thanks,
Cole

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

Patch

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 44c4b890b9..70fc510cdb 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -526,6 +526,8 @@  VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
               /* 330 */
               "virtio-net-pci-non-transitional",
+              "vhost-scsi-pci-transitional",
+              "vhost-scsi-pci-non-transitional",
     );
 
 
@@ -1118,6 +1120,8 @@  struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
     {"virtio-blk-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_BLK_NON_TRANSITIONAL},
     {"virtio-net-pci-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL},
     {"virtio-net-pci-non-transitional", QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL},
+    {"vhost-scsi-pci-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL},
+    {"vhost-scsi-pci-non-transitional", QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL},
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 1a92c00575..f213ad98cc 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -510,6 +510,8 @@  typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */
 
     /* 330 */
     QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL, /* -device virtio-net-pci-non-transitional */
+    QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL, /* -device vhost-scsi-pci-transitional */
+    QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL, /* -device vhost-scsi-pci-non-transitional */
 
     QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d3b74211b7..7cdbd215a6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -471,13 +471,18 @@  qemuBuildVirtioTransitional(virBufferPtr buf,
             tmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_NET_TRANSITIONAL;
             ntmodel_cap = QEMU_CAPS_DEVICE_VIRTIO_NET_NON_TRANSITIONAL;
             break;
+        case VIR_DOMAIN_DEVICE_HOSTDEV:
+            has_tmodel = model == VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_VIRTIO_TRANSITIONAL;
+            has_ntmodel = model == VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_VIRTIO_NON_TRANSITIONAL;
+            tmodel_cap = QEMU_CAPS_DEVICE_VHOST_SCSI_TRANSITIONAL;
+            ntmodel_cap = QEMU_CAPS_DEVICE_VHOST_SCSI_NON_TRANSITIONAL;
+            break;
 
         case VIR_DOMAIN_DEVICE_LEASE:
         case VIR_DOMAIN_DEVICE_FS:
         case VIR_DOMAIN_DEVICE_INPUT:
         case VIR_DOMAIN_DEVICE_SOUND:
         case VIR_DOMAIN_DEVICE_VIDEO:
-        case VIR_DOMAIN_DEVICE_HOSTDEV:
         case VIR_DOMAIN_DEVICE_WATCHDOG:
         case VIR_DOMAIN_DEVICE_CONTROLLER:
         case VIR_DOMAIN_DEVICE_GRAPHICS:
@@ -5038,10 +5043,11 @@  qemuBuildSCSIVHostHostdevDevStr(const virDomainDef *def,
         goto cleanup;
     }
 
-    if (ARCH_IS_S390(def->os.arch))
-        virBufferAddLit(&buf, "vhost-scsi-ccw");
-    else
-        virBufferAddLit(&buf, "vhost-scsi-pci");
+    if (qemuBuildVirtioTransitional(&buf, "vhost-scsi", qemuCaps,
+                                    dev->info->type,
+                                    hostsrc->model, NULL,
+                                    VIR_DOMAIN_DEVICE_HOSTDEV) < 0)
+        goto cleanup;
 
     virBufferAsprintf(&buf, ",wwpn=%s,vhostfd=%s,id=%s",
                       hostsrc->wwpn,
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 654567c500..c334ba441f 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -785,11 +785,13 @@  qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
             return pcieFlags;
 
         /* according to pbonzini, from the guest PoV vhost-scsi devices
-         * are the same as virtio-scsi, so they should use virtioFlags
-         * (same as virtio-scsi) to determine Express vs. legacy placement
+         * are the same as virtio-scsi, so they should follor virtio logic
          */
-        if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST)
+        if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST) {
+            if (hostdev->source.subsys.u.scsi_host.model == VIR_DOMAIN_HOSTDEV_SUBSYS_SCSI_VHOST_MODEL_TYPE_VIRTIO_TRANSITIONAL)
+                return pciFlags;
             return virtioFlags;
+        }
 
         if (!(pciDev = virPCIDeviceNew(hostAddr->domain,
                                        hostAddr->bus,
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
index 137dcaf156..c5079c4028 100644
--- a/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml
@@ -216,6 +216,8 @@ 
   <flag name='virtio-blk-pci-non-transitional'/>
   <flag name='virtio-net-pci-transitional'/>
   <flag name='virtio-net-pci-non-transitional'/>
+  <flag name='vhost-scsi-pci-transitional'/>
+  <flag name='vhost-scsi-pci-non-transitional'/>
   <version>3001050</version>
   <kvmVersion>0</kvmVersion>
   <microcodeVersion>446361</microcodeVersion>
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 2eaa509c04..5ab8560377 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
@@ -34,8 +34,8 @@  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 \
+-device vhost-scsi-pci,disable-legacy=on,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 82255909c4..c8dbffda65 100644
--- a/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/virtio-non-transitional.x86_64-latest.args
@@ -34,8 +34,8 @@  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 \
+-device vhost-scsi-pci-non-transitional,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-3.1.0.args b/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args
index 4e991d6187..38a9e348b3 100644
--- a/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args
+++ b/tests/qemuxml2argvdata/virtio-transitional.x86_64-3.1.0.args
@@ -27,7 +27,6 @@  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 \
@@ -35,7 +34,7 @@  id=virtio-disk0,bootindex=1 \
 -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 \
+bus=pci.2,addr=0x3 \
 -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 dab25ba2e8..ab2c35514d 100644
--- a/tests/qemuxml2argvdata/virtio-transitional.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/virtio-transitional.x86_64-latest.args
@@ -27,15 +27,14 @@  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 \
+-device vhost-scsi-pci-transitional,wwpn=naa.5123456789abcde0,vhostfd=3,\
+id=hostdev0,bus=pci.2,addr=0x3 \
 -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
 resourcecontrol=deny \
 -msg timestamp=on
diff --git a/tests/qemuxml2xmloutdata/virtio-transitional.xml b/tests/qemuxml2xmloutdata/virtio-transitional.xml
index ce7b109845..8c1baced0e 100644
--- a/tests/qemuxml2xmloutdata/virtio-transitional.xml
+++ b/tests/qemuxml2xmloutdata/virtio-transitional.xml
@@ -39,11 +39,6 @@ 
       <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'/>
@@ -53,7 +48,7 @@ 
     <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'/>
+      <address type='pci' domain='0x0000' bus='0x02' slot='0x03' function='0x0'/>
     </hostdev>
     <memballoon model='none'/>
   </devices>