Re: [PATCH for-8.2] Revert "ui/console: allow to override the default VC"

[Marc-André Lureau]

Hi

On Thu, Nov 16, 2023 at 10:25 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This reverts commit 1bec1cc0da497e55c16e2a7b50f94cdb2a02197f.  This
> commit 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'
>
> Revert the commit to restore the previous handling of "-display
> none".
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1974
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

By simply reverting, we break qemu with --disable-pixman:
qemu-system-x86_64: 'vc' is not a valid char driver name

The change of behaviour was a consequence of Paolo suggestion from v2:
https://patchew.org/QEMU/20230918135206.2739222-1-marcandre.lureau@redhat.com/20230918135206.2739222-6-marcandre.lureau@redhat.com/#4b890258-3426-0e0f-dd65-6114b9bee5e3@redhat.com

Let's get back to the current behaviour and not add muxed console in
any case (disable pixman or not). I'll try to make a patch asap.

> ---
>  include/ui/console.h |  2 --
>  system/vl.c          | 27 ++++++++++-----------------
>  ui/console.c         | 17 -----------------
>  3 files changed, 10 insertions(+), 36 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index a4a49ffc640..acb61a7f152 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -462,14 +462,12 @@ struct QemuDisplay {
>      DisplayType type;
>      void (*early_init)(DisplayOptions *opts);
>      void (*init)(DisplayState *ds, DisplayOptions *opts);
> -    const char *vc;
>  };
>
>  void qemu_display_register(QemuDisplay *ui);
>  bool qemu_display_find_default(DisplayOptions *opts);
>  void qemu_display_early_init(DisplayOptions *opts);
>  void qemu_display_init(DisplayState *ds, DisplayOptions *opts);
> -const char *qemu_display_get_vc(DisplayOptions *opts);
>  void qemu_display_help(void);
>
>  /* vnc.c */
> diff --git a/system/vl.c b/system/vl.c
> index 5af7ced2a16..3d64a90f253 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -1372,7 +1372,6 @@ static void qemu_setup_display(void)
>  static void qemu_create_default_devices(void)
>  {
>      MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
> -    const char *vc = qemu_display_get_vc(&dpy);
>
>      if (is_daemonized()) {
>          /* According to documentation and historically, -nographic redirects
> @@ -1391,30 +1390,24 @@ static void qemu_create_default_devices(void)
>          }
>      }
>
> -    if (nographic || (!vc && !is_daemonized() && isatty(STDOUT_FILENO))) {
> -        if (default_parallel) {
> +    if (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_serial) {
> +            if (default_serial)
>                  add_device_config(DEV_SERIAL, "stdio");
> -            }
> -            if (default_monitor) {
> +            if (default_monitor)
>                  monitor_parse("stdio", "readline", false);
> -            }
>          }
>      } else {
> -        if (default_serial) {
> -            add_device_config(DEV_SERIAL, vc ?: "null");
> -        }
> -        if (default_parallel) {
> -            add_device_config(DEV_PARALLEL, vc ?: "null");
> -        }
> -        if (default_monitor && vc) {
> -            monitor_parse(vc, "readline", false);
> -        }
> +        if (default_serial)
> +            add_device_config(DEV_SERIAL, "vc:80Cx24C");
> +        if (default_parallel)
> +            add_device_config(DEV_PARALLEL, "vc:80Cx24C");
> +        if (default_monitor)
> +            monitor_parse("vc:80Cx24C", "readline", false);
>      }
>
>      if (default_net) {
> diff --git a/ui/console.c b/ui/console.c
> index 8e688d35695..676d0cbaba2 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1677,23 +1677,6 @@ void qemu_display_init(DisplayState *ds, 
> DisplayOptions *opts)
>      dpys[opts->type]->init(ds, opts);
>  }
>
> -const char *qemu_display_get_vc(DisplayOptions *opts)
> -{
> -    assert(opts->type < DISPLAY_TYPE__MAX);
> -    if (opts->type == DISPLAY_TYPE_NONE) {
> -        return NULL;
> -    }
> -    assert(dpys[opts->type] != NULL);
> -    if (dpys[opts->type]->vc) {
> -        return dpys[opts->type]->vc;
> -    } else {
> -#ifdef CONFIG_PIXMAN
> -        return "vc:80Cx24C";
> -#endif
> -    }
> -    return NULL;
> -}
> -
>  void qemu_display_help(void)
>  {
>      int idx;
> --
> 2.34.1
>
>


-- 
Marc-André Lureau