| Message ID | 20231117143506.1521718-2-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | UI: fix default VC regressions | expand |
On Fri, 17 Nov 2023 at 14:35, <marcandre.lureau@redhat.com> wrote: > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Commit 1bec1cc0d ("ui/console: allow to override the default VC") changed > the behaviour of the "-display none" option, so that it now creates a > QEMU monitor on the terminal. "-display none" should not be tangled up > with whether we create a monitor or a serial terminal; it should purely > and only disable the graphical window. Changing its behaviour like this > breaks command lines which, for example, use semihosting for their > output and don't want a graphical window, as they now get a monitor they > never asked for. > > It also breaks the command line we document for Xen in > docs/system/i386/xen.html: > > $ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel-irqchip=split \ > -display none -chardev stdio,mux=on,id=char0,signal=off -mon char0 \ > -device xen-console,chardev=char0 -drive file=${GUEST_IMAGE},if=xen > > qemu-system-x86_64: cannot use stdio by multiple character devices > qemu-system-x86_64: could not connect serial device to character backend > 'stdio' > > When qemu is compiled without PIXMAN, by default the serials aren't > muxed with the monitor anymore on stdio. The serials are redirected to > "null" instead, and the monitor isn't set up. > > Fixes: commit 1bec1cc0d ("ui/console: allow to override the default VC") > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > system/vl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/system/vl.c b/system/vl.c > index 5af7ced2a1..14bf0cf0bf 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void) > } > } > > - if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) { > + if (nographic) { > if (default_parallel) { > add_device_config(DEV_PARALLEL, "null"); > } This fixes the regression I was seeing with the semihosting use case. I haven't checked the Xen setup. Tested-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On Mon, 2023-11-20 at 12:42 +0000, Peter Maydell wrote: > On Fri, 17 Nov 2023 at 14:35, <marcandre.lureau@redhat.com> wrote: > > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > Commit 1bec1cc0d ("ui/console: allow to override the default VC") > > changed > > the behaviour of the "-display none" option, so that it now creates > > a > > QEMU monitor on the terminal. "-display none" should not be tangled > > up > > with whether we create a monitor or a serial terminal; it should > > purely > > and only disable the graphical window. Changing its behaviour like > > this > > breaks command lines which, for example, use semihosting for their > > output and don't want a graphical window, as they now get a monitor > > they > > never asked for. > > > > It also breaks the command line we document for Xen in > > docs/system/i386/xen.html: > > > > $ ./qemu-system-x86_64 --accel kvm,xen-version=0x40011,kernel- > > irqchip=split \ > > -display none -chardev stdio,mux=on,id=char0,signal=off -mon > > char0 \ > > -device xen-console,chardev=char0 -drive > > file=${GUEST_IMAGE},if=xen > > > > qemu-system-x86_64: cannot use stdio by multiple character devices > > qemu-system-x86_64: could not connect serial device to character > > backend > > 'stdio' > > > > When qemu is compiled without PIXMAN, by default the serials aren't > > muxed with the monitor anymore on stdio. The serials are redirected > > to > > "null" instead, and the monitor isn't set up. > > > > Fixes: commit 1bec1cc0d ("ui/console: allow to override the default > > VC") > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > system/vl.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/system/vl.c b/system/vl.c > > index 5af7ced2a1..14bf0cf0bf 100644 > > --- a/system/vl.c > > +++ b/system/vl.c > > @@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void) > > } > > } > > > > - if (nographic || (!vc && !is_daemonized() && > > isatty(STDOUT_FILENO))) { > > + if (nographic) { > > if (default_parallel) { > > add_device_config(DEV_PARALLEL, "null"); > > } > > This fixes the regression I was seeing with the semihosting > use case. I haven't checked the Xen setup. Should probably work, but we want the better fix for Xen anyway: https://lore.kernel.org/qemu-devel/20231115172723.1161679-3-dwmw2@infradead.org/
On Mon, 2023-11-20 at 12:42 +0000, Peter Maydell wrote: > > This fixes the regression I was seeing with the semihosting > use case. I haven't checked the Xen setup. > > Tested-by: Peter Maydell <peter.maydell@linaro.org> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> It does also work for the Xen command line (with my other fix reverted). Tested-by: David Woodhouse <dwmw@amazon.co.uk> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
diff --git a/system/vl.c b/system/vl.c index 5af7ced2a1..14bf0cf0bf 100644 --- a/system/vl.c +++ b/system/vl.c @@ -1391,7 +1391,7 @@ static void qemu_create_default_devices(void) } } - if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) { + if (nographic) { if (default_parallel) { add_device_config(DEV_PARALLEL, "null"); }