* [Qemu-devel] [PATCH] stop using stdio for monitor/serial/etc with -daemonize
@ 2012-09-25 10:43 Michael Tokarev
2012-09-25 11:23 ` Peter Maydell
0 siblings, 1 reply; 4+ messages in thread
From: Michael Tokarev @ 2012-09-25 10:43 UTC (permalink / raw)
To: qemu-devel; +Cc: Stefan Hajnoczi, Michael Tokarev
Current code binds monitor and serial port to the guest console
unless -nographic is specified, which is okay. But when there's
no guest console (-nographic), the code tries to use stdio for
the same default devices. But it does not check for -daemonize
at the same time -- because when -daemonize is given, there's no
point at using stdin since it will be closed down the line.
However, when serial port is attached to stdin, tty control
modes are changed (switching tty to raw mode), and qemu will
switch to background, leaving the tty in bad state.
Take -daemonize into account too, when assigning default devices,
and for -nographic -daemonize case, assign them to "null" instead.
This is https://bugs.launchpad.net/qemu/+bug/1024275
or http://bugs.debian.org/549195 .
This patch depends on another patch, 995ee2bf469de6,
"curses: don't initialize curses when qemu is daemonized",
by Hitoshi Mitake, which creates is_daemonized() routine.
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
vl.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/vl.c b/vl.c
index 48049ef..ae1794f 100644
--- a/vl.c
+++ b/vl.c
@@ -3395,17 +3395,26 @@ int main(int argc, char **argv, char **envp)
if (display_type == DT_NOGRAPHIC) {
if (default_parallel)
add_device_config(DEV_PARALLEL, "null");
- if (default_serial && default_monitor) {
- add_device_config(DEV_SERIAL, "mon:stdio");
- } else if (default_virtcon && default_monitor) {
- add_device_config(DEV_VIRTCON, "mon:stdio");
+ if (!is_daemonized()) {
+ if (default_serial && default_monitor) {
+ add_device_config(DEV_SERIAL, "mon:stdio");
+ } else if (default_virtcon && default_monitor) {
+ add_device_config(DEV_VIRTCON, "mon:stdio");
+ } else {
+ if (default_serial)
+ add_device_config(DEV_SERIAL, "stdio");
+ if (default_virtcon)
+ add_device_config(DEV_VIRTCON, "stdio");
+ if (default_monitor)
+ monitor_parse("stdio", "readline");
+ }
} else {
if (default_serial)
- add_device_config(DEV_SERIAL, "stdio");
+ add_device_config(DEV_SERIAL, "null");
if (default_virtcon)
- add_device_config(DEV_VIRTCON, "stdio");
+ add_device_config(DEV_VIRTCON, "null");
if (default_monitor)
- monitor_parse("stdio", "readline");
+ monitor_parse("stdio", "null");
}
} else {
if (default_serial)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] stop using stdio for monitor/serial/etc with -daemonize
2012-09-25 10:43 [Qemu-devel] [PATCH] stop using stdio for monitor/serial/etc with -daemonize Michael Tokarev
@ 2012-09-25 11:23 ` Peter Maydell
2012-09-25 11:53 ` Michael Tokarev
0 siblings, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2012-09-25 11:23 UTC (permalink / raw)
To: Michael Tokarev; +Cc: Stefan Hajnoczi, qemu-devel
On 25 September 2012 11:43, Michael Tokarev <mjt@tls.msk.ru> wrote:
> --- a/vl.c
> +++ b/vl.c
> @@ -3395,17 +3395,26 @@ int main(int argc, char **argv, char **envp)
> if (display_type == DT_NOGRAPHIC) {
> if (default_parallel)
> add_device_config(DEV_PARALLEL, "null");
> - if (default_serial && default_monitor) {
> - add_device_config(DEV_SERIAL, "mon:stdio");
> - } else if (default_virtcon && default_monitor) {
> - add_device_config(DEV_VIRTCON, "mon:stdio");
> + if (!is_daemonized()) {
> + if (default_serial && default_monitor) {
> + add_device_config(DEV_SERIAL, "mon:stdio");
> + } else if (default_virtcon && default_monitor) {
> + add_device_config(DEV_VIRTCON, "mon:stdio");
> + } else {
> + if (default_serial)
> + add_device_config(DEV_SERIAL, "stdio");
> + if (default_virtcon)
> + add_device_config(DEV_VIRTCON, "stdio");
> + if (default_monitor)
> + monitor_parse("stdio", "readline");
> + }
Braces :-)
> } else {
> if (default_serial)
> - add_device_config(DEV_SERIAL, "stdio");
> + add_device_config(DEV_SERIAL, "null");
> if (default_virtcon)
> - add_device_config(DEV_VIRTCON, "stdio");
> + add_device_config(DEV_VIRTCON, "null");
> if (default_monitor)
> - monitor_parse("stdio", "readline");
> + monitor_parse("stdio", "null");
monitor_parse()'s second argument is the mode, which I think can
only be "readline" or "control" -- should this be "null", "readline"
instead?
Would it be possible to condense this chain of ifs down a bit
by having a variable which gets appropriately set to
"stdio", "null" or "vc:80Cx24C" and then used in a single
set of add_device_config() calls?
-- PMM
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] stop using stdio for monitor/serial/etc with -daemonize
2012-09-25 11:23 ` Peter Maydell
@ 2012-09-25 11:53 ` Michael Tokarev
2012-09-25 13:49 ` Michael Tokarev
0 siblings, 1 reply; 4+ messages in thread
From: Michael Tokarev @ 2012-09-25 11:53 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Hajnoczi, qemu-devel
On 25.09.2012 15:23, Peter Maydell wrote:
> On 25 September 2012 11:43, Michael Tokarev <mjt@tls.msk.ru> wrote:
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -3395,17 +3395,26 @@ int main(int argc, char **argv, char **envp)
>> if (display_type == DT_NOGRAPHIC) {
>> if (default_parallel)
>> add_device_config(DEV_PARALLEL, "null");
>> - if (default_serial && default_monitor) {
>> - add_device_config(DEV_SERIAL, "mon:stdio");
>> - } else if (default_virtcon && default_monitor) {
>> - add_device_config(DEV_VIRTCON, "mon:stdio");
>> + if (!is_daemonized()) {
>> + if (default_serial && default_monitor) {
>> + add_device_config(DEV_SERIAL, "mon:stdio");
>> + } else if (default_virtcon && default_monitor) {
>> + add_device_config(DEV_VIRTCON, "mon:stdio");
>> + } else {
>> + if (default_serial)
>> + add_device_config(DEV_SERIAL, "stdio");
>> + if (default_virtcon)
>> + add_device_config(DEV_VIRTCON, "stdio");
>> + if (default_monitor)
>> + monitor_parse("stdio", "readline");
>> + }
>
> Braces :-)
That's the original code. N/p, can add braces. It will make the
code unnecessary bigger but who cares.
>> - monitor_parse("stdio", "readline");
>> + monitor_parse("stdio", "null");
>
> monitor_parse()'s second argument is the mode, which I think can
> only be "readline" or "control" -- should this be "null", "readline"
> instead?
Actually it is a typo -- I corrected it in my working tree but forgot
to commit before git-format-patch.
> Would it be possible to condense this chain of ifs down a bit
> by having a variable which gets appropriately set to
> "stdio", "null" or "vc:80Cx24C" and then used in a single
> set of add_device_config() calls?
I think it can be, yes. Will try again :)
/mjt
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] stop using stdio for monitor/serial/etc with -daemonize
2012-09-25 11:53 ` Michael Tokarev
@ 2012-09-25 13:49 ` Michael Tokarev
0 siblings, 0 replies; 4+ messages in thread
From: Michael Tokarev @ 2012-09-25 13:49 UTC (permalink / raw)
To: Peter Maydell; +Cc: Stefan Hajnoczi, qemu-devel
On 25.09.2012 15:53, Michael Tokarev wrote:
> On 25.09.2012 15:23, Peter Maydell wrote:
[]
>> Would it be possible to condense this chain of ifs down a bit
>> by having a variable which gets appropriately set to
>> "stdio", "null" or "vc:80Cx24C" and then used in a single
>> set of add_device_config() calls?
>
> I think it can be, yes. Will try again :)
I don't think it is better (just an example, it doesn't quite work yet):
/* Create default monitor, serial, parallel and virtcon devices. */
/* reuse optarg variable */
optarg = NULL;
if (display_type != DT_NOGRAPHIC) {
/* regular case, all devices directed to the guest console */
optarg = "vc:80Cx24C";
} else if (is_daemonized()) {
/* nographic and daemonize, everything goes to null */
optarg = "null";
} else { /* -nographic and no -daemonize */
/* can't have both serial and virtcon on stdio */
if (default_serial && default_monitor) {
add_device_config(DEV_SERIAL, "mon:stdio");
} else if (default_virtcon && default_monitor) {
add_device_config(DEV_VIRTCON, "mon:stdio");
} else {
optarg = "stdio";
}
}
if (optarg && default_serial) {
add_device_config(DEV_SERIAL, optarg);
}
if (default_parallel) {
add_device_config(DEV_PARALLEL, display_type != DT_NOGRAPHIC ? "null" : optarg);
}
if (optarg && default_monitor) {
monitor_parse(optarg, "readline");
}
if (optarg && default_virtcon) {
add_device_config(DEV_VIRTCON, optarg);
}
/mjt
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-25 13:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-25 10:43 [Qemu-devel] [PATCH] stop using stdio for monitor/serial/etc with -daemonize Michael Tokarev
2012-09-25 11:23 ` Peter Maydell
2012-09-25 11:53 ` Michael Tokarev
2012-09-25 13:49 ` Michael Tokarev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).