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

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(a)linux.ibm.com&gt; > --- > 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 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list