Re: [PATCH] gtk: disable GTK Clipboard with a new option 'gtk_clipboard'

[Jim Fehlig]

I should make myself useful around here on occasion when items are within my 
skill set. But I already struggle to find time for that in the libvirt community 
:-).

On 11/8/22 09:23, Claudio Fontana wrote:
> The GTK Clipboard implementation may cause guest hangs.
>
> Therefore implement a new configure switch --enable-gtk-clipboard,
> disabled by default, as a meson option.
>
> Regenerate the meson build options to include it.
>
> The initialization of the clipboard is gtk.c, as well as the
> compilation of gtk-clipboard.c are now conditional on this new option
> to be set.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1150
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>   meson.build                   | 9 +++++++++
>   meson_options.txt             | 7 +++++++
>   scripts/meson-buildoptions.sh | 3 +++
>   ui/gtk.c                      | 2 ++
>   ui/meson.build                | 5 ++++-
>   5 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 1d448272ab..8bb96e5e8c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1243,6 +1243,8 @@ if nettle.found() and gmp.found()
>   endif
>   
>   
> +have_gtk_clipboard = false

Can this be initialized with get_option(), instead of the two calls below?

> +
>   gtk = not_found
>   gtkx11 = not_found
>   vte = not_found
> @@ -1258,12 +1260,18 @@ if not get_option('gtk').auto() or have_system
>                           kwargs: static_kwargs)
>       gtk = declare_dependency(dependencies: [gtk, gtkx11])
>   
> +    have_gtk_clipboard = get_option('gtk_clipboard').enabled()
> +
>       if not get_option('vte').auto() or have_system
>         vte = dependency('vte-2.91',
>                          method: 'pkg-config',
>                          required: get_option('vte'),
>                          kwargs: static_kwargs)
>       endif
> +  else
> +    if get_option('gtk_clipboard').enabled()
> +      error('GTK clipboard requested, but GTK not found')
> +    endif
>     endif
>   endif
>   
> @@ -1842,6 +1850,7 @@ if glusterfs.found()
>   endif
>   config_host_data.set('CONFIG_GTK', gtk.found())
>   config_host_data.set('CONFIG_VTE', vte.found())
> +config_host_data.set('CONFIG_GTK_CLIPBOARD', have_gtk_clipboard)
>   config_host_data.set('CONFIG_LIBATTR', have_old_libattr)
>   config_host_data.set('CONFIG_LIBCAP_NG', libcap_ng.found())
>   config_host_data.set('CONFIG_EBPF', libbpf.found())
> diff --git a/meson_options.txt b/meson_options.txt
> index 66128178bf..4b749ca549 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -219,6 +219,13 @@ option('vnc_sasl', type : 'feature', value : 'auto',
>          description: 'SASL authentication for VNC server')
>   option('vte', type : 'feature', value : 'auto',
>          description: 'vte support for the gtk UI')
> +
> +# GTK Clipboard implementation is disabled by default, since it may cause hangs
> +# of the guest VCPUs. See gitlab issue 1150:
> +# https://gitlab.com/qemu-project/qemu/-/issues/1150
> +
> +option('gtk_clipboard', type: 'feature', value : 'disabled',
> +       description: 'clipboard support for the gtk UI (EXPERIMENTAL, MAY 
> HANG)')

'boolean' seems a more appropriate type, but I see 'feature' is common practice 
with these various options. Is gtk_clipboard marked experimental elsewhere? Is 
there a need for the warning text?

>   option('xkbcommon', type : 'feature', value : 'auto',
>          description: 'xkbcommon support')
>   option('zstd', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 2cb0de5601..a542853bfd 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -93,6 +93,7 @@ meson_options_help() {
>     printf "%s\n" '  glusterfs       Glusterfs block device driver'
>     printf "%s\n" '  gnutls          GNUTLS cryptography support'
>     printf "%s\n" '  gtk             GTK+ user interface'
> +  printf "%s\n" '  gtk-clipboard   clipboard support for the gtk UI 
> (EXPERIMENTAL, MAY HANG)'

Same here. None of the other options have such warning. Cant this be treated 
like the others, just another option to be enabled or disabled? Whether or not 
the option works is another matter.

Jim

>     printf "%s\n" '  guest-agent     Build QEMU Guest Agent'
>     printf "%s\n" '  guest-agent-msi Build MSI package for the QEMU Guest Agent'
>     printf "%s\n" '  hax             HAX acceleration support'
> @@ -274,6 +275,8 @@ _meson_option_parse() {
>       --disable-gprof) printf "%s" -Dgprof=false ;;
>       --enable-gtk) printf "%s" -Dgtk=enabled ;;
>       --disable-gtk) printf "%s" -Dgtk=disabled ;;
> +    --enable-gtk-clipboard) printf "%s" -Dgtk_clipboard=enabled ;;
> +    --disable-gtk-clipboard) printf "%s" -Dgtk_clipboard=disabled ;;
>       --enable-guest-agent) printf "%s" -Dguest_agent=enabled ;;
>       --disable-guest-agent) printf "%s" -Dguest_agent=disabled ;;
>       --enable-guest-agent-msi) printf "%s" -Dguest_agent_msi=enabled ;;
> diff --git a/ui/gtk.c b/ui/gtk.c
> index 7ec21f7798..4817623c8f 100644
> --- a/ui/gtk.c
> +++ b/ui/gtk.c
> @@ -2403,7 +2403,9 @@ static void gtk_display_init(DisplayState *ds, 
> DisplayOptions *opts)
>           opts->u.gtk.show_tabs) {
>           gtk_menu_item_activate(GTK_MENU_ITEM(s->show_tabs_item));
>       }
> +#ifdef CONFIG_GTK_CLIPBOARD
>       gd_clipboard_init(s);
> +#endif /* CONFIG_GTK_CLIPBOARD */
>   }
>   
>   static void early_gtk_display_init(DisplayOptions *opts)
> diff --git a/ui/meson.build b/ui/meson.build
> index ec13949776..c1b137bf33 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -97,7 +97,10 @@ if gtk.found()
>     softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('win32-kbd-hook.c'))
>   
>     gtk_ss = ss.source_set()
> -  gtk_ss.add(gtk, vte, pixman, files('gtk.c', 'gtk-clipboard.c'))
> +  gtk_ss.add(gtk, vte, pixman, files('gtk.c'))
> +  if have_gtk_clipboard
> +    gtk_ss.add(files('gtk-clipboard.c'))
> +  endif
>     gtk_ss.add(when: x11, if_true: files('x_keymap.c'))
>     gtk_ss.add(when: opengl, if_true: files('gtk-gl-area.c'))
>     gtk_ss.add(when: [x11, opengl], if_true: files('gtk-egl.c'))