[PATCH v1] tools: make "opengl" generic

[Penny Zheng <Penny.Zheng@xxxxxxx>]

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>
---
 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");
-- 
2.34.1