Since QEMU 2.10, there's a new cmdline option '-display egl-headless'
which enables OpenGL support for cases where we can't render on a local
display (i.e. <listen> is set to either 'address' or 'network').
This is to work around the current restriction on the local node with
native OpenGL SPICE support. However, compared to native support,
egl-headless has overhead, because the rendered framebuffer has to be
copied back to QEMU's display area which is then accessed by either
SPICE or VNC and sent to the remote side.
This patch enables this configuration feature by introducing a new
SPICE-only attribute 'native' which helps libvirt to determine whether
to use '-spice gl=on' or '-display egl-headless' on the cmdline. If the
attribute wasn't specified but OpenGL is to be enabled, then given the
nature of the <listen> element, libvirt will determine the default
value.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
docs/formatdomain.html.in | 8 ++++
docs/schemas/domaincommon.rng | 16 +++++--
src/conf/domain_conf.c | 49 ++++++++++++++++++++--
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 43 ++++++++++++-------
.../qemuxml2argvdata/graphics-spice-gl-native.args | 26 ++++++++++++
.../qemuxml2argvdata/graphics-spice-gl-native.xml | 25 +++++++++++
.../graphics-spice-gl-non-native.args | 27 ++++++++++++
.../graphics-spice-gl-non-native.xml | 24 +++++++++++
tests/qemuxml2argvtest.c | 8 ++++
.../video-virtio-gpu-spice-gl.xml | 2 +-
11 files changed, 206 insertions(+), 23 deletions(-)
create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-native.args
create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-native.xml
create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-non-native.args
create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-non-native.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index aa0d6b26df..c6ebd39bd9 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6461,6 +6461,14 @@ qemu-kvm -net nic,model=? /dev/null
You can enable or disable OpenGL support explicitly with
the <code>gl</code> element, by setting the <code>enable</code>
property. (QEMU only, <span class="since">since 1.3.3</span>).
+ Additionally, attribute <code>native</code>
+ (<span class="since">Since 4.6.0</span>) can be used to specify
+ whether native OpenGL support should be used with SPICE or
+ egl-headless should be used instead. Note that the native OpenGL
+ support is only limited to the local setup (<code>listen</code> is
+ either 'none' or 'socket') and for remote access egl-headless
+ needs to be used. The supported values for this attribute are 'on'
+ and 'off'.
</p>
<p>
By default, QEMU will pick the first available GPU DRM render node.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 20649c5f6f..4ad53d976b 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3066,6 +3066,11 @@
<attribute name="enable">
<ref name="virYesNo"/>
</attribute>
+ <optional>
+ <attribute name="native">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
<empty/>
</element>
</optional>
@@ -3322,9 +3327,14 @@
<ref name="absFilePath"/>
</attribute>
</optional>
- <empty/>
- </element>
- </optional>
+ <optional>
+ <attribute name="native">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
+ <empty/>
+ </element>
+ </optional>
</interleave>
</group>
<group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2ccd9e124f..6d399a198e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4280,6 +4280,7 @@ virDomainDefPostParseGraphics(virDomainDef *def)
* same. */
if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
virDomainGraphicsListenDefPtr glisten = &graphics->listens[0];
+ virDomainGraphicsGLDefPtr gl = graphics->gl;
if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS &&
graphics->data.spice.port == 0 &&
@@ -4288,6 +4289,28 @@ virDomainDefPostParseGraphics(virDomainDef *def)
VIR_FREE(glisten->address);
glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE;
}
+
+ /* Next we need to figure out how to properly configure the OpenGL
+ * if that is enabled and the 'native' attribute is missing.
+ * The cases are:
+ * 1) Listen type is either 'socket' or 'none' - SPICE native
+ * OpenGL support (,gl=on) should be used because we're
+ * rendering on a local display.
+ *
+ * 2) Listen is either network or address - SPICE can't use
+ * the native OpenGL support remotely yet, so we use
+ * native='no' and format '-display egl-headless' onto the
+ * cmdline.
+ */
+ if (graphics->gl &&
+ graphics->gl->enable == VIR_TRISTATE_BOOL_YES &&
+ graphics->gl->native == VIR_TRISTATE_BOOL_ABSENT) {
+ gl->native = VIR_TRISTATE_BOOL_NO;
+
+ if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE ||
+ glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET)
+ gl->native = VIR_TRISTATE_BOOL_YES;
+ }
}
}
}
@@ -13573,6 +13596,7 @@ virDomainGraphicsGLDefParseXML(virDomainGraphicsDefPtr def,
{
virDomainGraphicsGLDefPtr gl = NULL;
char *enable = NULL;
+ char *native = NULL;
int ret = -1;
if (!node)
@@ -13596,14 +13620,26 @@ virDomainGraphicsGLDefParseXML(virDomainGraphicsDefPtr def,
goto cleanup;
}
- if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
- gl->rendernode = virXMLPropString(node, "rendernode");
+ /* SPICE recognizes a few more attributes */
+ if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+ gl->rendernode = virXMLPropString(node, "rendernode");
+
+ native = virXMLPropString(node, "native");
+ if (native &&
+ (gl->native = virTristateBoolTypeFromString(native)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown value for attribute enable '%s'"),
+ enable);
+ goto cleanup;
+ }
+ }
VIR_STEAL_PTR(def->gl, gl);
ret = 0;
cleanup:
VIR_FREE(enable);
+ VIR_FREE(native);
virDomainGraphicsGLDefFree(gl);
return ret;
}
@@ -26184,8 +26220,13 @@ virDomainGraphicsGLDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def)
virBufferAsprintf(buf, "<gl enable='%s'",
virTristateBoolTypeToString(gl->enable));
- if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
- virBufferEscapeString(buf, " rendernode='%s'", gl->rendernode);
+ if (gl->enable == VIR_TRISTATE_BOOL_YES) {
+ if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+ virBufferAsprintf(buf, " native='%s'",
+ virTristateBoolTypeToString(gl->native));
+ virBufferEscapeString(buf, " rendernode='%s'", gl->rendernode);
+ }
+ }
virBufferAddLit(buf, "/>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 20dc1334c4..99b5896391 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1602,6 +1602,7 @@ typedef struct _virDomainGraphicsGLDef virDomainGraphicsGLDef;
typedef virDomainGraphicsGLDef *virDomainGraphicsGLDefPtr;
struct _virDomainGraphicsGLDef {
virTristateBool enable;
+ virTristateBool native; /* -spice gl=on vs -display egl-headless for QEMU */
char *rendernode; /* SPICE only */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 89a8408df6..fc80a6c3a6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7896,27 +7896,39 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
static int
qemuBuildGraphicsSPICEGLCommandLine(virDomainGraphicsGLDefPtr gl,
+ virCommandPtr cmd,
virBufferPtr opt,
virQEMUCapsPtr qemuCaps)
{
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("This QEMU doesn't support spice OpenGL"));
- return -1;
- }
-
- virBufferAddLit(opt, "gl=on,");
-
- if (gl->rendernode) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) {
+ if (gl->native == VIR_TRISTATE_BOOL_NO) {
+ /* For non-native OpenGL, we need to add egl-headless to the cmdline.
+ *
+ * NB: QEMU defaults to '-spice gl=off', so we don't have to add that
+ * explicitly, especially since we're not testing for GL capability
+ * presence.
+ */
+ virCommandAddArg(cmd, "-display");
+ virCommandAddArg(cmd, "egl-headless");
+ } else {
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("This QEMU doesn't support spice OpenGL rendernode"));
+ _("This QEMU doesn't support spice OpenGL"));
return -1;
}
- virBufferAddLit(opt, "rendernode=");
- virQEMUBuildBufferEscapeComma(opt, gl->rendernode);
- virBufferAddLit(opt, ",");
+ virBufferAddLit(opt, "gl=on,");
+
+ if (gl->rendernode) {
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("This QEMU doesn't support spice OpenGL rendernode"));
+ return -1;
+ }
+
+ virBufferAddLit(opt, "rendernode=");
+ virQEMUBuildBufferEscapeComma(opt, gl->rendernode);
+ virBufferAddLit(opt, ",");
+ }
}
return 0;
@@ -8137,7 +8149,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
/* OpenGL magic */
if (graphics->gl &&
graphics->gl->enable == VIR_TRISTATE_BOOL_YES &&
- qemuBuildGraphicsSPICEGLCommandLine(graphics->gl, &opt, qemuCaps) < 0)
+ qemuBuildGraphicsSPICEGLCommandLine(graphics->gl, cmd,
+ &opt, qemuCaps) < 0)
goto error;
virBufferTrim(&opt, ",", -1);
diff --git a/tests/qemuxml2argvdata/graphics-spice-gl-native.args b/tests/qemuxml2argvdata/graphics-spice-gl-native.args
new file mode 100644
index 0000000000..70d6694022
--- /dev/null
+++ b/tests/qemuxml2argvdata/graphics-spice-gl-native.args
@@ -0,0 +1,26 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=spice \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot c \
+-usb \
+-spice port=0,gl=on,rendernode=/dev/dri/foo \
+-vga cirrus \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/graphics-spice-gl-native.xml b/tests/qemuxml2argvdata/graphics-spice-gl-native.xml
new file mode 100644
index 0000000000..abfe628a9c
--- /dev/null
+++ b/tests/qemuxml2argvdata/graphics-spice-gl-native.xml
@@ -0,0 +1,25 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='spice' autoport='no'>
+ <listen type='none'/>
+ <gl enable='yes' native='yes' rendernode='/dev/dri/foo'/>
+ </graphics>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/graphics-spice-gl-non-native.args b/tests/qemuxml2argvdata/graphics-spice-gl-non-native.args
new file mode 100644
index 0000000000..9833cd6c0f
--- /dev/null
+++ b/tests/qemuxml2argvdata/graphics-spice-gl-non-native.args
@@ -0,0 +1,27 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=spice \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot c \
+-usb \
+-display egl-headless \
+-spice port=0 \
+-vga cirrus \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/graphics-spice-gl-non-native.xml b/tests/qemuxml2argvdata/graphics-spice-gl-non-native.xml
new file mode 100644
index 0000000000..003f2406ac
--- /dev/null
+++ b/tests/qemuxml2argvdata/graphics-spice-gl-non-native.xml
@@ -0,0 +1,24 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-i686</emulator>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='spice' autoport='no'>
+ <gl enable='yes' native='no'/>
+ </graphics>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c310349b57..e82496d403 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1256,6 +1256,14 @@ mymain(void)
QEMU_CAPS_SPICE_UNIX,
QEMU_CAPS_DEVICE_CIRRUS_VGA);
driver.config->spiceAutoUnixSocket = false;
+ DO_TEST("graphics-spice-gl-native",
+ QEMU_CAPS_SPICE,
+ QEMU_CAPS_SPICE_GL,
+ QEMU_CAPS_SPICE_RENDERNODE,
+ QEMU_CAPS_DEVICE_CIRRUS_VGA);
+ DO_TEST("graphics-spice-gl-non-native",
+ QEMU_CAPS_SPICE,
+ QEMU_CAPS_DEVICE_CIRRUS_VGA);
DO_TEST("input-usbmouse", NONE);
DO_TEST("input-usbtablet", NONE);
diff --git a/tests/qemuxml2xmloutdata/video-virtio-gpu-spice-gl.xml b/tests/qemuxml2xmloutdata/video-virtio-gpu-spice-gl.xml
index 720d362230..578341e09b 100644
--- a/tests/qemuxml2xmloutdata/video-virtio-gpu-spice-gl.xml
+++ b/tests/qemuxml2xmloutdata/video-virtio-gpu-spice-gl.xml
@@ -31,7 +31,7 @@
<input type='keyboard' bus='ps2'/>
<graphics type='spice'>
<listen type='none'/>
- <gl enable='yes' rendernode='/dev/dri/foo'/>
+ <gl enable='yes' native='yes' rendernode='/dev/dri/foo'/>
</graphics>
<video>
<model type='virtio' heads='1' primary='yes'>
--
2.14.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 06/27/2018 09:34 AM, Erik Skultety wrote:
> Since QEMU 2.10, there's a new cmdline option '-display egl-headless'
> which enables OpenGL support for cases where we can't render on a local
> display (i.e. <listen> is set to either 'address' or 'network').
> This is to work around the current restriction on the local node with
> native OpenGL SPICE support. However, compared to native support,
> egl-headless has overhead, because the rendered framebuffer has to be
> copied back to QEMU's display area which is then accessed by either
> SPICE or VNC and sent to the remote side.
blank line for readability
> This patch enables this configuration feature by introducing a new
> SPICE-only attribute 'native' which helps libvirt to determine whether
> to use '-spice gl=on' or '-display egl-headless' on the cmdline. If the
> attribute wasn't specified but OpenGL is to be enabled, then given the
> nature of the <listen> element, libvirt will determine the default
> value.
>
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
> docs/formatdomain.html.in | 8 ++++
> docs/schemas/domaincommon.rng | 16 +++++--
> src/conf/domain_conf.c | 49 ++++++++++++++++++++--
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 43 ++++++++++++-------
> .../qemuxml2argvdata/graphics-spice-gl-native.args | 26 ++++++++++++
> .../qemuxml2argvdata/graphics-spice-gl-native.xml | 25 +++++++++++
> .../graphics-spice-gl-non-native.args | 27 ++++++++++++
> .../graphics-spice-gl-non-native.xml | 24 +++++++++++
> tests/qemuxml2argvtest.c | 8 ++++
> .../video-virtio-gpu-spice-gl.xml | 2 +-
> 11 files changed, 206 insertions(+), 23 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-native.args
> create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-native.xml
> create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-non-native.args
> create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-non-native.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index aa0d6b26df..c6ebd39bd9 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6461,6 +6461,14 @@ qemu-kvm -net nic,model=? /dev/null
> You can enable or disable OpenGL support explicitly with
> the <code>gl</code> element, by setting the <code>enable</code>
> property. (QEMU only, <span class="since">since 1.3.3</span>).
> + Additionally, attribute <code>native</code>
> + (<span class="since">Since 4.6.0</span>) can be used to specify
> + whether native OpenGL support should be used with SPICE or
> + egl-headless should be used instead. Note that the native OpenGL
> + support is only limited to the local setup (<code>listen</code> is
> + either 'none' or 'socket') and for remote access egl-headless
> + needs to be used. The supported values for this attribute are 'on'
> + and 'off'.
> </p>
> <p>
> By default, QEMU will pick the first available GPU DRM render node.
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 20649c5f6f..4ad53d976b 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3066,6 +3066,11 @@
> <attribute name="enable">
> <ref name="virYesNo"/>
> </attribute>
> + <optional>
> + <attribute name="native">
> + <ref name="virYesNo"/>
> + </attribute>
> + </optional>
> <empty/>
> </element>
> </optional>
> @@ -3322,9 +3327,14 @@
> <ref name="absFilePath"/>
> </attribute>
> </optional>
> - <empty/>
> - </element>
> - </optional>
> + <optional>
> + <attribute name="native">
> + <ref name="virYesNo"/>
> + </attribute>
> + </optional>
> + <empty/>
> + </element>
> + </optional>
> </interleave>
> </group>
> <group>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 2ccd9e124f..6d399a198e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4280,6 +4280,7 @@ virDomainDefPostParseGraphics(virDomainDef *def)
> * same. */
> if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> virDomainGraphicsListenDefPtr glisten = &graphics->listens[0];
> + virDomainGraphicsGLDefPtr gl = graphics->gl;
>
> if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS &&
> graphics->data.spice.port == 0 &&
> @@ -4288,6 +4289,28 @@ virDomainDefPostParseGraphics(virDomainDef *def)
> VIR_FREE(glisten->address);
> glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE;
> }
> +
> + /* Next we need to figure out how to properly configure the OpenGL
> + * if that is enabled and the 'native' attribute is missing.
> + * The cases are:
> + * 1) Listen type is either 'socket' or 'none' - SPICE native
> + * OpenGL support (,gl=on) should be used because we're
> + * rendering on a local display.
> + *
> + * 2) Listen is either network or address - SPICE can't use
> + * the native OpenGL support remotely yet, so we use
> + * native='no' and format '-display egl-headless' onto the
> + * cmdline.
> + */
> + if (graphics->gl &&
> + graphics->gl->enable == VIR_TRISTATE_BOOL_YES &&
> + graphics->gl->native == VIR_TRISTATE_BOOL_ABSENT) {
> + gl->native = VIR_TRISTATE_BOOL_NO;
> +
> + if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE ||
> + glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET)
> + gl->native = VIR_TRISTATE_BOOL_YES;
> + }
So the upside/downside to setting something in the XML that wasn't
previously set by the consumer is that "we're stuck with it" going
forward, right? Additionally, we cannot ascertain whether we set native
to NO or YES or if the consumer did.
So the "other" side of this is this is being done in a domain_conf
function instead of some qemu device post parse function since if I'm
reading the RFC comments right, the desire is not to somehow
automagically add it, but rather force some usage.
Still, I wonder if what's been done follows the spirit of Gerd's comment:
"Hmm, I think it would be better to have egl-headless explicitly
configured in the domain xml instead of doing it automagically depending
on configuration."
could be turned into a "headless='yes|no|absent'" instead of native and
described as being necessary for specific conditions. Then perhaps a
qemu domain validation check to "force" the issue.
> }
> }
> }
> @@ -13573,6 +13596,7 @@ virDomainGraphicsGLDefParseXML(virDomainGraphicsDefPtr def,
> {
> virDomainGraphicsGLDefPtr gl = NULL;
> char *enable = NULL;
> + char *native = NULL;
> int ret = -1;
>
> if (!node)
> @@ -13596,14 +13620,26 @@ virDomainGraphicsGLDefParseXML(virDomainGraphicsDefPtr def,
> goto cleanup;
> }
>
> - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
> - gl->rendernode = virXMLPropString(node, "rendernode");
> + /* SPICE recognizes a few more attributes */
> + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> + gl->rendernode = virXMLPropString(node, "rendernode");
> +
> + native = virXMLPropString(node, "native");
> + if (native &&
> + (gl->native = virTristateBoolTypeFromString(native)) <= 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown value for attribute enable '%s'"),
> + enable);
I believe you wanted "s/enable/native" for 2 instances, copy pasta error
> + goto cleanup;
> + }
> + }
>
> VIR_STEAL_PTR(def->gl, gl);
>
> ret = 0;
> cleanup:
> VIR_FREE(enable);
> + VIR_FREE(native);
> virDomainGraphicsGLDefFree(gl);
> return ret;
> }
> @@ -26184,8 +26220,13 @@ virDomainGraphicsGLDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def)
> virBufferAsprintf(buf, "<gl enable='%s'",
> virTristateBoolTypeToString(gl->enable));
>
> - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
> - virBufferEscapeString(buf, " rendernode='%s'", gl->rendernode);
> + if (gl->enable == VIR_TRISTATE_BOOL_YES) {
Not clear why the YES guard was put in place. Not that "enable=no and
rendernode=xxx" means anything, but that would be dropped with this change.
Since rng/grammar using interleave, order of attributes shouldn't matter
should it?
> + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> + virBufferAsprintf(buf, " native='%s'",
> + virTristateBoolTypeToString(gl->native));
> + virBufferEscapeString(buf, " rendernode='%s'", gl->rendernode);
> + }
> + }
>
> virBufferAddLit(buf, "/>\n");
> }
Do you think we need ABI checks for this? Could be challenging given the
post parse setting of the value.
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 20dc1334c4..99b5896391 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1602,6 +1602,7 @@ typedef struct _virDomainGraphicsGLDef virDomainGraphicsGLDef;
> typedef virDomainGraphicsGLDef *virDomainGraphicsGLDefPtr;
> struct _virDomainGraphicsGLDef {
> virTristateBool enable;
> + virTristateBool native; /* -spice gl=on vs -display egl-headless for QEMU */
> char *rendernode; /* SPICE only */
> };
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 89a8408df6..fc80a6c3a6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7896,27 +7896,39 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg,
>
> static int
> qemuBuildGraphicsSPICEGLCommandLine(virDomainGraphicsGLDefPtr gl,
> + virCommandPtr cmd,
> virBufferPtr opt,
> virQEMUCapsPtr qemuCaps)
> {
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("This QEMU doesn't support spice OpenGL"));
> - return -1;
> - }
> -
> - virBufferAddLit(opt, "gl=on,");
> -
> - if (gl->rendernode) {
> - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) {
> + if (gl->native == VIR_TRISTATE_BOOL_NO) {
> + /* For non-native OpenGL, we need to add egl-headless to the cmdline.
> + *
> + * NB: QEMU defaults to '-spice gl=off', so we don't have to add that
> + * explicitly, especially since we're not testing for GL capability
> + * presence.
> + */
> + virCommandAddArg(cmd, "-display");
> + virCommandAddArg(cmd, "egl-headless");
Similar thoughts as previous patch related to egl-headless and need for
capability and/or details related to what's being used to gate it's
presence on the command line.
Honestly it's confusing to read "when" it's supposed to be used between
the new code and what was written in the RFC. There's a comment
regarding nvidia, non-opengl config for vfio, where it's not clear to me
whether headless could be used (IOW: some gl=no condition).
John
> + } else {
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL)) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("This QEMU doesn't support spice OpenGL rendernode"));
> + _("This QEMU doesn't support spice OpenGL"));
> return -1;
> }
>
> - virBufferAddLit(opt, "rendernode=");
> - virQEMUBuildBufferEscapeComma(opt, gl->rendernode);
> - virBufferAddLit(opt, ",");
> + virBufferAddLit(opt, "gl=on,");
> +
> + if (gl->rendernode) {
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_RENDERNODE)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("This QEMU doesn't support spice OpenGL rendernode"));
> + return -1;
> + }
> +
> + virBufferAddLit(opt, "rendernode=");
> + virQEMUBuildBufferEscapeComma(opt, gl->rendernode);
> + virBufferAddLit(opt, ",");
> + }
> }
>
> return 0;
> @@ -8137,7 +8149,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> /* OpenGL magic */
> if (graphics->gl &&
> graphics->gl->enable == VIR_TRISTATE_BOOL_YES &&
> - qemuBuildGraphicsSPICEGLCommandLine(graphics->gl, &opt, qemuCaps) < 0)
> + qemuBuildGraphicsSPICEGLCommandLine(graphics->gl, cmd,
> + &opt, qemuCaps) < 0)
> goto error;
>
> virBufferTrim(&opt, ",", -1);
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 28, 2018 at 06:34:56PM -0400, John Ferlan wrote:
>
>
> On 06/27/2018 09:34 AM, Erik Skultety wrote:
> > Since QEMU 2.10, there's a new cmdline option '-display egl-headless'
> > which enables OpenGL support for cases where we can't render on a local
> > display (i.e. <listen> is set to either 'address' or 'network').
> > This is to work around the current restriction on the local node with
> > native OpenGL SPICE support. However, compared to native support,
> > egl-headless has overhead, because the rendered framebuffer has to be
> > copied back to QEMU's display area which is then accessed by either
> > SPICE or VNC and sent to the remote side.
>
> blank line for readability
>
> > This patch enables this configuration feature by introducing a new
> > SPICE-only attribute 'native' which helps libvirt to determine whether
> > to use '-spice gl=on' or '-display egl-headless' on the cmdline. If the
> > attribute wasn't specified but OpenGL is to be enabled, then given the
> > nature of the <listen> element, libvirt will determine the default
> > value.
> >
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> > docs/formatdomain.html.in | 8 ++++
> > docs/schemas/domaincommon.rng | 16 +++++--
> > src/conf/domain_conf.c | 49 ++++++++++++++++++++--
> > src/conf/domain_conf.h | 1 +
> > src/qemu/qemu_command.c | 43 ++++++++++++-------
> > .../qemuxml2argvdata/graphics-spice-gl-native.args | 26 ++++++++++++
> > .../qemuxml2argvdata/graphics-spice-gl-native.xml | 25 +++++++++++
> > .../graphics-spice-gl-non-native.args | 27 ++++++++++++
> > .../graphics-spice-gl-non-native.xml | 24 +++++++++++
> > tests/qemuxml2argvtest.c | 8 ++++
> > .../video-virtio-gpu-spice-gl.xml | 2 +-
> > 11 files changed, 206 insertions(+), 23 deletions(-)
> > create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-native.args
> > create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-native.xml
> > create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-non-native.args
> > create mode 100644 tests/qemuxml2argvdata/graphics-spice-gl-non-native.xml
> >
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index aa0d6b26df..c6ebd39bd9 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -6461,6 +6461,14 @@ qemu-kvm -net nic,model=? /dev/null
> > You can enable or disable OpenGL support explicitly with
> > the <code>gl</code> element, by setting the <code>enable</code>
> > property. (QEMU only, <span class="since">since 1.3.3</span>).
> > + Additionally, attribute <code>native</code>
> > + (<span class="since">Since 4.6.0</span>) can be used to specify
> > + whether native OpenGL support should be used with SPICE or
> > + egl-headless should be used instead. Note that the native OpenGL
> > + support is only limited to the local setup (<code>listen</code> is
> > + either 'none' or 'socket') and for remote access egl-headless
> > + needs to be used. The supported values for this attribute are 'on'
> > + and 'off'.
> > </p>
> > <p>
> > By default, QEMU will pick the first available GPU DRM render node.
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index 20649c5f6f..4ad53d976b 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -3066,6 +3066,11 @@
> > <attribute name="enable">
> > <ref name="virYesNo"/>
> > </attribute>
> > + <optional>
> > + <attribute name="native">
> > + <ref name="virYesNo"/>
> > + </attribute>
> > + </optional>
> > <empty/>
> > </element>
> > </optional>
> > @@ -3322,9 +3327,14 @@
> > <ref name="absFilePath"/>
> > </attribute>
> > </optional>
> > - <empty/>
> > - </element>
> > - </optional>
> > + <optional>
> > + <attribute name="native">
> > + <ref name="virYesNo"/>
> > + </attribute>
> > + </optional>
> > + <empty/>
> > + </element>
> > + </optional>
> > </interleave>
> > </group>
> > <group>
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 2ccd9e124f..6d399a198e 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -4280,6 +4280,7 @@ virDomainDefPostParseGraphics(virDomainDef *def)
> > * same. */
> > if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> > virDomainGraphicsListenDefPtr glisten = &graphics->listens[0];
> > + virDomainGraphicsGLDefPtr gl = graphics->gl;
> >
> > if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS &&
> > graphics->data.spice.port == 0 &&
> > @@ -4288,6 +4289,28 @@ virDomainDefPostParseGraphics(virDomainDef *def)
> > VIR_FREE(glisten->address);
> > glisten->type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE;
> > }
> > +
> > + /* Next we need to figure out how to properly configure the OpenGL
> > + * if that is enabled and the 'native' attribute is missing.
> > + * The cases are:
> > + * 1) Listen type is either 'socket' or 'none' - SPICE native
> > + * OpenGL support (,gl=on) should be used because we're
> > + * rendering on a local display.
> > + *
> > + * 2) Listen is either network or address - SPICE can't use
> > + * the native OpenGL support remotely yet, so we use
> > + * native='no' and format '-display egl-headless' onto the
> > + * cmdline.
> > + */
> > + if (graphics->gl &&
> > + graphics->gl->enable == VIR_TRISTATE_BOOL_YES &&
> > + graphics->gl->native == VIR_TRISTATE_BOOL_ABSENT) {
> > + gl->native = VIR_TRISTATE_BOOL_NO;
> > +
> > + if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE ||
> > + glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET)
> > + gl->native = VIR_TRISTATE_BOOL_YES;
> > + }
>
> So the upside/downside to setting something in the XML that wasn't
> previously set by the consumer is that "we're stuck with it" going
> forward, right? Additionally, we cannot ascertain whether we set native
> to NO or YES or if the consumer did.
Does it matter? If libvirt set it, then we set it in a way that is safe for us,
IOW we can ensure the vm will start using that setting. If the consumer set it,
then we evaluate the setting and either fail or proceed with starting up the
vm.
>
> So the "other" side of this is this is being done in a domain_conf
> function instead of some qemu device post parse function since if I'm
> reading the RFC comments right, the desire is not to somehow
> automagically add it, but rather force some usage.
Correct, although, it's not that our "automagic" was wrong (in fact it was
safe), but it wasn't very flexible and 'egl-headless' can find use cases even
without mdevs, that was the main driver to expose it.
>
> Still, I wonder if what's been done follows the spirit of Gerd's comment:
>
> "Hmm, I think it would be better to have egl-headless explicitly
> configured in the domain xml instead of doing it automagically depending
> on configuration."
As I already explained in the cover letter reply, to me the facts that a
standalone graphics type='headless' doesn't make sense, is basically just some
kind of OpenGL switch and can't be combined with some of the other types felt
like a wrong way of doing things, that's why I merged it with the 'gl' element.
But okay, I'll give it a try on a separate branch and see how 'making it a
graphics type' goes. The one thing that won't be necessary and worth doing
anymore is separating the 'gl' data into a separate element, but I'm curious
how much code will be saved (if any) if done this way.
>
> could be turned into a "headless='yes|no|absent'" instead of native and
> described as being necessary for specific conditions. Then perhaps a
> qemu domain validation check to "force" the issue.
>
> > }
> > }
> > }
> > @@ -13573,6 +13596,7 @@ virDomainGraphicsGLDefParseXML(virDomainGraphicsDefPtr def,
> > {
> > virDomainGraphicsGLDefPtr gl = NULL;
> > char *enable = NULL;
> > + char *native = NULL;
> > int ret = -1;
> >
> > if (!node)
> > @@ -13596,14 +13620,26 @@ virDomainGraphicsGLDefParseXML(virDomainGraphicsDefPtr def,
> > goto cleanup;
> > }
> >
> > - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
> > - gl->rendernode = virXMLPropString(node, "rendernode");
> > + /* SPICE recognizes a few more attributes */
> > + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> > + gl->rendernode = virXMLPropString(node, "rendernode");
> > +
> > + native = virXMLPropString(node, "native");
> > + if (native &&
> > + (gl->native = virTristateBoolTypeFromString(native)) <= 0) {
> > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> > + _("unknown value for attribute enable '%s'"),
> > + enable);
>
> I believe you wanted "s/enable/native" for 2 instances, copy pasta error
>
> > + goto cleanup;
> > + }
> > + }
> >
> > VIR_STEAL_PTR(def->gl, gl);
> >
> > ret = 0;
> > cleanup:
> > VIR_FREE(enable);
> > + VIR_FREE(native);
> > virDomainGraphicsGLDefFree(gl);
> > return ret;
> > }
> > @@ -26184,8 +26220,13 @@ virDomainGraphicsGLDefFormat(virBufferPtr buf, virDomainGraphicsDefPtr def)
> > virBufferAsprintf(buf, "<gl enable='%s'",
> > virTristateBoolTypeToString(gl->enable));
> >
> > - if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE)
> > - virBufferEscapeString(buf, " rendernode='%s'", gl->rendernode);
> > + if (gl->enable == VIR_TRISTATE_BOOL_YES) {
>
> Not clear why the YES guard was put in place. Not that "enable=no and
> rendernode=xxx" means anything, but that would be dropped with this change.
>
> Since rng/grammar using interleave, order of attributes shouldn't matter
> should it?
>
> > + if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
> > + virBufferAsprintf(buf, " native='%s'",
> > + virTristateBoolTypeToString(gl->native));
> > + virBufferEscapeString(buf, " rendernode='%s'", gl->rendernode);
> > + }
> > + }
> >
> > virBufferAddLit(buf, "/>\n");
> > }
>
> Do you think we need ABI checks for this? Could be challenging given the
> post parse setting of the value.
I don't see an ABI problem here, old libvirt doesn't know what to do with it
(and VNC will just ignore the gl element) and new enough libvirt would ensure
that -spice gl=on and -display egl-headless will not appear on the cmdline.
Everything else is something that's been working for a very long time.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[...]
>>> + /* Next we need to figure out how to properly configure the OpenGL
>>> + * if that is enabled and the 'native' attribute is missing.
>>> + * The cases are:
>>> + * 1) Listen type is either 'socket' or 'none' - SPICE native
>>> + * OpenGL support (,gl=on) should be used because we're
>>> + * rendering on a local display.
>>> + *
>>> + * 2) Listen is either network or address - SPICE can't use
>>> + * the native OpenGL support remotely yet, so we use
>>> + * native='no' and format '-display egl-headless' onto the
>>> + * cmdline.
>>> + */
>>> + if (graphics->gl &&
>>> + graphics->gl->enable == VIR_TRISTATE_BOOL_YES &&
>>> + graphics->gl->native == VIR_TRISTATE_BOOL_ABSENT) {
>>> + gl->native = VIR_TRISTATE_BOOL_NO;
>>> +
>>> + if (glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NONE ||
>>> + glisten->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_SOCKET)
>>> + gl->native = VIR_TRISTATE_BOOL_YES;
>>> + }
>>
>> So the upside/downside to setting something in the XML that wasn't
>> previously set by the consumer is that "we're stuck with it" going
>> forward, right? Additionally, we cannot ascertain whether we set native
>> to NO or YES or if the consumer did.
>
> Does it matter? If libvirt set it, then we set it in a way that is safe for us,
> IOW we can ensure the vm will start using that setting. If the consumer set it,
> then we evaluate the setting and either fail or proceed with starting up the
> vm.
>
To me in particular, no it doesn't matter, but I note it because setting
a value however reasonable is something "of note". Does the domain
"work" without us setting that or does it fail to start? IOW: Are things
failing today/now without this being set?
>>
>> So the "other" side of this is this is being done in a domain_conf
>> function instead of some qemu device post parse function since if I'm
>> reading the RFC comments right, the desire is not to somehow
>> automagically add it, but rather force some usage.
>
> Correct, although, it's not that our "automagic" was wrong (in fact it was
> safe), but it wasn't very flexible and 'egl-headless' can find use cases even
> without mdevs, that was the main driver to expose it.
>
The relationship to mdev/hostdev is what I assumed in the long run this
was primarily geared towards. Having it without mdev would seem to be
sub-optimal especially w/ the required network component. It's taking
me a bit to figure this stuff out - you've been working with it, so I'm
trying to catch up.
>>
>> Still, I wonder if what's been done follows the spirit of Gerd's comment:
>>
>> "Hmm, I think it would be better to have egl-headless explicitly
>> configured in the domain xml instead of doing it automagically depending
>> on configuration."
>
> As I already explained in the cover letter reply, to me the facts that a
> standalone graphics type='headless' doesn't make sense, is basically just some
> kind of OpenGL switch and can't be combined with some of the other types felt
> like a wrong way of doing things, that's why I merged it with the 'gl' element.
> But okay, I'll give it a try on a separate branch and see how 'making it a
> graphics type' goes. The one thing that won't be necessary and worth doing
> anymore is separating the 'gl' data into a separate element, but I'm curious
> how much code will be saved (if any) if done this way.
>
Is it "fair" or "right" to say there's two ways to do gl support - one
where Spice/VNC is connected locally and one where it's connected
remotely. Then remotely is split into networked connection and
(eventually in subsequent patches) vGPU connection?
So you chose "native" to provide that functionality which gets some
automagic setting because we've determined there to be a need. Whether
that's because a domain fails to start or just is (well) brain dead is
what isn't clear to me.
Since we're not co-located - this is "my way" of talking through things
to "learn" and make sure what you've developed matches what someone
reading it "expected" as functionality. Automagic setting isn't
inherently wrong, but sometimes it needs to be carefully looked at to
make sure the reasons for doing it are "understandable".
[...]
>> Do you think we need ABI checks for this? Could be challenging given the
>> post parse setting of the value.
>
> I don't see an ABI problem here, old libvirt doesn't know what to do with it
> (and VNC will just ignore the gl element) and new enough libvirt would ensure
> that -spice gl=on and -display egl-headless will not appear on the cmdline.
> Everything else is something that's been working for a very long time.
>
I'm thinking about going forward. Once this is "in", if someone changes
the setting, then is that a problem ABI wise. You have a technology with
a network or vGPU connection that could change if the native changes.
Then of course there's the automagic setting. Of course there's no ABI
checking done yet on graphics - so I could be off in the weeds.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.