[Qemu-devel] [PATCH v2 0/3] add missing virtio aliases

[Qemu-devel] [PATCH v2 0/3] add missing virtio aliases

[Sascha Silbe]

Split out from the series fixing qemu-iotest 140 on s390x [1].

This series makes crafting qemu command lines that work across
multiple (virtio-supporting) platforms at lot easier.

Changes since the original series:
- included patch to improve the error message when the target device
  class of an alias doesn't exist
- added comment about keeping alias table sorted
- covering all non-abstract virtio device classes rather than just
  those already implemented on s390x

Sascha Silbe (3):
  qdev-monitor: improve error message when alias device is unavailable
  qdev-monitor: sort alias table by typename
  qdev-monitor: add missing aliases for virtio device classes

 qdev-monitor.c | 42 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 8 deletions(-)

[1] mid:1455023713-104799-1-git-send-email-silbe@linux.vnet.ibm.com
    "[PATCH 0/3] qemu-iotests: fix 140 on s390x" by Sascha Silbe
    <silbe@linux.vnet.ibm.com> on 2016-02-09
-- 
2.1.4

[Qemu-devel] [PATCH v2 1/3] qdev-monitor: improve error message when alias device is unavailable

[Sascha Silbe]

When trying to instantiate an alias that points to a device class that
doesn't exist, the error message looks like qemu misunderstood the
request:

$ s390x-softmmu/qemu-system-s390x -device virtio-gpu
qemu-system-s390x: -device virtio-gpu: 'virtio-gpu-ccw' is not a valid
device model name

Special-case the error message to make it explicit that alias
expansion is going on:

$ s390x-softmmu/qemu-system-s390x -device virtio-gpu
qemu-system-s390x: -device virtio-gpu: 'virtio-gpu' (alias
'virtio-gpu-ccw') is not a valid device model name

Suggested-By: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
v1->v2: new patch
---
 qdev-monitor.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 81e3ff3..e5136d7 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -188,6 +188,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
 {
     ObjectClass *oc;
     DeviceClass *dc;
+    const char *original_name = *driver;
 
     oc = object_class_by_name(*driver);
     if (!oc) {
@@ -200,7 +201,12 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
     }
 
     if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) {
-        error_setg(errp, "'%s' is not a valid device model name", *driver);
+        if (*driver != original_name) {
+            error_setg(errp, "'%s' (alias '%s') is not a valid device model"
+                       " name", original_name, *driver);
+        } else {
+            error_setg(errp, "'%s' is not a valid device model name", *driver);
+        }
         return NULL;
     }
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 1/3] qdev-monitor: improve error message when alias device is unavailable

[Cornelia Huck]

On Thu, 18 Feb 2016 22:44:12 +0100
Sascha Silbe <silbe@linux.vnet.ibm.com> wrote:

> When trying to instantiate an alias that points to a device class that
> doesn't exist, the error message looks like qemu misunderstood the
> request:
> 
> $ s390x-softmmu/qemu-system-s390x -device virtio-gpu
> qemu-system-s390x: -device virtio-gpu: 'virtio-gpu-ccw' is not a valid
> device model name
> 
> Special-case the error message to make it explicit that alias
> expansion is going on:
> 
> $ s390x-softmmu/qemu-system-s390x -device virtio-gpu
> qemu-system-s390x: -device virtio-gpu: 'virtio-gpu' (alias
> 'virtio-gpu-ccw') is not a valid device model name
> 
> Suggested-By: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> v1->v2: new patch
> ---
>  qdev-monitor.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

[Qemu-devel] [PATCH v2 2/3] qdev-monitor: sort alias table by typename

[Sascha Silbe]

Sort the alias table by typename so it's easier to see which aliases
exist.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
v1->v2: Added comment asking devs to keep table sorted
---
 qdev-monitor.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index e5136d7..4e3681c 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -39,19 +39,20 @@ typedef struct QDevAlias
     uint32_t arch_mask;
 } QDevAlias;
 
+/* Please keep this table sorted by typename. */
 static const QDevAlias qdev_alias_table[] = {
-    { "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-    { "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-    { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "e1000", "e1000-82540em" },
+    { "ich9-ahci", "ahci" },
+    { "kvm-pci-assign", "pci-assign" },
+    { "lsi53c895a", "lsi" },
     { "virtio-balloon-pci", "virtio-balloon",
             QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
+    { "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
+    { "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
-    { "lsi53c895a", "lsi" },
-    { "ich9-ahci", "ahci" },
-    { "kvm-pci-assign", "pci-assign" },
-    { "e1000", "e1000-82540em" },
+    { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { }
 };
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 2/3] qdev-monitor: sort alias table by typename

[Cornelia Huck]

On Thu, 18 Feb 2016 22:44:13 +0100
Sascha Silbe <silbe@linux.vnet.ibm.com> wrote:

> Sort the alias table by typename so it's easier to see which aliases
> exist.
> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> v1->v2: Added comment asking devs to keep table sorted
> ---
>  qdev-monitor.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

[Qemu-devel] [PATCH v2 3/3] qdev-monitor: add missing aliases for virtio device classes

[Sascha Silbe]

virtio-{blk,balloon,net,serial} are aliases for their actual,
architecture-dependent implementations (*-ccw on s390x, *-pci on other
architectures supporting virtio). This makes it a lot easier to craft
qemu invocations that work on all supported architectures. Complete
the set to cover all existing non-abstract virtio device classes.

For virtio-balloon, only the CCW implementation was missing.

Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
v1->v2:
- Completely wired up all (non-abstract) virtio device classes, even
  those that still lack a CCW implementation.

Markus, does your Reviewed-by apply to this version as well? (Wasn't
sure so I didn't include it)
---
 qdev-monitor.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 4e3681c..be6a07e 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -45,14 +45,33 @@ static const QDevAlias qdev_alias_table[] = {
     { "ich9-ahci", "ahci" },
     { "kvm-pci-assign", "pci-assign" },
     { "lsi53c895a", "lsi" },
+    { "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
+    { "virtio-9p-pci", "virtio-9p", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_S390X },
     { "virtio-balloon-pci", "virtio-balloon",
             QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
     { "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_S390X },
+    { "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
+    { "virtio-input-host-pci", "virtio-input-host",
+            QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
+    { "virtio-keyboard-pci", "virtio-keyboard",
+            QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_S390X },
+    { "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
     { "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_S390X },
+    { "virtio-rng-pci", "virtio-rng", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_S390X },
+    { "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
     { "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+    { "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
+    { "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
     { }
 };
 
-- 
2.1.4

Re: [Qemu-devel] [PATCH v2 3/3] qdev-monitor: add missing aliases for virtio device classes

[Cornelia Huck]

On Thu, 18 Feb 2016 22:44:14 +0100
Sascha Silbe <silbe@linux.vnet.ibm.com> wrote:

> virtio-{blk,balloon,net,serial} are aliases for their actual,
> architecture-dependent implementations (*-ccw on s390x, *-pci on other
> architectures supporting virtio). This makes it a lot easier to craft
> qemu invocations that work on all supported architectures. Complete
> the set to cover all existing non-abstract virtio device classes.
> 
> For virtio-balloon, only the CCW implementation was missing.

There's also virtio-vga but that one looks quite hardcoded around pci.

> 
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> v1->v2:
> - Completely wired up all (non-abstract) virtio device classes, even
>   those that still lack a CCW implementation.
> 
> Markus, does your Reviewed-by apply to this version as well? (Wasn't
> sure so I didn't include it)
> ---
>  qdev-monitor.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Re: [Qemu-devel] [PATCH v2 3/3] qdev-monitor: add missing aliases for virtio device classes

[Sascha Silbe]

Dear Conny,

Cornelia Huck <cornelia.huck@de.ibm.com> writes:

>> virtio-{blk,balloon,net,serial} are aliases for their actual,
>> architecture-dependent implementations (*-ccw on s390x, *-pci on other
>> architectures supporting virtio). This makes it a lot easier to craft
>> qemu invocations that work on all supported architectures. Complete
>> the set to cover all existing non-abstract virtio device classes.
[...]
> There's also virtio-vga but that one looks quite hardcoded around pci.

Yeah, it's not even called virtio-vga-pci but rather just virtio-vga, so
we can't do any virtio-foo → virtio-foo-{pci,ccw} alias magic for it.


[...]
> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Thanks for the reviews!

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-devel] [PATCH v2 0/3] add missing virtio aliases

[Sascha Silbe]

Sascha Silbe <silbe@linux.vnet.ibm.com> writes:

[...]
> This series makes crafting qemu command lines that work across
> multiple (virtio-supporting) platforms at lot easier.
[...]

Ping. Conny has already given her Reviewed-By; Markus gave it for a
previous version of the third patch. Are there any other opinions on
this series?

It was posted in mid-February (i.e. before soft freeze) and I'd like it
to land in qemu 2.6.

Sascha
-- 
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641

Re: [Qemu-devel] [PATCH v2 0/3] add missing virtio aliases

[Michael S. Tsirkin]

On Thu, Feb 18, 2016 at 10:44:11PM +0100, Sascha Silbe wrote:
> Split out from the series fixing qemu-iotest 140 on s390x [1].
> 
> This series makes crafting qemu command lines that work across
> multiple (virtio-supporting) platforms at lot easier.
> 
> Changes since the original series:
> - included patch to improve the error message when the target device
>   class of an alias doesn't exist
> - added comment about keeping alias table sorted
> - covering all non-abstract virtio device classes rather than just
>   those already implemented on s390x
> 
> Sascha Silbe (3):
>   qdev-monitor: improve error message when alias device is unavailable
>   qdev-monitor: sort alias table by typename
>   qdev-monitor: add missing aliases for virtio device classes
> 
>  qdev-monitor.c | 42 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> [1] mid:1455023713-104799-1-git-send-email-silbe@linux.vnet.ibm.com
>     "[PATCH 0/3] qemu-iotests: fix 140 on s390x" by Sascha Silbe
>     <silbe@linux.vnet.ibm.com> on 2016-02-09

I don't see issues here:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

You should also Cc lcapitulino@redhat.com and eblake@redhat.com.
Markus, will you take this series?

> -- 
> 2.1.4

Re: [Qemu-devel] [PATCH v2 0/3] add missing virtio aliases

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Feb 18, 2016 at 10:44:11PM +0100, Sascha Silbe wrote:
>> Split out from the series fixing qemu-iotest 140 on s390x [1].
>> 
>> This series makes crafting qemu command lines that work across
>> multiple (virtio-supporting) platforms at lot easier.
>> 
>> Changes since the original series:
>> - included patch to improve the error message when the target device
>>   class of an alias doesn't exist
>> - added comment about keeping alias table sorted
>> - covering all non-abstract virtio device classes rather than just
>>   those already implemented on s390x
>> 
>> Sascha Silbe (3):
>>   qdev-monitor: improve error message when alias device is unavailable
>>   qdev-monitor: sort alias table by typename
>>   qdev-monitor: add missing aliases for virtio device classes
>> 
>>  qdev-monitor.c | 42 ++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 34 insertions(+), 8 deletions(-)
>> 
>> [1] mid:1455023713-104799-1-git-send-email-silbe@linux.vnet.ibm.com
>>     "[PATCH 0/3] qemu-iotests: fix 140 on s390x" by Sascha Silbe
>>     <silbe@linux.vnet.ibm.com> on 2016-02-09
>
> I don't see issues here:
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
>
> You should also Cc lcapitulino@redhat.com and eblake@redhat.com.

Cc'ing them now.

> Markus, will you take this series?

qdev-monitor falls in the crack between monitor (maintained by me) and
qdev (no maintainer, which is a bloody shame).  I can take it through my
tree.

Re: [Qemu-devel] [PATCH v2 0/3] add missing virtio aliases

[Markus Armbruster]

Sascha Silbe <silbe@linux.vnet.ibm.com> writes:

> Split out from the series fixing qemu-iotest 140 on s390x [1].
>
> This series makes crafting qemu command lines that work across
> multiple (virtio-supporting) platforms at lot easier.
>
> Changes since the original series:
> - included patch to improve the error message when the target device
>   class of an alias doesn't exist
> - added comment about keeping alias table sorted
> - covering all non-abstract virtio device classes rather than just
>   those already implemented on s390x
>
> Sascha Silbe (3):
>   qdev-monitor: improve error message when alias device is unavailable
>   qdev-monitor: sort alias table by typename
>   qdev-monitor: add missing aliases for virtio device classes
>
>  qdev-monitor.c | 42 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 8 deletions(-)
>
> [1] mid:1455023713-104799-1-git-send-email-silbe@linux.vnet.ibm.com
>     "[PATCH 0/3] qemu-iotests: fix 140 on s390x" by Sascha Silbe
>     <silbe@linux.vnet.ibm.com> on 2016-02-09

Applied to my monitor-next branch, will hold the queue for a couple of
days to give others a chance to review, thanks!