Re: [Qemu-devel] [PATCH] chardev: add vte chardev

[Anthony Liguori <aliguori@us.ibm.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Kill the dirty hack which hooks gtk vte initialization into the qemu
> consoles subsystem.  The vte terminals are not related to qemu consoles
> at all.  This simply doesn't belong there and it stands in the way when
> cleaning up the qemu consoles subsystem.  So fix it up, quickly, before
> it sneaks into a release.


My only concern is that if someone had a command line like:

qemu -serial stdio -monitor vc

It now breaks with your series.  'vc' is a graphical chardev and it
should be up to what UI layer to decide how to express it.  That's the
idea behind the hooks.

What's the issue your having with console cleanup?

Regards,

Anthony Liguori

>
> Make vtes normal chardev backends, like everybody else is.  Tweak the
> default chardevs accordingly, so you'll get monitor+serial on vte
> when using gtk.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>
> To be applied on top of the chardev qapi conversion patch series
>
>  include/char/char.h  |    6 ++++++
>  include/ui/console.h |    5 -----
>  qapi-schema.json     |    1 +
>  qemu-char.c          |    9 ++++++++-
>  ui/console.c         |   14 +-------------
>  ui/gtk.c             |    7 +------
>  vl.c                 |   44 ++++++++++++++++++++------------------------
>  7 files changed, 37 insertions(+), 49 deletions(-)
>
> diff --git a/include/char/char.h b/include/char/char.h
> index d6a0351..09edfcc 100644
> --- a/include/char/char.h
> +++ b/include/char/char.h
> @@ -267,4 +267,10 @@ CharDriverState *qemu_chr_open_msmouse(void);
>  /* baum.c */
>  CharDriverState *chr_baum_init(void);
>  
> +/* console.c */
> +CharDriverState *text_console_init(ChardevVC *vc);
> +
> +/* gtk.c */
> +CharDriverState *gd_vte_init(void);
> +
>  #endif
> diff --git a/include/ui/console.h b/include/ui/console.h
> index a37cf65..7ae32aa 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -450,11 +450,6 @@ void qemu_console_resize(DisplayState *ds, int width, int height);
>  void qemu_console_copy(DisplayState *ds, int src_x, int src_y,
>                         int dst_x, int dst_y, int w, int h);
>  
> -typedef CharDriverState *(VcHandler)(ChardevVC *vc);
> -
> -CharDriverState *vc_init(ChardevVC *vc);
> -void register_vc_handler(VcHandler *handler);
> -
>  /* sdl.c */
>  void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
>  
> diff --git a/qapi-schema.json b/qapi-schema.json
> index fdaa9da..c26ffd2 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3296,6 +3296,7 @@
>                                         'spicevmc' : 'ChardevSpiceChannel',
>                                         'spiceport' : 'ChardevSpicePort',
>                                         'vc'     : 'ChardevVC',
> +                                       'vte'    : 'ChardevDummy',
>                                         'memory' : 'ChardevRingbuf' } }
>  
>  ##
> diff --git a/qemu-char.c b/qemu-char.c
> index e633797..e18728a 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3027,6 +3027,7 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename)
>          strcmp(filename, "pty")     == 0 ||
>          strcmp(filename, "msmouse") == 0 ||
>          strcmp(filename, "braille") == 0 ||
> +        strcmp(filename, "vte")     == 0 ||
>          strcmp(filename, "stdio")   == 0) {
>          qemu_opt_set(opts, "backend", filename);
>          return opts;
> @@ -3758,8 +3759,13 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>          break;
>  #endif
>      case CHARDEV_BACKEND_KIND_VC:
> -        chr = vc_init(backend->vc);
> +        chr = text_console_init(backend->vc);
>          break;
> +#ifdef CONFIG_GTK
> +    case CHARDEV_BACKEND_KIND_VTE:
> +        chr = gd_vte_init();
> +        break;
> +#endif
>      case CHARDEV_BACKEND_KIND_MEMORY:
>          chr = qemu_chr_open_ringbuf(backend->memory, errp);
>          break;
> @@ -3823,6 +3829,7 @@ static void register_types(void)
>      register_char_driver_qapi("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL);
>      register_char_driver_qapi("pipe", CHARDEV_BACKEND_KIND_PIPE,
>                                qemu_chr_parse_pipe);
> +    register_char_driver_qapi("vte", CHARDEV_BACKEND_KIND_VTE, NULL);
>  }
>  
>  type_init(register_types);
> diff --git a/ui/console.c b/ui/console.c
> index 27e87f8..820ec6a 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1537,7 +1537,7 @@ static void text_console_do_init(CharDriverState *chr, DisplayState *ds)
>          chr->init(chr);
>  }
>  
> -static CharDriverState *text_console_init(ChardevVC *vc)
> +CharDriverState *text_console_init(ChardevVC *vc)
>  {
>      CharDriverState *chr;
>      QemuConsole *s;
> @@ -1577,18 +1577,6 @@ static CharDriverState *text_console_init(ChardevVC *vc)
>      return chr;
>  }
>  
> -static VcHandler *vc_handler = text_console_init;
> -
> -CharDriverState *vc_init(ChardevVC *vc)
> -{
> -    return vc_handler(vc);
> -}
> -
> -void register_vc_handler(VcHandler *handler)
> -{
> -    vc_handler = handler;
> -}
> -
>  void text_consoles_set_display(DisplayState *ds)
>  {
>      int i;
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 794dab1..76111b4 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -991,7 +991,7 @@ static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
>  static int nb_vcs;
>  static CharDriverState *vcs[MAX_VCS];
>  
> -static CharDriverState *gd_vc_handler(ChardevVC *unused)
> +CharDriverState *gd_vte_init(void)
>  {
>      CharDriverState *chr;
>  
> @@ -1003,11 +1003,6 @@ static CharDriverState *gd_vc_handler(ChardevVC *unused)
>      return chr;
>  }
>  
> -void early_gtk_display_init(void)
> -{
> -    register_vc_handler(gd_vc_handler);
> -}
> -
>  static gboolean gd_vc_in(GIOChannel *chan, GIOCondition cond, void *opaque)
>  {
>      VirtualConsole *vc = opaque;
> diff --git a/vl.c b/vl.c
> index a621aec..2efa1eb 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4016,6 +4016,19 @@ int main(int argc, char **argv, char **envp)
>  #endif
>      }
>  
> +    if (display_type == DT_DEFAULT && !display_remote) {
> +#if defined(CONFIG_GTK)
> +        display_type = DT_GTK;
> +#elif defined(CONFIG_SDL) || defined(CONFIG_COCOA)
> +        display_type = DT_SDL;
> +#elif defined(CONFIG_VNC)
> +        vnc_display = "localhost:0,to=99";
> +        show_vnc_port = 1;
> +#else
> +        display_type = DT_NONE;
> +#endif
> +    }
> +
>      if (display_type == DT_NOGRAPHIC) {
>          if (default_parallel)
>              add_device_config(DEV_PARALLEL, "null");
> @@ -4037,38 +4050,21 @@ int main(int argc, char **argv, char **envp)
>                  monitor_parse("stdio", "readline");
>          }
>      } else {
> +        const char *chardev = (display_type == DT_GTK)
> +            ? "vte" : "vc:80Cx24C";
>          if (default_serial)
> -            add_device_config(DEV_SERIAL, "vc:80Cx24C");
> +            add_device_config(DEV_SERIAL, chardev);
>          if (default_parallel)
> -            add_device_config(DEV_PARALLEL, "vc:80Cx24C");
> +            add_device_config(DEV_PARALLEL, chardev);
>          if (default_monitor)
> -            monitor_parse("vc:80Cx24C", "readline");
> +            monitor_parse(chardev, "readline");
>          if (default_virtcon)
> -            add_device_config(DEV_VIRTCON, "vc:80Cx24C");
> +            add_device_config(DEV_VIRTCON, chardev);
>          if (default_sclp) {
> -            add_device_config(DEV_SCLP, "vc:80Cx24C");
> +            add_device_config(DEV_SCLP, chardev);
>          }
>      }
>  
> -    if (display_type == DT_DEFAULT && !display_remote) {
> -#if defined(CONFIG_GTK)
> -        display_type = DT_GTK;
> -#elif defined(CONFIG_SDL) || defined(CONFIG_COCOA)
> -        display_type = DT_SDL;
> -#elif defined(CONFIG_VNC)
> -        vnc_display = "localhost:0,to=99";
> -        show_vnc_port = 1;
> -#else
> -        display_type = DT_NONE;
> -#endif
> -    }
> -
> -#if defined(CONFIG_GTK)
> -    if (display_type == DT_GTK) {
> -        early_gtk_display_init();
> -    }
> -#endif
> -
>      socket_init();
>  
>      if (qemu_opts_foreach(qemu_find_opts("chardev"), chardev_init_func, NULL, 1) != 0)
> -- 
> 1.7.9.7