[libvirt] [PATCH v1 0/7] Add support for video and input devices on S390

Hi, This patch series adds Libvirt support for video and input devices for QEMU guests on S390. QEMU v2.11.0 added support for the virtio-gpu-ccw device [1] and virtio-{keyboard, mouse, tablet}-ccw devices [2], which can be used as video and input devices respectively. Thanks Farhan [1] https://git.qemu.org/?p=qemu.git;a=commit;h=1f8ad88935f5cb5a2968909e392dbeee... [2] https://git.qemu.org/?p=qemu.git;a=commit;h=3382cf1fabbf722dce931846853dae71... Farhan Ali (7): qemu: Fix comment for 'qemuValidateDevicePCISlotsChipsets' qemu: Add support for virtio-gpu-ccw device tests: Add test case for virtio-gpu-ccw qemu: Use correct bus type for input devices qemu: Add support for virtio input ccw devices tests: Add test case for virtio input ccw devices news: Update for virtio-gpu-ccw and virtio input ccw devices docs/formatdomain.html.in | 5 ++ docs/news.xml | 10 +++ src/qemu/qemu_capabilities.c | 13 ++++ src/qemu/qemu_capabilities.h | 6 ++ src/qemu/qemu_command.c | 32 +++++++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 12 +++- src/qemu/qemu_process.c | 5 +- .../qemucapabilitiesdata/caps_2.11.0.s390x.replies | 83 +++++++++++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 6 +- tests/qemuxml2argvdata/input-virtio-ccw.args | 26 +++++++ tests/qemuxml2argvdata/input-virtio-ccw.xml | 29 ++++++++ .../qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml | 18 +++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.args | 25 +++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml | 35 +++++++++ tests/qemuxml2argvtest.c | 15 ++++ tests/qemuxml2xmloutdata/input-virtio-ccw.xml | 36 ++++++++++ .../video-virtio-gpu-ccw-auto.xml | 34 +++++++++ tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml | 38 ++++++++++ tests/qemuxml2xmltest.c | 23 ++++++ 20 files changed, 434 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.args create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/input-virtio-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml -- 2.7.4

There is no function 'qemuValidateDevicePCISlotsChipsets', it should be qemuDomainValidateDevicePCISlotsChipsets. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 3fcb36a..ae49cf2 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1815,7 +1815,7 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, * - Video (slot 2) * * - These integrated devices were already added by - * qemuValidateDevicePCISlotsChipsets invoked right before this function + * qemuDomainValidateDevicePCISlotsChipsets invoked right before this function * * Incrementally assign slots from 3 onwards: * -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
There is no function 'qemuValidateDevicePCISlotsChipsets', it should be qemuDomainValidateDevicePCISlotsChipsets.
Commit id '177db487' renamed 'qemuValidateDevicePCISlotsChipsets' to 'qemuDomainValidateDevicePCISlotsChipsets', but didn't adjust comment.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John (I'll push this one shortly)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 3fcb36a..ae49cf2 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1815,7 +1815,7 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, * - Video (slot 2) * * - These integrated devices were already added by - * qemuValidateDevicePCISlotsChipsets invoked right before this function + * qemuDomainValidateDevicePCISlotsChipsets invoked right before this function * * Incrementally assign slots from 3 onwards: *

QEMU on S390 (since v2.11) can support the virtio-gpu-ccw device, so on S390 assign CCW address for a video device. Also introduce a new capability for the virtio-gpu-ccw device. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 3 + src/qemu/qemu_capabilities.c | 5 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 ++++- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 8 +++ src/qemu/qemu_process.c | 5 +- .../qemucapabilitiesdata/caps_2.11.0.s390x.replies | 83 +++++++++++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 3 +- 9 files changed, 114 insertions(+), 14 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6fd2189..0908709 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6484,6 +6484,9 @@ qemu-kvm -net nic,model=? /dev/null <dd> The optional <code>address</code> sub-element can be used to tie the video device to a particular PCI slot. + On S390, <code>address</code> can be used to provide the + CCW address for the video device (<span class="since"> + since 4.2.0</span>). </dd> <dt><code>driver</code></dt> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b5eb8cf..9db4c31 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "virtio-gpu-ccw", ); @@ -1694,6 +1695,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "virtio-gpu-ccw", QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { @@ -1932,6 +1934,9 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "spapr-pci-host-bridge", virQEMUCapsObjectPropsSpaprPCIHostBridge, ARRAY_CARDINALITY(virQEMUCapsObjectPropsSpaprPCIHostBridge), QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE }, + { "virtio-gpu-ccw", virQEMUCapsObjectPropsVirtioGpu, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu), + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, }; struct virQEMUCapsPropTypeObjects { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be..b4852e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW, /* -device virtio-gpu-ccw */ 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 fa0aa5d..ba63670 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -132,7 +132,7 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl", "", /* don't support parallels */ - "virtio-gpu-pci", + "virtio-gpu", "" /* don't support gop */); VIR_ENUM_DECL(qemuSoundCodec) @@ -4262,7 +4262,21 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, goto error; } - virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias); + if (STREQ(model, "virtio-gpu")) { + switch (video->info.type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + virBufferAsprintf(&buf, "%s-pci", model); + break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: + virBufferAsprintf(&buf, "%s-ccw", model); + break; + } + } else { + virBufferAsprintf(&buf, "%s", model); + } + + virBufferAsprintf(&buf, ",id=%s", video->info.alias); if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",virgl=%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee02ecd..b47b4d1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5156,7 +5156,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { if ARCH_IS_PPC64(def->os.arch) dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; - else if (qemuDomainIsVirt(def)) + else if (qemuDomainIsVirt(def) || ARCH_IS_S390(def->os.arch)) dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO; else dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index ae49cf2..49a293e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -308,6 +308,14 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, } } + for (i = 0; i < def->nvideos; i++) { + virDomainVideoDefPtr video = def->videos[i]; + + if (video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) + video->info.type = type; + } + for (i = 0; i < def->ninputs; i++) { if (def->inputs[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && def->inputs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 57c06c7..1afb71f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4778,7 +4778,10 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm, (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) { + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("this QEMU does not support '%s' video device"), virDomainVideoTypeToString(video->type)); diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies index 6768937..53c4804 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies @@ -3515,6 +3515,71 @@ { "return": [ { + "name": "notify_on_empty", + "description": "on/off", + "type": "bool" + }, + { + "name": "ioeventfd", + "description": "on/off", + "type": "bool" + }, + { + "name": "any_layout", + "description": "on/off", + "type": "bool" + }, + { + "name": "devno", + "description": "Identifier of an I/O device in the channel subsystem, example: fe.1.23ab", + "type": "str" + }, + { + "name": "indirect_desc", + "description": "on/off", + "type": "bool" + }, + { + "name": "event_idx", + "description": "on/off", + "type": "bool" + }, + { + "name": "virtio-backend", + "type": "child<virtio-gpu-device>" + }, + { + "name": "yres", + "type": "uint32" + }, + { + "name": "xres", + "type": "uint32" + }, + { + "name": "iommu_platform", + "description": "on/off", + "type": "bool" + }, + { + "name": "max_outputs", + "type": "uint32" + }, + { + "name": "max_hostmem", + "type": "size" + }, + { + "name": "max_revision", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-2.7", "cpu-max": 248 @@ -3562,7 +3627,7 @@ "cpu-max": 248 } ], - "id": "libvirt-41" + "id": "libvirt-42" } { @@ -4096,20 +4161,20 @@ "migration-safe": true } ], - "id": "libvirt-42" + "id": "libvirt-43" } { "return": [ ], - "id": "libvirt-43" + "id": "libvirt-44" } { "return": [ "emulator" ], - "id": "libvirt-44" + "id": "libvirt-45" } { @@ -5230,7 +5295,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" } { @@ -5288,7 +5353,7 @@ "capability": "x-multifd" } ], - "id": "libvirt-46" + "id": "libvirt-47" } { @@ -15156,7 +15221,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" } { @@ -15195,11 +15260,11 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" } { - "id": "libvirt-49", + "id": "libvirt-50", "error": { "class": "GenericError", "desc": "Property '.migratable' not found" diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index f7f102f..ec5c396 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -146,9 +146,10 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> + <flag name='virtio-gpu-ccw'/> <version>2011000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>341724</microcodeVersion> + <microcodeVersion>342885</microcodeVersion> <package></package> <arch>s390x</arch> <hostCPU type='kvm' model='z14-base' migratability='no'> -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
QEMU on S390 (since v2.11) can support the virtio-gpu-ccw device, so on S390 assign CCW address for a video device.
Also introduce a new capability for the virtio-gpu-ccw device.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 3 + src/qemu/qemu_capabilities.c | 5 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 ++++- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 8 +++ src/qemu/qemu_process.c | 5 +- .../qemucapabilitiesdata/caps_2.11.0.s390x.replies | 83 +++++++++++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 3 +- 9 files changed, 114 insertions(+), 14 deletions(-)
First off - other upstream changes has caused this to not be able to apply w/ git am -3 - so once patch1 is pushed, you'll need to rework and repost the series (hopefully this time you won't have the strange split with a CC address having "--dry-run" appended to an email ;-) Since I cannot compile this one - I'll just be able to go through visually and note a few things... Primarily though - this particular patch should be split up a bit more. Separate out the capabilities into their own separate "capabilities" patch and the second patch becomes a "functionality" patch.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6fd2189..0908709 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6484,6 +6484,9 @@ qemu-kvm -net nic,model=? /dev/null <dd> The optional <code>address</code> sub-element can be used to tie the video device to a particular PCI slot. + On S390, <code>address</code> can be used to provide the + CCW address for the video device (<span class="since"> + since 4.2.0</span>). </dd>
<dt><code>driver</code></dt>
... The above would go with the "functionality" patch and the next few with a "capability" patch...
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b5eb8cf..9db4c31 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "virtio-gpu-ccw", );
@@ -1694,6 +1695,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "virtio-gpu-ccw", QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { @@ -1932,6 +1934,9 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "spapr-pci-host-bridge", virQEMUCapsObjectPropsSpaprPCIHostBridge, ARRAY_CARDINALITY(virQEMUCapsObjectPropsSpaprPCIHostBridge), QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE }, + { "virtio-gpu-ccw", virQEMUCapsObjectPropsVirtioGpu, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu), + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, };
struct virQEMUCapsPropTypeObjects { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be..b4852e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW, /* -device virtio-gpu-ccw */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;
... Above here with the "capability" patch and below with the "functionality" patch...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fa0aa5d..ba63670 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -132,7 +132,7 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl", "", /* don't support parallels */ - "virtio-gpu-pci", + "virtio-gpu", "" /* don't support gop */);
VIR_ENUM_DECL(qemuSoundCodec) @@ -4262,7 +4262,21 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, goto error; }
- virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias); + if (STREQ(model, "virtio-gpu")) {
Rather than STREQ comparison perhaps: if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { since that's what derives "virtio-gpu" for model anyway.
+ switch (video->info.type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + virBufferAsprintf(&buf, "%s-pci", model); + break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: + virBufferAsprintf(&buf, "%s-ccw", model); + break; + }
Let's not use a switch here - as that implies there could be more than just two values in the '.type' field... I'd go with: if (video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) virBufferAsprintf(&buf, "%s-ccw", model); else virBufferAsprintf(&buf, "%s-pci", model);
+ } else { + virBufferAsprintf(&buf, "%s", model); + } + + virBufferAsprintf(&buf, ",id=%s", video->info.alias);
if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",virgl=%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee02ecd..b47b4d1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5156,7 +5156,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { if ARCH_IS_PPC64(def->os.arch) dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; - else if (qemuDomainIsVirt(def)) + else if (qemuDomainIsVirt(def) || ARCH_IS_S390(def->os.arch)) dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO; else dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index ae49cf2..49a293e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -308,6 +308,14 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, } }
There's a comment at the top of the function that could add "video" to the list as well...
+ for (i = 0; i < def->nvideos; i++) { + virDomainVideoDefPtr video = def->videos[i]; + + if (video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) + video->info.type = type; + } + for (i = 0; i < def->ninputs; i++) { if (def->inputs[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && def->inputs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 57c06c7..1afb71f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4778,7 +4778,10 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm, (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) { + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("this QEMU does not support '%s' video device"), virDomainVideoTypeToString(video->type));
... The rest would go with the capabilities patch....
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies index 6768937..53c4804 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies @@ -3515,6 +3515,71 @@ { "return": [ { + "name": "notify_on_empty", + "description": "on/off", + "type": "bool" + }, + { + "name": "ioeventfd", + "description": "on/off", + "type": "bool" + }, + { + "name": "any_layout", + "description": "on/off", + "type": "bool" + }, + { + "name": "devno", + "description": "Identifier of an I/O device in the channel subsystem, example: fe.1.23ab", + "type": "str" + }, + { + "name": "indirect_desc", + "description": "on/off", + "type": "bool" + }, + { + "name": "event_idx", + "description": "on/off", + "type": "bool" + }, + { + "name": "virtio-backend", + "type": "child<virtio-gpu-device>" + }, + { + "name": "yres", + "type": "uint32" + }, + { + "name": "xres", + "type": "uint32" + }, + { + "name": "iommu_platform", + "description": "on/off", + "type": "bool" + }, + { + "name": "max_outputs", + "type": "uint32" + }, + { + "name": "max_hostmem", + "type": "size" + }, + { + "name": "max_revision", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-2.7", "cpu-max": 248 @@ -3562,7 +3627,7 @@ "cpu-max": 248 } ], - "id": "libvirt-41" + "id": "libvirt-42" }
{ @@ -4096,20 +4161,20 @@ "migration-safe": true } ], - "id": "libvirt-42" + "id": "libvirt-43" }
{ "return": [ ], - "id": "libvirt-43" + "id": "libvirt-44" }
{ "return": [ "emulator" ], - "id": "libvirt-44" + "id": "libvirt-45" }
{ @@ -5230,7 +5295,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" }
{ @@ -5288,7 +5353,7 @@ "capability": "x-multifd" } ], - "id": "libvirt-46" + "id": "libvirt-47" }
{ @@ -15156,7 +15221,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" }
{ @@ -15195,11 +15260,11 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" }
{ - "id": "libvirt-49", + "id": "libvirt-50", "error": { "class": "GenericError", "desc": "Property '.migratable' not found" diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index f7f102f..ec5c396 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -146,9 +146,10 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> + <flag name='virtio-gpu-ccw'/> <version>2011000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>341724</microcodeVersion> + <microcodeVersion>342885</microcodeVersion>
<sigh> - I assume this is because someone updated something after the original pull of the 2.11 caps... You used the VIR_TEST_REGENERATE_OUTPUT=1 to create too, right? John
<package></package> <arch>s390x</arch> <hostCPU type='kvm' model='z14-base' migratability='no'>

Hi Jon, On 03/16/2018 09:38 AM, John Ferlan wrote:
On 03/08/2018 11:07 AM, Farhan Ali wrote:
QEMU on S390 (since v2.11) can support the virtio-gpu-ccw device, so on S390 assign CCW address for a video device.
Also introduce a new capability for the virtio-gpu-ccw device.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 3 + src/qemu/qemu_capabilities.c | 5 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 ++++- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 8 +++ src/qemu/qemu_process.c | 5 +- .../qemucapabilitiesdata/caps_2.11.0.s390x.replies | 83 +++++++++++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 3 +- 9 files changed, 114 insertions(+), 14 deletions(-)
First off - other upstream changes has caused this to not be able to apply w/ git am -3 - so once patch1 is pushed, you'll need to rework and repost the series (hopefully this time you won't have the strange split with a CC address having "--dry-run" appended to an email ;-)
Ah yes, it was silly of me! I didn't realize till it was too late :)
Since I cannot compile this one - I'll just be able to go through visually and note a few things... Primarily though - this particular patch should be split up a bit more. Separate out the capabilities into their own separate "capabilities" patch and the second patch becomes a "functionality" patch.
Sure, I will move capabilities into a different patch.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6fd2189..0908709 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6484,6 +6484,9 @@ qemu-kvm -net nic,model=? /dev/null <dd> The optional <code>address</code> sub-element can be used to tie the video device to a particular PCI slot. + On S390, <code>address</code> can be used to provide the + CCW address for the video device (<span class="since"> + since 4.2.0</span>). </dd>
<dt><code>driver</code></dt>
... The above would go with the "functionality" patch and the next few with a "capability" patch...
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index b5eb8cf..9db4c31 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -459,6 +459,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "pl011", "machine.pseries.max-cpu-compat", "dump-completed", + "virtio-gpu-ccw", );
@@ -1694,6 +1695,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "sclplmconsole", QEMU_CAPS_DEVICE_SCLPLMCONSOLE }, { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, + { "virtio-gpu-ccw", QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { @@ -1932,6 +1934,9 @@ static struct virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = { { "spapr-pci-host-bridge", virQEMUCapsObjectPropsSpaprPCIHostBridge, ARRAY_CARDINALITY(virQEMUCapsObjectPropsSpaprPCIHostBridge), QEMU_CAPS_DEVICE_SPAPR_PCI_HOST_BRIDGE }, + { "virtio-gpu-ccw", virQEMUCapsObjectPropsVirtioGpu, + ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu), + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, };
struct virQEMUCapsPropTypeObjects { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c2ec2be..b4852e5 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -444,6 +444,7 @@ typedef enum { QEMU_CAPS_DEVICE_PL011, /* -device pl011 (not user-instantiable) */ QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW, /* -device virtio-gpu-ccw */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags;
... Above here with the "capability" patch and below with the "functionality" patch...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fa0aa5d..ba63670 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -132,7 +132,7 @@ VIR_ENUM_IMPL(qemuDeviceVideoSecondary, VIR_DOMAIN_VIDEO_TYPE_LAST, "", /* don't support vbox */ "qxl", "", /* don't support parallels */ - "virtio-gpu-pci", + "virtio-gpu", "" /* don't support gop */);
VIR_ENUM_DECL(qemuSoundCodec) @@ -4262,7 +4262,21 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, goto error; }
- virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias); + if (STREQ(model, "virtio-gpu")) {
Rather than STREQ comparison perhaps:
if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
since that's what derives "virtio-gpu" for model anyway.
VIR_DOMAIN_VIDEO_TYPE_VIRTIO can be both "virtio-gpu" (qemuDeviceVideoSecondary) and "virtio-vga" (qemuDeviceVideo), so that's why I didn't use video.type check.
+ switch (video->info.type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + virBufferAsprintf(&buf, "%s-pci", model); + break; + + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: + virBufferAsprintf(&buf, "%s-ccw", model); + break; + }
Let's not use a switch here - as that implies there could be more than just two values in the '.type' field... I'd go with:
Sure.
if (video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) virBufferAsprintf(&buf, "%s-ccw", model); else virBufferAsprintf(&buf, "%s-pci", model);
+ } else { + virBufferAsprintf(&buf, "%s", model); + } + + virBufferAsprintf(&buf, ",id=%s", video->info.alias);
if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",virgl=%s", diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee02ecd..b47b4d1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5156,7 +5156,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_DEFAULT) { if ARCH_IS_PPC64(def->os.arch) dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VGA; - else if (qemuDomainIsVirt(def)) + else if (qemuDomainIsVirt(def) || ARCH_IS_S390(def->os.arch)) dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_VIRTIO; else dev->data.video->type = VIR_DOMAIN_VIDEO_TYPE_CIRRUS; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index ae49cf2..49a293e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -308,6 +308,14 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, } }
There's a comment at the top of the function that could add "video" to the list as well...
+ for (i = 0; i < def->nvideos; i++) { + virDomainVideoDefPtr video = def->videos[i]; + + if (video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) + video->info.type = type; + } + for (i = 0; i < def->ninputs; i++) { if (def->inputs[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && def->inputs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 57c06c7..1afb71f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4778,7 +4778,10 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm, (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) || (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))) { + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) || + (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("this QEMU does not support '%s' video device"), virDomainVideoTypeToString(video->type));
... The rest would go with the capabilities patch....
diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies index 6768937..53c4804 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.replies @@ -3515,6 +3515,71 @@ { "return": [ { + "name": "notify_on_empty", + "description": "on/off", + "type": "bool" + }, + { + "name": "ioeventfd", + "description": "on/off", + "type": "bool" + }, + { + "name": "any_layout", + "description": "on/off", + "type": "bool" + }, + { + "name": "devno", + "description": "Identifier of an I/O device in the channel subsystem, example: fe.1.23ab", + "type": "str" + }, + { + "name": "indirect_desc", + "description": "on/off", + "type": "bool" + }, + { + "name": "event_idx", + "description": "on/off", + "type": "bool" + }, + { + "name": "virtio-backend", + "type": "child<virtio-gpu-device>" + }, + { + "name": "yres", + "type": "uint32" + }, + { + "name": "xres", + "type": "uint32" + }, + { + "name": "iommu_platform", + "description": "on/off", + "type": "bool" + }, + { + "name": "max_outputs", + "type": "uint32" + }, + { + "name": "max_hostmem", + "type": "size" + }, + { + "name": "max_revision", + "type": "uint32" + } + ], + "id": "libvirt-41" +} + +{ + "return": [ + { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-2.7", "cpu-max": 248 @@ -3562,7 +3627,7 @@ "cpu-max": 248 } ], - "id": "libvirt-41" + "id": "libvirt-42" }
{ @@ -4096,20 +4161,20 @@ "migration-safe": true } ], - "id": "libvirt-42" + "id": "libvirt-43" }
{ "return": [ ], - "id": "libvirt-43" + "id": "libvirt-44" }
{ "return": [ "emulator" ], - "id": "libvirt-44" + "id": "libvirt-45" }
{ @@ -5230,7 +5295,7 @@ "option": "drive" } ], - "id": "libvirt-45" + "id": "libvirt-46" }
{ @@ -5288,7 +5353,7 @@ "capability": "x-multifd" } ], - "id": "libvirt-46" + "id": "libvirt-47" }
{ @@ -15156,7 +15221,7 @@ "meta-type": "object" } ], - "id": "libvirt-47" + "id": "libvirt-48" }
{ @@ -15195,11 +15260,11 @@ } } }, - "id": "libvirt-48" + "id": "libvirt-49" }
{ - "id": "libvirt-49", + "id": "libvirt-50", "error": { "class": "GenericError", "desc": "Property '.migratable' not found" diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index f7f102f..ec5c396 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -146,9 +146,10 @@ <flag name='disk-share-rw'/> <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> + <flag name='virtio-gpu-ccw'/> <version>2011000</version> <kvmVersion>0</kvmVersion> - <microcodeVersion>341724</microcodeVersion> + <microcodeVersion>342885</microcodeVersion>
<sigh> - I assume this is because someone updated something after the original pull of the 2.11 caps...
You used the VIR_TEST_REGENERATE_OUTPUT=1 to create too, right?
John
<package></package> <arch>s390x</arch> <hostCPU type='kvm' model='z14-base' migratability='no'>

A test case to test the virtio-gpu-ccw device. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml | 18 ++++++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.args | 25 ++++++++++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml | 35 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ .../video-virtio-gpu-ccw-auto.xml | 34 +++++++++++++++++++ tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml | 38 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 14 ++++++++ 7 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml b/tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml new file mode 100644 index 0000000..2d5da38 --- /dev/null +++ b/tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1803</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + </disk> + <graphics type='vnc'/> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-ccw.args b/tests/qemuxml2argvdata/video-virtio-gpu-ccw.args new file mode 100644 index 0000000..e5e8854 --- /dev/null +++ b/tests/qemuxml2argvdata/video-virtio-gpu-ccw.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1803 \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-ccw,devno=fe.0.0000,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-vnc 127.0.0.1:0 \ +-device virtio-gpu-ccw,id=video0,max_outputs=1,devno=fe.0.0002 \ +-device virtio-gpu-ccw,id=video1,max_outputs=1,devno=fe.0.0003 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml b/tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml new file mode 100644 index 0000000..b327595 --- /dev/null +++ b/tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1803</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </disk> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + <graphics type='vnc'/> + <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/> + </video> + <video> + <model type='virtio' heads='1'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0003'/> + </video> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 688846b..59e5bda 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2966,6 +2966,13 @@ mymain(void) QEMU_CAPS_HDA_DUPLEX); DO_TEST("user-aliases2", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_ICH9_AHCI); + DO_TEST("video-virtio-gpu-ccw", QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); diff --git a/tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml b/tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml new file mode 100644 index 0000000..c198dad --- /dev/null +++ b/tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1803</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </disk> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </video> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml b/tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml new file mode 100644 index 0000000..d293183 --- /dev/null +++ b/tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml @@ -0,0 +1,38 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1803</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </disk> + <graphics type='vnc' port='-1' autoport='yes'> + <listen type='address'/> + </graphics> + <video> + <model type='virtio' heads='1' primary='yes'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/> + </video> + <video> + <model type='virtio' heads='1'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0003'/> + </video> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 0eb9e6c..766f143 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1310,6 +1310,20 @@ mymain(void) DO_TEST("video-qxl-heads", NONE); DO_TEST("video-qxl-noheads", NONE); DO_TEST("video-virtio-gpu-secondary", NONE); + DO_TEST("video-virtio-gpu-ccw", + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW); + DO_TEST("video-virtio-gpu-ccw-auto", + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_DEVICE_VIRTIO_GPU, + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS, + QEMU_CAPS_VNC, + QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW); DO_TEST("intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU); -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
A test case to test the virtio-gpu-ccw device.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml | 18 ++++++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.args | 25 ++++++++++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml | 35 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ .../video-virtio-gpu-ccw-auto.xml | 34 +++++++++++++++++++ tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml | 38 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 14 ++++++++ 7 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml
The tests look OK - could have been merged with the "functionality" patch, but it's OK that they're separate - as long as they exist it's a good thing. I assume the xml2xml output would be "valid" after the "capability" patch - if so you could extract it into it's own patch if you felt really compelled to do so. John

On 03/16/2018 09:39 AM, John Ferlan wrote:
On 03/08/2018 11:07 AM, Farhan Ali wrote:
A test case to test the virtio-gpu-ccw device.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- .../qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml | 18 ++++++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.args | 25 ++++++++++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml | 35 ++++++++++++++++++++ tests/qemuxml2argvtest.c | 7 ++++ .../video-virtio-gpu-ccw-auto.xml | 34 +++++++++++++++++++ tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml | 38 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 14 ++++++++ 7 files changed, 171 insertions(+) create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml
The tests look OK - could have been merged with the "functionality" patch, but it's OK that they're separate - as long as they exist it's a good thing.
I assume the xml2xml output would be "valid" after the "capability" patch - if so you could extract it into it's own patch if you felt really compelled to do so.
John
I just feel it's easier to understand to have them all in one patch :)

commit 7210cef452db 'qemu: build command line for virtio input devices' introduced an error, by checking if input bus type is VIR_DOMAIN_DISK_BUS_VIRTIO. Fix it by using the correct bus type for input devices. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 49a293e..d66c3d0 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -317,7 +317,7 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, } for (i = 0; i < def->ninputs; i++) { - if (def->inputs[i]->bus == VIR_DOMAIN_DISK_BUS_VIRTIO && + if (def->inputs[i]->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO && def->inputs[i]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) def->inputs[i]->info.type = type; } -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
commit 7210cef452db 'qemu: build command line for virtio input devices' introduced an error, by checking if input bus type is VIR_DOMAIN_DISK_BUS_VIRTIO.
Fix it by using the correct bus type for input devices.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Nice catch! Been around awhile too. Doesn't look like there was a test to catch it either. Reviewed-by: John Ferlan <jferlan@redhat.com> John (I'll push this along with patch 1 since it's separable)

On 03/16/2018 09:39 AM, John Ferlan wrote:
On 03/08/2018 11:07 AM, Farhan Ali wrote:
commit 7210cef452db 'qemu: build command line for virtio input devices' introduced an error, by checking if input bus type is VIR_DOMAIN_DISK_BUS_VIRTIO.
Fix it by using the correct bus type for input devices.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/qemu/qemu_domain_address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Nice catch! Been around awhile too. Doesn't look like there was a test to catch it either.
Hopefully my virtio ccw input tests should be enough to cover this?
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
(I'll push this along with patch 1 since it's separable)
Thanks so much for all your review, I really appreciate all your feeedback :)

QEMU on S390 (since v2.11) can support virtio input ccw devices. So build the qemu command line for ccw devices. Also introduce capabilities for virtio-{keyboard, mouse, tablet}-ccw devices. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 2 ++ src/qemu/qemu_capabilities.c | 8 ++++++++ src/qemu/qemu_capabilities.h | 5 +++++ src/qemu/qemu_command.c | 14 +++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 3 +++ 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0908709..08dc74b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6048,6 +6048,8 @@ qemu-kvm -net nic,model=? /dev/null sub-element <code><address></code> which can tie the device to a particular PCI slot, <a href="#elementsAddress">documented above</a>. + On S390, <code>address</code> can be used to provide a CCW address for + an input device (<span class="since">since 4.2.0</span>). For type <code>passthrough</code>, the mandatory sub-element <code>source</code> must have an <code>evdev</code> attribute containing the absolute path to the diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9db4c31..14564e8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -460,6 +460,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "machine.pseries.max-cpu-compat", "dump-completed", "virtio-gpu-ccw", + "virtio-keyboard-ccw", + + /* 285 */ + "virtio-mouse-ccw", + "virtio-tablet-ccw", ); @@ -1696,6 +1701,9 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, { "virtio-gpu-ccw", QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, + { "virtio-keyboard-ccw", QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW }, + { "virtio-mouse-ccw", QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW }, + { "virtio-tablet-ccw", QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b4852e5..3f3c29f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -445,6 +445,11 @@ typedef enum { QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW, /* -device virtio-gpu-ccw */ + QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW, /* -device virtio-keyboard-ccw */ + + /* 285 */ + QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, /* -device virtio-mouse-ccw */ + QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */ 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 ba63670..5477e14 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3921,6 +3921,8 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { suffix = "-pci"; + } else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + suffix = "-ccw"; } else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { suffix = "-device"; } else { @@ -3932,7 +3934,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, switch ((virDomainInputType) dev->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE) || + (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-mouse is not supported by this QEMU binary")); goto error; @@ -3940,7 +3944,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, virBufferAsprintf(&buf, "virtio-mouse%s,id=%s", suffix, dev->info.alias); break; case VIR_DOMAIN_INPUT_TYPE_TABLET: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) || + (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-tablet is not supported by this QEMU binary")); goto error; @@ -3948,7 +3954,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, virBufferAsprintf(&buf, "virtio-tablet%s,id=%s", suffix, dev->info.alias); break; case VIR_DOMAIN_INPUT_TYPE_KBD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) || + (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-keyboard is not supported by this QEMU binary")); goto error; diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index ec5c396..fda5c36 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -147,6 +147,9 @@ <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> <flag name='virtio-gpu-ccw'/> + <flag name='virtio-keyboard-ccw'/> + <flag name='virtio-mouse-ccw'/> + <flag name='virtio-tablet-ccw'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342885</microcodeVersion> -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
QEMU on S390 (since v2.11) can support virtio input ccw devices. So build the qemu command line for ccw devices.
Also introduce capabilities for virtio-{keyboard, mouse, tablet}-ccw devices.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 2 ++ src/qemu/qemu_capabilities.c | 8 ++++++++ src/qemu/qemu_capabilities.h | 5 +++++ src/qemu/qemu_command.c | 14 +++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 3 +++ 5 files changed, 29 insertions(+), 3 deletions(-)
Similar to earlier - let's split the "capability" and "functionality" John
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0908709..08dc74b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6048,6 +6048,8 @@ qemu-kvm -net nic,model=? /dev/null sub-element <code><address></code> which can tie the device to a particular PCI slot, <a href="#elementsAddress">documented above</a>. + On S390, <code>address</code> can be used to provide a CCW address for + an input device (<span class="since">since 4.2.0</span>).
For type <code>passthrough</code>, the mandatory sub-element <code>source</code> must have an <code>evdev</code> attribute containing the absolute path to the diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 9db4c31..14564e8 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -460,6 +460,11 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "machine.pseries.max-cpu-compat", "dump-completed", "virtio-gpu-ccw", + "virtio-keyboard-ccw", + + /* 285 */ + "virtio-mouse-ccw", + "virtio-tablet-ccw", );
@@ -1696,6 +1701,9 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "isa-serial", QEMU_CAPS_DEVICE_ISA_SERIAL }, { "pl011", QEMU_CAPS_DEVICE_PL011 }, { "virtio-gpu-ccw", QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW }, + { "virtio-keyboard-ccw", QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW }, + { "virtio-mouse-ccw", QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW }, + { "virtio-tablet-ccw", QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b4852e5..3f3c29f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -445,6 +445,11 @@ typedef enum { QEMU_CAPS_MACHINE_PSERIES_MAX_CPU_COMPAT, /* -machine pseries,max-cpu-compat= */ QEMU_CAPS_DUMP_COMPLETED, /* DUMP_COMPLETED event */ QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW, /* -device virtio-gpu-ccw */ + QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW, /* -device virtio-keyboard-ccw */ + + /* 285 */ + QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, /* -device virtio-mouse-ccw */ + QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW, /* -device virtio-tablet-ccw */
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 ba63670..5477e14 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3921,6 +3921,8 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { suffix = "-pci"; + } else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { + suffix = "-ccw"; } else if (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO) { suffix = "-device"; } else { @@ -3932,7 +3934,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def,
switch ((virDomainInputType) dev->type) { case VIR_DOMAIN_INPUT_TYPE_MOUSE: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_MOUSE) || + (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-mouse is not supported by this QEMU binary")); goto error; @@ -3940,7 +3944,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, virBufferAsprintf(&buf, "virtio-mouse%s,id=%s", suffix, dev->info.alias); break; case VIR_DOMAIN_INPUT_TYPE_TABLET: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_TABLET) || + (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-tablet is not supported by this QEMU binary")); goto error; @@ -3948,7 +3954,9 @@ qemuBuildVirtioInputDevStr(const virDomainDef *def, virBufferAsprintf(&buf, "virtio-tablet%s,id=%s", suffix, dev->info.alias); break; case VIR_DOMAIN_INPUT_TYPE_KBD: - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_KEYBOARD) || + (dev->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-keyboard is not supported by this QEMU binary")); goto error; diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index ec5c396..fda5c36 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -147,6 +147,9 @@ <flag name='iscsi.password-secret'/> <flag name='dump-completed'/> <flag name='virtio-gpu-ccw'/> + <flag name='virtio-keyboard-ccw'/> + <flag name='virtio-mouse-ccw'/> + <flag name='virtio-tablet-ccw'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342885</microcodeVersion>

Test for virtio-{keyboard, mouse, tablet}-ccw. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/input-virtio-ccw.args | 26 +++++++++++++++++++ tests/qemuxml2argvdata/input-virtio-ccw.xml | 29 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 8 ++++++ tests/qemuxml2xmloutdata/input-virtio-ccw.xml | 36 +++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 9 +++++++ 5 files changed, 108 insertions(+) create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.args create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/input-virtio-ccw.xml diff --git a/tests/qemuxml2argvdata/input-virtio-ccw.args b/tests/qemuxml2argvdata/input-virtio-ccw.args new file mode 100644 index 0000000..6ee318c --- /dev/null +++ b/tests/qemuxml2argvdata/input-virtio-ccw.args @@ -0,0 +1,26 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-M s390-ccw-virtio \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1803 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \ +-device virtio-blk-ccw,devno=fe.0.0000,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-keyboard-ccw,id=input0,devno=fe.0.0002 \ +-device virtio-mouse-ccw,id=input1,devno=fe.0.0003 \ +-device virtio-tablet-ccw,id=input2,devno=fe.0.0004 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001 diff --git a/tests/qemuxml2argvdata/input-virtio-ccw.xml b/tests/qemuxml2argvdata/input-virtio-ccw.xml new file mode 100644 index 0000000..a971662 --- /dev/null +++ b/tests/qemuxml2argvdata/input-virtio-ccw.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1803</uuid> + <memory>219136</memory> + <currentMemory>219136</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0'/> + </disk> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x1'/> + </memballoon> + <input type='keyboard' bus='virtio'/> + <input type='mouse' bus='virtio'/> + <input type='tablet' bus='virtio'/> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 59e5bda..9c1d3b5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2973,6 +2973,14 @@ mymain(void) QEMU_CAPS_VNC, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW); + DO_TEST("input-virtio-ccw", QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW, + QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, + QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); diff --git a/tests/qemuxml2xmloutdata/input-virtio-ccw.xml b/tests/qemuxml2xmloutdata/input-virtio-ccw.xml new file mode 100644 index 0000000..693472b --- /dev/null +++ b/tests/qemuxml2xmloutdata/input-virtio-ccw.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1803</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </disk> + <input type='keyboard' bus='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0002'/> + </input> + <input type='mouse' bus='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0003'/> + </input> + <input type='tablet' bus='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0004'/> + </input> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0001'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 766f143..61abf66 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1358,6 +1358,15 @@ mymain(void) DO_TEST("user-aliases", NONE); + DO_TEST("input-virtio-ccw", + QEMU_CAPS_VIRTIO_CCW, + QEMU_CAPS_VIRTIO_KEYBOARD, + QEMU_CAPS_VIRTIO_MOUSE, + QEMU_CAPS_VIRTIO_TABLET, + QEMU_CAPS_DEVICE_VIRTIO_KEYBOARD_CCW, + QEMU_CAPS_DEVICE_VIRTIO_MOUSE_CCW, + QEMU_CAPS_DEVICE_VIRTIO_TABLET_CCW); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakerootdir); -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
Test for virtio-{keyboard, mouse, tablet}-ccw.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/input-virtio-ccw.args | 26 +++++++++++++++++++ tests/qemuxml2argvdata/input-virtio-ccw.xml | 29 +++++++++++++++++++++ tests/qemuxml2argvtest.c | 8 ++++++ tests/qemuxml2xmloutdata/input-virtio-ccw.xml | 36 +++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 9 +++++++ 5 files changed, 108 insertions(+) create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.args create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/input-virtio-ccw.xml
Similar to 3/7 - seems reasonable. John

Document support for the virtio-gpu-ccw and virtio-{keyboard, mouse, tablet}-ccw devices. Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index a51ca97..7eb71e3 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,16 @@ <libvirt> <release version="v4.2.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: Provide virtio-gpu-ccw and virtio input ccw support + </summary> + <description> + Support the virtio-gpu-ccw device as a video device and + virtio-{keyboard, mouse, tablet}-ccw devices as input devices + on S390. + </description> + </change> </section> <section title="Improvements"> <change> -- 2.7.4

On 03/08/2018 11:07 AM, Farhan Ali wrote:
Document support for the virtio-gpu-ccw and virtio-{keyboard, mouse, tablet}-ccw devices.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
Yay, Andrea will be happy someone who remembered news.xml ;-)
diff --git a/docs/news.xml b/docs/news.xml index a51ca97..7eb71e3 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,16 @@ <libvirt> <release version="v4.2.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: Provide virtio-gpu-ccw and virtio input ccw support
How about: "Provide ccw address support for graphics and input devices" John
+ </summary> + <description> + Support the virtio-gpu-ccw device as a video device and + virtio-{keyboard, mouse, tablet}-ccw devices as input devices + on S390. + </description> + </change> </section> <section title="Improvements"> <change>

On 03/16/2018 09:40 AM, John Ferlan wrote:
On 03/08/2018 11:07 AM, Farhan Ali wrote:
Document support for the virtio-gpu-ccw and virtio-{keyboard, mouse, tablet}-ccw devices.
Signed-off-by: Farhan Ali <alifm@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- docs/news.xml | 10 ++++++++++ 1 file changed, 10 insertions(+)
Yay, Andrea will be happy someone who remembered news.xml ;-)
I try my best not to forget it ;)
diff --git a/docs/news.xml b/docs/news.xml index a51ca97..7eb71e3 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -35,6 +35,16 @@ <libvirt> <release version="v4.2.0" date="unreleased"> <section title="New features"> + <change> + <summary> + qemu: Provide virtio-gpu-ccw and virtio input ccw support
How about:
"Provide ccw address support for graphics and input devices"
John
Seems reasonable. Will update
+ </summary> + <description> + Support the virtio-gpu-ccw device as a video device and + virtio-{keyboard, mouse, tablet}-ccw devices as input devices + on S390. + </description> + </change> </section> <section title="Improvements"> <change>

A polite ping :) Would appreciate some feedback on these patches. Thanks Farhan On 03/08/2018 11:06 AM, Farhan Ali wrote:
Hi,
This patch series adds Libvirt support for video and input devices for QEMU guests on S390. QEMU v2.11.0 added support for the virtio-gpu-ccw device [1] and virtio-{keyboard, mouse, tablet}-ccw devices [2], which can be used as video and input devices respectively.
Thanks Farhan
[1] https://git.qemu.org/?p=qemu.git;a=commit;h=1f8ad88935f5cb5a2968909e392dbeee... [2] https://git.qemu.org/?p=qemu.git;a=commit;h=3382cf1fabbf722dce931846853dae71...
Farhan Ali (7): qemu: Fix comment for 'qemuValidateDevicePCISlotsChipsets' qemu: Add support for virtio-gpu-ccw device tests: Add test case for virtio-gpu-ccw qemu: Use correct bus type for input devices qemu: Add support for virtio input ccw devices tests: Add test case for virtio input ccw devices news: Update for virtio-gpu-ccw and virtio input ccw devices
docs/formatdomain.html.in | 5 ++ docs/news.xml | 10 +++ src/qemu/qemu_capabilities.c | 13 ++++ src/qemu/qemu_capabilities.h | 6 ++ src/qemu/qemu_command.c | 32 +++++++-- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 12 +++- src/qemu/qemu_process.c | 5 +- .../qemucapabilitiesdata/caps_2.11.0.s390x.replies | 83 +++++++++++++++++++--- tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 6 +- tests/qemuxml2argvdata/input-virtio-ccw.args | 26 +++++++ tests/qemuxml2argvdata/input-virtio-ccw.xml | 29 ++++++++ .../qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml | 18 +++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.args | 25 +++++++ tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml | 35 +++++++++ tests/qemuxml2argvtest.c | 15 ++++ tests/qemuxml2xmloutdata/input-virtio-ccw.xml | 36 ++++++++++ .../video-virtio-gpu-ccw-auto.xml | 34 +++++++++ tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml | 38 ++++++++++ tests/qemuxml2xmltest.c | 23 ++++++ 20 files changed, 434 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.args create mode 100644 tests/qemuxml2argvdata/input-virtio-ccw.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.args create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/input-virtio-ccw.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw-auto.xml create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-ccw.xml
participants (2)
-
Farhan Ali
-
John Ferlan