On 07/10/2018 01:37 PM, Ján Tomko wrote:
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@xxxxxxxxxxxxx>
---
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
Changed it.
+
+ 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)
Changed 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.
Well I did not create the original comment but adjusted it for
completeness. Since the function name is not really speaking for itself
I kind of understand the intend. Do you suggest that I should delete the
comment completely?
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.
I agree and John already made me aware of that. Have not yet gotten to
it but soon hope to. Can we leave it as is for now and change it when
LATEST caps has been extended to s390?
Jano
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
Mit freundlichen Grüßen/Kind regards
Boris Fiuczynski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
