On 25.04.2018 11:50, David Hildenbrand wrote: > On 25.04.2018 07:21, Thomas Huth wrote: >> The consoles ("sclpconsole" and "sclplmconsole") can only be configured >> with "-device" and "-chardev" so far. Other machines use the convenience >> option "-serial" to configure the default consoles, even for virtual >> consoles like spapr-vty on the pseries machine. So let's support this >> option on s390x, too. This way we can easily enable the serial console >> here again with "-nodefaults", for example: >> >> qemu-system-s390x -no-shutdown -nographic -nodefaults -serial mon:stdio >> >> ... which is way shorter than typing: >> >> qemu-system-s390x -no-shutdown -nographic -nodefaults \ >> -chardev stdio,id=c1,mux=on -device sclpconsole,chardev=c1 \ >> -mon chardev=c1 >> >> The -serial parameter can also be used if you only want to see the QEMU >> monitor on stdio without using -nodefaults, but not the console output. >> That's something that is pretty impossible with the current code today: >> >> qemu-system-s390x -no-shutdown -nographic -serial none >> >> While we're at it, this patch also maps the second -serial option to the >> "sclplmconsole", so that there is now an easy way to configure this second >> console on s390x, too, for example: >> >> qemu-system-s390x -no-shutdown -nographic -serial null -serial mon:stdio >> >> Additionally, the new code is also smaller than the old one and we have >> less s390x-specific code in vl.c :-) >> >> I've also checked that migration still works as expected by migrating >> a guest with console output back and forth between a qemu-system-s390x >> that has this patch and an instance without this patch. >> >> Signed-off-by: Thomas Huth <th...@redhat.com> >> --- >> RFC -> v1: >> - Improved the patch description (provided examples) >> - Moved the "init consoles" hunk in ccw_init to a later point in time >> so that the output of "info qom-tree" shows the same order of devices >> in the "/machine/unattached" tree. >> >> hw/s390x/event-facility.c | 14 +++++++++++ >> hw/s390x/s390-virtio-ccw.c | 19 +++++++++++++-- >> include/hw/boards.h | 1 - >> include/hw/s390x/event-facility.h | 2 ++ >> vl.c | 50 >> --------------------------------------- >> 5 files changed, 33 insertions(+), 53 deletions(-) >> >> diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c >> index 9c24bc6..e6940a2 100644 >> --- a/hw/s390x/event-facility.c >> +++ b/hw/s390x/event-facility.c >> @@ -511,3 +511,17 @@ static void register_types(void) >> } >> >> type_init(register_types) >> + >> +BusState *sclp_get_event_facility_bus(void) >> +{ >> + Object *busobj; >> + SCLPEventsBus *sbus; >> + >> + busobj = object_resolve_path_type("", TYPE_SCLP_EVENTS_BUS, NULL); >> + sbus = OBJECT_CHECK(SCLPEventsBus, busobj, TYPE_SCLP_EVENTS_BUS); >> + if (!sbus) { >> + return NULL; >> + } >> + >> + return &sbus->qbus; >> +} >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index 435f7c9..62d909e 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -288,6 +288,15 @@ static void s390_create_virtio_net(BusState *bus, const >> char *name) >> } >> } >> >> +static void s390_create_sclpconsole(const char *type, Chardev *chardev) >> +{ >> + DeviceState *dev; >> + >> + dev = qdev_create(sclp_get_event_facility_bus(), type); >> + qdev_prop_set_chr(dev, "chardev", chardev); >> + qdev_init_nofail(dev); >> +} >> + >> static void ccw_init(MachineState *machine) >> { >> int ret; >> @@ -346,6 +355,14 @@ static void ccw_init(MachineState *machine) >> /* Create VirtIO network adapters */ >> s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw"); >> >> + /* init consoles */ >> + if (serial_hds[0]) { >> + s390_create_sclpconsole("sclpconsole", serial_hds[0]); >> + } >> + if (serial_hds[1]) { >> + s390_create_sclpconsole("sclplmconsole", serial_hds[1]); >> + } > > What happens if more -serial are defined? An error? Silently ignored?
Silently ignored, since this is also what almost all other machines are doing (look for serial_hds in hw/ and you'll see what I mean). > (e.g. do we have to redefine MAX_SERIAL_PORTS on s390x or add checking > code here?) As all the other machines are also not redefining MAX_SERIAL_PORTS, I think we should also not do this on s390x now, should we? Thomas