| Message ID | 1398697233-15102-1-git-send-email-ncmike@ncultra.org |
|---|---|
| State | New |
| Headers | show |
28.04.2014 19:00, Mike Day wrote: > When the deprecated -nographic option is used with the -mon option in > readline mode, qemu will create a second character device for stdio > and place it over the stdio chardev put into place by the -mon > option. This causes the terminal to stop echoeing characters upon exit > from Qemu. > > Fix by checking for the existing chardev before adding another. Gosh. This is uuuuuuugly. Maybe, at least, we can add some variable which gets incremented for each -mon chardev= ? I dunno, that's about the same ugliness. Oh well... :( /mjt
On Mon, Apr 28, 2014 at 3:53 PM, Michael Tokarev <mjt@tls.msk.ru> wrote: > Gosh. This is uuuuuuugly. > > Maybe, at least, we can add some variable which gets incremented for > each -mon chardev= ? I dunno, that's about the same ugliness. Oh > well... :( Suggestions much appreciated, I think its ugly versus intrusive, but I'm not familiar with this part of qemu. There is another case with -monitor where you can configure two chardevs for stdio and it does the same thing. I left that one alone, its more complicated. Mike
[ambru@redhat.com bounces, removed from Cc] 29.04.2014 00:07, Mike Day wrote: [] > Suggestions much appreciated, I think its ugly versus intrusive, but > I'm not familiar with this part of qemu. > > There is another case with -monitor where you can configure two > chardevs for stdio and it does the same thing. I left that one alone, > its more complicated. Actually it looks like we should protect from fighting for stdio from various places. We already had something similar in commit ab51b1d568e02c80b1abf9016bda3a86dc1db389 -- see bits in qemu-char.c. I guess we should have some global variable like "stdio_occuped", set it to 1 when -daemonize is specified, and set and check it each time we try to use stdio for something. This way we'll prevent various parts of qemu from fighting for stdio. I don't have time right now to do that, maybe later today or this week. Thanks! /mjt
On Mo, 2014-04-28 at 11:00 -0400, Mike Day wrote: > When the deprecated -nographic option is used with the -mon option in > readline mode, qemu will create a second character device for stdio > and place it over the stdio chardev put into place by the -mon > option. This causes the terminal to stop echoeing characters upon exit > from Qemu. Welcome to the fun world of default devices. We'd love to get rid of them as they also cause other similar grief, but we can't if we want maintain backward compatibility :( I strongly suggest to simply use '-nodefaults' if default devices get into your way. Defining the serial line the way you want it using '-serial $args' or '-device isa-serial,$args' will work (in that specific case) too. I don't feel like adding more band-aid to paper over the fundamental issue. Too much band-aid tends to cause other unwanted side effects. cheers, Gerd
On Tue, Apr 29, 2014 at 3:09 AM, Gerd Hoffmann <kraxel@redhat.com> wrote: > > I don't feel like adding more band-aid to paper over the fundamental > issue. Too much band-aid tends to cause other unwanted side effects. Fair enough, I tend to agree. thanks for the review. Mike
On Tue, Apr 29, 2014 at 1:02 AM, Michael Tokarev <mjt@tls.msk.ru> wrote: > I guess we should have some global variable like "stdio_occuped", set > it to 1 when -daemonize is specified, and set and check it each time > we try to use stdio for something. This way we'll prevent various > parts of qemu from fighting for stdio. That does seem to be a more fundamental solution. Thanks for the review. Mike
diff --git a/vl.c b/vl.c index 236f95e..3104b20 100644 --- a/vl.c +++ b/vl.c @@ -93,6 +93,7 @@ int main(int argc, char **argv) #include "sysemu/kvm.h" #include "qapi/qmp/qjson.h" #include "qemu/option.h" +#include "qemu/option_int.h" #include "qemu/config-file.h" #include "qemu-options.h" #include "qmp-commands.h" @@ -524,6 +525,20 @@ static QemuOptsList qemu_mem_opts = { }, }; +static int qemu_opts_loopfun(QemuOpts *opts, void *opaque) +{ + QemuOpt *opt; + + if (!strncmp(((QemuOpt *)opts->list)->name, "mon", 3)) { + QTAILQ_FOREACH(opt, &opts->head, next) { + if (!strncmp(opt->name, "chardev", 7)) { + return 1; + } + } + } + return 0; +} + /** * Get machine options * @@ -4113,6 +4128,11 @@ int main(int argc, char **argv, char **envp) } if (display_type == DT_NOGRAPHIC) { + int have_stdio = 0; + QemuOptsList *opts = qemu_find_opts("mon"); + if (opts) { + have_stdio = qemu_opts_foreach(opts, qemu_opts_loopfun, NULL, 0); + } if (default_parallel) add_device_config(DEV_PARALLEL, "null"); if (default_serial && default_monitor) { @@ -4122,7 +4142,7 @@ int main(int argc, char **argv, char **envp) } else if (default_sclp && default_monitor) { add_device_config(DEV_SCLP, "mon:stdio"); } else { - if (default_serial) + if (default_serial && !have_stdio) add_device_config(DEV_SERIAL, "stdio"); if (default_virtcon) add_device_config(DEV_VIRTCON, "stdio");
When the deprecated -nographic option is used with the -mon option in readline mode, qemu will create a second character device for stdio and place it over the stdio chardev put into place by the -mon option. This causes the terminal to stop echoeing characters upon exit from Qemu. Fix by checking for the existing chardev before adding another. Signed-off-by: Mike Day <ncmike@ncultra.org> --- To reproduce, use -mon and -nographic together. I was able to reproduce it using # qemu-system-x86_64 -enable-kvm -m 1G -chardev stdio,id=mon0 \ # -mon chardev=mon0,mode=readline -nographic --- vl.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)