1. Patchew
  2. QEMU
  3. [PATCH for-8.2 0/3] UI: fix default VC regressions
  4. View patch

[PATCH for-8.2 1/3] vl: revert behaviour for -display none

marcandre.lureau@redhat.com posted 3 patches 1 year ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
[PATCH for-8.2 1/3] vl: revert behaviour for -display none
Posted by marcandre.lureau@redhat.com 1 year ago 
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");
         }
-- 
2.41.0
Re: [PATCH for-8.2 1/3] vl: revert behaviour for -display none
Posted by Peter Maydell 1 year ago 
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
Re: [PATCH for-8.2 1/3] vl: revert behaviour for -display none
Posted by David Woodhouse 1 year ago 
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>
Re: [EXTERNAL] [PATCH for-8.2 1/3] vl: revert behaviour for -display none
Posted by David Woodhouse 1 year ago 
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/

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.