[libvirt] [PATCH 1/2] qemu: Add ccw support for vhost-vsock

[Ján Tomko]

On Wed, Jul 04, 2018 at 01:21:44PM +0200, Boris Fiuczynski wrote:
>Add support and tests for vhost-vsock-ccw.
>
>Signed-off-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
>---
> src/qemu/qemu_command.c                       | 15 +++++++--
> src/qemu/qemu_domain_address.c                |  7 +++-
> .../vhost-vsock-ccw-auto.args                 | 27 ++++++++++++++++
> .../qemuxml2argvdata/vhost-vsock-ccw-auto.xml | 25 +++++++++++++++
> tests/qemuxml2argvdata/vhost-vsock-ccw.args   | 27 ++++++++++++++++
> tests/qemuxml2argvdata/vhost-vsock-ccw.xml    | 32 +++++++++++++++++++
> tests/qemuxml2argvtest.c                      |  4 +++
> .../vhost-vsock-ccw-auto.xml                  | 32 +++++++++++++++++++
> tests/qemuxml2xmloutdata/vhost-vsock-ccw.xml  |  1 +
> tests/qemuxml2xmltest.c                       |  5 +++
> 10 files changed, 172 insertions(+), 3 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw-auto.args
> create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw-auto.xml
> create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw.args
> create mode 100644 tests/qemuxml2argvdata/vhost-vsock-ccw.xml
> create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-ccw-auto.xml
> create mode 120000 tests/qemuxml2xmloutdata/vhost-vsock-ccw.xml
>
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index 04c5c28438..ab6944f68e 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -10089,10 +10089,21 @@ qemuBuildVsockDevStr(virDomainDefPtr def,
> {
>     qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData;
>     virBuffer buf = VIR_BUFFER_INITIALIZER;
>-    const char *device = "vhost-vsock-pci";
>     char *ret = NULL;
>+    const char *devsuffix;
>
>-    virBufferAsprintf(&buf, "%s", device);
>+    if (vsock->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>+        devsuffix = "pci";
>+    } else if (vsock->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
>+        devsuffix = "ccw";
>+    } else {

>+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+                       _("unsupported address type %s for vsock device"),
>+                       virDomainDeviceAddressTypeToString(vsock->info.type));
>+        goto cleanup;
>+    }

This check belongs in qemuDomainDeviceDefValidateVsock

>+
>+    virBufferAsprintf(&buf, "vhost-vsock-%s", devsuffix);

Having the whole device name in one string would be nicer than dealing
with the device suffix, e.g.:

if (type == _CCW)
    device = "vhost-vsock-ccw"
else
    device = "vhost-vsock-pci"

(or even virBufferAddLit, without the temporary variable - I forgot what
led me to use it)

>     virBufferAsprintf(&buf, ",id=%s", vsock->info.alias);
>     virBufferAsprintf(&buf, ",guest-cid=%u", vsock->guest_cid);
>     virBufferAsprintf(&buf, ",vhostfd=%s%u", fdprefix, priv->vhostfd);
>diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>index eb11a660d7..9639595175 100644
>--- a/src/qemu/qemu_domain_address.c
>+++ b/src/qemu/qemu_domain_address.c
>@@ -306,7 +306,8 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,
>        declare address-less virtio devices to be of address type 'type'
>        disks, networks, videos, consoles, controllers, memballoon and rng
>        in this order
>-       if type is ccw filesystem devices are declared to be of address type ccw
>+       if type is ccw filesystem and vsock devices are declared to be of
>+       address type ccw
>     */

This whole comment is pointless - it's repeating what the function does,
not why.

>     size_t i;
>

[...]

>+  <devices>
>+    <emulator>/usr/bin/qemu-system-s390x</emulator>
>+    <disk type='block' device='disk'>
>+      <driver name='qemu' type='raw'/>
>+      <source dev='/dev/HostVG/QEMUGuest1'/>
>+      <target dev='hda' bus='virtio'/>
>+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/>
>+    </disk>
>+    <memballoon model='virtio'>
>+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/>
>+    </memballoon>
>+    <panic model='s390'/>
>+    <vsock model='virtio'>
>+      <cid auto='no' address='4'/>
>+      <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0003'/>
>+    </vsock>
>+  </devices>
>+</domain>
>diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>index d6911f9344..60bd9585e5 100644
>--- a/tests/qemuxml2argvtest.c
>+++ b/tests/qemuxml2argvtest.c
>@@ -2914,6 +2914,10 @@ mymain(void)
>
>     DO_TEST_CAPS_LATEST("vhost-vsock");
>     DO_TEST_CAPS_LATEST("vhost-vsock-auto");
>+    DO_TEST("vhost-vsock-ccw", QEMU_CAPS_DEVICE_VHOST_VSOCK,
>+            QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
>+    DO_TEST("vhost-vsock-ccw-auto", QEMU_CAPS_DEVICE_VHOST_VSOCK,
>+            QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);

It would be nice to extend the LATEST caps coverage to s390 as well, to
make sure that adding a new capability won't affect the vsock device.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180710/ec7324f1/attachment-0001.sig>