[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1] tools: make "opengl" generic



Hello,

Le 28/07/2025 à 06:45, Penny Zheng a écrit :
> Display option, like vnc, sdl, etc, will be checked against in latest QEMU
> whether it is compatile with opengl context. And vnc is incompatible with GL
> context.
> Now, when running hvm domain with gl context on, such as
> "device_model_args_hvm = ["-display", "sdl,gl=on"]", we will fail with
> the error of "qemu-system-i386: -vnc 127.0.0.1:0,to=99: Display vnc is
> incompatible with the GL context", as vnc is set enabled on default
> for HVM domain.
>
> We shall move "opengl" option out of specifc sdl display, to make it
> generic. Then when users explicitly set "opengl = 1", default values for
> vnc shall be changed to disabled and libxl__dm_vnc() needs to return NULL
> indicating vnc being disabled.
>
> If users select both vnc and opengl in xl configuration, creation
> will fail and error out incompatible info.
> To keep consistency, we also make "opengl" generic for vfb[] options
>
> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>

There is a way to make vnc work with opengl by using the egl-headless
display (-display egl-headless,gl=on), which can coexist along vnc. So
that we could avoid having "opengl" as a sdl-specific option.

> ---
>   tools/libs/light/libxl_console.c |  4 ++--
>   tools/libs/light/libxl_create.c  | 10 ++++++----
>   tools/libs/light/libxl_dm.c      |  7 ++++++-
>   tools/libs/light/libxl_types.idl |  3 ++-
>   tools/xl/xl_parse.c              | 17 +++++++++--------
>   tools/xl/xl_sxp.c                |  6 +++---
>   6 files changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/tools/libs/light/libxl_console.c 
> b/tools/libs/light/libxl_console.c
> index 044ca64676..fc3dfddc4d 100644
> --- a/tools/libs/light/libxl_console.c
> +++ b/tools/libs/light/libxl_console.c
> @@ -694,7 +694,7 @@ static int libxl__device_vfb_setdefault(libxl__gc *gc, 
> uint32_t domid,
>       }
>
>       libxl_defbool_setdefault(&vfb->sdl.enable, false);
> -    libxl_defbool_setdefault(&vfb->sdl.opengl, false);
> +    libxl_defbool_setdefault(&vfb->opengl, false);
>
>       rc = libxl__resolve_domid(gc, vfb->backend_domname, 
> &vfb->backend_domid);
>       return rc;
> @@ -733,7 +733,7 @@ static int libxl__set_xenstore_vfb(libxl__gc *gc, 
> uint32_t domid,
>       flexarray_append_pair(back, "sdl",
>                             libxl_defbool_val(vfb->sdl.enable) ? "1" : "0");
>       flexarray_append_pair(back, "opengl",
> -                          libxl_defbool_val(vfb->sdl.opengl) ? "1" : "0");
> +                          libxl_defbool_val(vfb->opengl) ? "1" : "0");
>       if (vfb->sdl.xauthority) {
>           flexarray_append_pair(back, "xauthority", vfb->sdl.xauthority);
>       }
> diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
> index 4301f17f90..7bbd1ff9b4 100644
> --- a/tools/libs/light/libxl_create.c
> +++ b/tools/libs/light/libxl_create.c
> @@ -339,7 +339,12 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>           if (!b_info->u.hvm.boot)
>               b_info->u.hvm.boot = libxl__strdup(NOGC, "cda");
>
> -        libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
> +        libxl_defbool_setdefault(&b_info->u.hvm.opengl, false);
> +
> +        if (libxl_defbool_val(b_info->u.hvm.opengl))
> +            libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, false);
> +        else
> +            libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
>           if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
>               libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
>               if (!b_info->u.hvm.vnc.listen)
> @@ -347,9 +352,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>           }
>
>           libxl_defbool_setdefault(&b_info->u.hvm.sdl.enable, false);
> -        if (libxl_defbool_val(b_info->u.hvm.sdl.enable)) {
> -            libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
> -        }
>
>           if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
>               libxl_defbool_setdefault(&b_info->u.hvm.spice.disable_ticketing,
> diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c
> index 511ec76a65..7adf473c81 100644
> --- a/tools/libs/light/libxl_dm.c
> +++ b/tools/libs/light/libxl_dm.c
> @@ -672,6 +672,10 @@ int libxl__domain_device_construct_rdm(libxl__gc *gc,
>   const libxl_vnc_info *libxl__dm_vnc(const libxl_domain_config *guest_config)
>   {
>       const libxl_vnc_info *vnc = NULL;
> +
> +    if (libxl_defbool_val(guest_config->b_info.u.hvm.opengl))
> +        return NULL;
> +
>       if (guest_config->b_info.type == LIBXL_DOMAIN_TYPE_HVM) {
>           vnc = &guest_config->b_info.u.hvm.vnc;
>       } else if (guest_config->num_vfbs > 0) {
> @@ -955,6 +959,7 @@ static int libxl__build_device_model_args_new(libxl__gc 
> *gc,
>       const char *path, *chardev;
>       bool is_stubdom = libxl_defbool_val(b_info->device_model_stubdomain);
>       int rc;
> +    bool has_opengl = libxl_defbool_val(b_info->u.hvm.opengl);
>
>       dm_args = flexarray_make(gc, 16, 1);
>       dm_envs = flexarray_make(gc, 16, 1);
> @@ -1084,7 +1089,7 @@ static int libxl__build_device_model_args_new(libxl__gc 
> *gc,
>           }
>
>           flexarray_append(dm_args, vncarg);
> -    } else if (!is_stubdom) {
> +    } else if (!is_stubdom && !has_opengl) {
>           /*
>            * Ensure that by default no vnc server is created.
>            */
> diff --git a/tools/libs/light/libxl_types.idl 
> b/tools/libs/light/libxl_types.idl
> index fe251649f3..ab768381ce 100644
> --- a/tools/libs/light/libxl_types.idl
> +++ b/tools/libs/light/libxl_types.idl
> @@ -349,7 +349,6 @@ libxl_spice_info = Struct("spice_info", [
>
>   libxl_sdl_info = Struct("sdl_info", [
>       ("enable",        libxl_defbool),
> -    ("opengl",        libxl_defbool),
>       ("display",       string),
>       ("xauthority",    string),
>       ])
> @@ -670,6 +669,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                          ("acpi_firmware",    string),
>                                          ("hdtype",           libxl_hdtype),
>                                          ("nographic",        libxl_defbool),
> +                                       ("opengl",           libxl_defbool),
>                                          ("vga",              
> libxl_vga_interface_info),
>                                          ("vnc",              libxl_vnc_info),
>                                          # keyboard layout, default is en-us 
> keyboard
> @@ -748,6 +748,7 @@ libxl_device_vfb = Struct("device_vfb", [
>       ("backend_domid", libxl_domid),
>       ("backend_domname",string),
>       ("devid",         libxl_devid),
> +    ("opengl",        libxl_defbool),
>       ("vnc",           libxl_vnc_info),
>       ("sdl",           libxl_sdl_info),
>       # set keyboard layout, default is en-us keyboard
> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 28cdbf07c2..9e9adcec77 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -272,7 +272,6 @@ static void parse_top_level_sdl_options(XLU_Config 
> *config,
>                                           libxl_sdl_info *sdl)
>   {
>       xlu_cfg_get_defbool(config, "sdl", &sdl->enable, 0);
> -    xlu_cfg_get_defbool(config, "opengl", &sdl->opengl, 0);
>       xlu_cfg_replace_string (config, "display", &sdl->display, 0);
>       xlu_cfg_replace_string (config, "xauthority", &sdl->xauthority, 0);
>   }
> @@ -1291,7 +1290,7 @@ void parse_config_data(const char *config_source,
>   {
>       libxl_physinfo physinfo;
>       const char *buf;
> -    long l, vcpus = 0;
> +    long l, vcpus = 0, vnc_enabled = 0;
>       XLU_Config *config;
>       XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
>                      *usbctrls, *usbdevs, *p9devs, *vdispls, *pvcallsifs_devs;
> @@ -2572,7 +2571,7 @@ skip_nic:
>                   } else if (!strcmp(p, "sdl")) {
>                       libxl_defbool_set(&vfb->sdl.enable, atoi(p2 + 1));
>                   } else if (!strcmp(p, "opengl")) {
> -                    libxl_defbool_set(&vfb->sdl.opengl, atoi(p2 + 1));
> +                    libxl_defbool_set(&vfb->opengl, atoi(p2 + 1));
>                   } else if (!strcmp(p, "display")) {
>                       free(vfb->sdl.display);
>                       vfb->sdl.display = strdup(p2 + 1);
> @@ -2791,14 +2790,16 @@ skip_usbdev:
>
>   #undef parse_extra_args
>
> +    if (!xlu_cfg_get_long (config, "vnc", &l, 0))
> +        vnc_enabled = l;
> +    xlu_cfg_get_defbool(config, "opengl", &b_info->u.hvm.opengl, 0);
> +    if (vnc_enabled && libxl_defbool_val(b_info->u.hvm.opengl)) {
> +        fprintf(stderr, "vnc is incompatible with opengl\n");
> +        exit(1);
> +    }
>       /* If we've already got vfb=[] for PV guest then ignore top level
>        * VNC config. */
>       if (c_info->type == LIBXL_DOMAIN_TYPE_PV && !d_config->num_vfbs) {
> -        long vnc_enabled = 0;
> -
> -        if (!xlu_cfg_get_long (config, "vnc", &l, 0))
> -            vnc_enabled = l;
> -
>           if (vnc_enabled) {
>               libxl_device_vfb *vfb;
>               libxl_device_vkb *vkb;
> diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
> index 4383ad177a..62a1d012c6 100644
> --- a/tools/xl/xl_sxp.c
> +++ b/tools/xl/xl_sxp.c
> @@ -120,7 +120,7 @@ void printf_info_sexp(int domid, libxl_domain_config 
> *d_config, FILE *fh)
>           fprintf(fh, "\t\t\t(sdl %s)\n",
>                  libxl_defbool_to_string(b_info->u.hvm.sdl.enable));
>           fprintf(fh, "\t\t\t(opengl %s)\n",
> -               libxl_defbool_to_string(b_info->u.hvm.sdl.opengl));
> +               libxl_defbool_to_string(b_info->u.hvm.opengl));
>           fprintf(fh, "\t\t\t(nographic %s)\n",
>                  libxl_defbool_to_string(b_info->u.hvm.nographic));
>           fprintf(fh, "\t\t\t(spice %s)\n",
> @@ -219,10 +219,10 @@ void printf_info_sexp(int domid, libxl_domain_config 
> *d_config, FILE *fh)
>           fprintf(fh, "\t\t\t(vncunused %s)\n",
>                  libxl_defbool_to_string(d_config->vfbs[i].vnc.findunused));
>           fprintf(fh, "\t\t\t(keymap %s)\n", d_config->vfbs[i].keymap);
> +        fprintf(fh, "\t\t\t(opengl %s)\n",
> +               libxl_defbool_to_string(d_config->vfbs[i].opengl));
>           fprintf(fh, "\t\t\t(sdl %s)\n",
>                  libxl_defbool_to_string(d_config->vfbs[i].sdl.enable));
> -        fprintf(fh, "\t\t\t(opengl %s)\n",
> -               libxl_defbool_to_string(d_config->vfbs[i].sdl.opengl));
>           fprintf(fh, "\t\t\t(display %s)\n", d_config->vfbs[i].sdl.display);
>           fprintf(fh, "\t\t\t(xauthority %s)\n", 
> d_config->vfbs[i].sdl.xauthority);
>           fprintf(fh, "\t\t)\n");



Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech





 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.