| Message ID | 1455023713-104799-3-git-send-email-silbe@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sascha Silbe <silbe@linux.vnet.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 virtio devices that are implemented on all > architectures supporting virtio. > > For virtio-balloon, only the CCW implementation was missing. > > Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> > --- > > This leaves out > virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because > they're currently only implemented using PCI, so there's no immediate > value in having them. It would nevertheless make sense to include them > so they can get used already and will start to Just Work™ on s390x > once a CCW implementation appears. I can post the corresponding patch > if there's any interest. I guess that's for the virtio people to decide. I'm cc'ing some. > --- > qdev-monitor.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qdev-monitor.c b/qdev-monitor.c > index 0145deb..9c4217c 100644 > --- a/qdev-monitor.c > +++ b/qdev-monitor.c > @@ -44,12 +44,19 @@ 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-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 }, > { }
On Thu, 11 Feb 2016 10:01:35 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Sascha Silbe <silbe@linux.vnet.ibm.com> writes: > > This leaves out > > virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because > > they're currently only implemented using PCI, so there's no immediate > > value in having them. It would nevertheless make sense to include them > > so they can get used already and will start to Just Work™ on s390x > > once a CCW implementation appears. I can post the corresponding patch > > if there's any interest. > > I guess that's for the virtio people to decide. I'm cc'ing some. What would the error look like if one decided to use e.g. virtio-gpu on s390x? If the error is more specific (i.e. virtio-gpu-ccw does not exist vs. virtio-gpu does not exist), I think adding the aliases makes sense: the user sees what is actually missing.
Dear Conny, Cornelia Huck <cornelia.huck@de.ibm.com> writes: > On Thu, 11 Feb 2016 10:01:35 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> Sascha Silbe <silbe@linux.vnet.ibm.com> writes: > >> > This leaves out >> > virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because >> > they're currently only implemented using PCI, so there's no immediate >> > value in having them. It would nevertheless make sense to include them >> > so they can get used already and will start to Just Work™ on s390x >> > once a CCW implementation appears. I can post the corresponding patch >> > if there's any interest. >> >> I guess that's for the virtio people to decide. I'm cc'ing some. > > What would the error look like if one decided to use e.g. virtio-gpu on > s390x? If the error is more specific (i.e. virtio-gpu-ccw does not > exist vs. virtio-gpu does not exist), I think adding the aliases makes > sense: the user sees what is actually missing. Interesting point. Indeed, if we already define the matching -ccw aliases, the error message may be slightly more useful: silbe@oc4731375738:~/recoverable/qemu$ s390x-softmmu/qemu-system-s390x -device virtio-gpu qemu-system-s390x: -device virtio-gpu: 'virtio-gpu-ccw' is not a valid device model name Though we should probably at least add a comment to the alias list mentioning that this is intentional. We might even want to adjust qdev_get_device_class() to print a more specific error message in this case. Sascha
On Thu, 11 Feb 2016 17:18:32 +0100 Sascha Silbe <silbe@linux.vnet.ibm.com> wrote: > Cornelia Huck <cornelia.huck@de.ibm.com> writes: > > > On Thu, 11 Feb 2016 10:01:35 +0100 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Sascha Silbe <silbe@linux.vnet.ibm.com> writes: > > > >> > This leaves out > >> > virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because > >> > they're currently only implemented using PCI, so there's no immediate > >> > value in having them. It would nevertheless make sense to include them > >> > so they can get used already and will start to Just Work™ on s390x > >> > once a CCW implementation appears. I can post the corresponding patch > >> > if there's any interest. [For laughs and giggles, I have wired up all of these devices for ccw. Excluding input-host (for which I did not have a suitable evdev), I can specify the various devices on the commandline and get some devices in the guest which do... nothing :) I won't pursue this further for now, as I currently don't have a convincing use case beyond "because we can".] > >> > >> I guess that's for the virtio people to decide. I'm cc'ing some. > > > > What would the error look like if one decided to use e.g. virtio-gpu on > > s390x? If the error is more specific (i.e. virtio-gpu-ccw does not > > exist vs. virtio-gpu does not exist), I think adding the aliases makes > > sense: the user sees what is actually missing. > > Interesting point. Indeed, if we already define the matching -ccw > aliases, the error message may be slightly more useful: > > silbe@oc4731375738:~/recoverable/qemu$ s390x-softmmu/qemu-system-s390x -device virtio-gpu > qemu-system-s390x: -device virtio-gpu: 'virtio-gpu-ccw' is not a valid device model name > > Though we should probably at least add a comment to the alias list > mentioning that this is intentional. We might even want to adjust > qdev_get_device_class() to print a more specific error message in this > case. qemu-system-s390x: -device virtio-gpu: 'virtio-gpu' (alias 'virtio-gpu-ccw') is not a valid device model name would make it obvious that some alias expansion had been going on. I think that would be useful.
Dear Conny,
Cornelia Huck <cornelia.huck@de.ibm.com> writes:
[virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet}]
> [For laughs and giggles, I have wired up all of these devices for ccw.
> Excluding input-host (for which I did not have a suitable evdev), I can
> specify the various devices on the commandline and get some devices in
> the guest which do... nothing :) I won't pursue this further for now,
> as I currently don't have a convincing use case beyond "because we
> can".]
Care to share the patch?
> qemu-system-s390x: -device virtio-gpu: 'virtio-gpu' (alias 'virtio-gpu-ccw') is not a valid device model name
>
> would make it obvious that some alias expansion had been going on. I
> think that would be useful.
Good idea. Will include a patch for this in the next version.
Sascha
diff --git a/qdev-monitor.c b/qdev-monitor.c index 0145deb..9c4217c 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -44,12 +44,19 @@ 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-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-{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 virtio devices that are implemented on all architectures supporting virtio. For virtio-balloon, only the CCW implementation was missing. Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com> --- This leaves out virtio-{gpu,input,input-hid,input-host,keyboard,mouse,tablet} because they're currently only implemented using PCI, so there's no immediate value in having them. It would nevertheless make sense to include them so they can get used already and will start to Just Work™ on s390x once a CCW implementation appears. I can post the corresponding patch if there's any interest. --- qdev-monitor.c | 7 +++++++ 1 file changed, 7 insertions(+)