Support OpenGL accelerated rendering when using SDL graphics in the
domain config. Add associated test and documentation.
Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk>
---
docs/formatdomain.html.in | 6 +++
docs/schemas/domaincommon.rng | 8 ++++
src/conf/domain_conf.c | 44 ++++++++++++++++++++-
src/conf/domain_conf.h | 1 +
tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++
.../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
7 files changed, 141 insertions(+), 2 deletions(-)
create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml
create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index caeb14e2f..a7ef9269f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6162,6 +6162,12 @@ qemu-kvm -net nic,model=? /dev/null
and an optional <code>fullscreen</code> attribute accepting values
<code>yes</code> or <code>no</code>.
</p>
+
+ <p>
+ You can use a <code>gl</code> with the <code>enable="yes"</code>
+ property to enable OpenGL support in SDL. Likewise you can
+ explicitly disable OpenGL support with <code>enable="no"</code>.
+ </p>
</dd>
<dt><code>vnc</code></dt>
<dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 0a6b29b2f..c4c500a3c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3031,6 +3031,14 @@
<ref name="virYesNo"/>
</attribute>
</optional>
+ <optional>
+ <element name="gl">
+ <attribute name="enable">
+ <ref name="virYesNo"/>
+ </attribute>
+ <empty/>
+ </element>
+ </optional>
</group>
<group>
<attribute name="type">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f678e26b2..85bfa800b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13448,11 +13448,18 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
static int
virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
- xmlNodePtr node)
+ xmlNodePtr node,
+ xmlXPathContextPtr ctxt)
{
+ xmlNodePtr save = ctxt->node;
+ char *enable;
+ int enableVal;
+ xmlNodePtr glNode;
char *fullscreen = virXMLPropString(node, "fullscreen");
int ret = -1;
+ ctxt->node = node;
+
if (fullscreen != NULL) {
if (STREQ(fullscreen, "yes")) {
def->data.sdl.fullscreen = true;
@@ -13470,9 +13477,30 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
def->data.sdl.xauth = virXMLPropString(node, "xauth");
def->data.sdl.display = virXMLPropString(node, "display");
+ glNode = virXPathNode("./gl", ctxt);
+ if (glNode) {
+ enable = virXMLPropString(glNode, "enable");
+ if (!enable) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("sdl gl element missing enable"));
+ goto cleanup;
+ }
+
+ enableVal = virTristateBoolTypeFromString(enable);
+ if (enableVal < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown enable value '%s'"), enable);
+ VIR_FREE(enable);
+ goto cleanup;
+ }
+ VIR_FREE(enable);
+ def->data.sdl.gl = enableVal;
+ }
+
ret = 0;
cleanup:
VIR_FREE(fullscreen);
+ ctxt->node = save;
return ret;
}
@@ -13901,7 +13929,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
goto error;
break;
case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
- if (virDomainGraphicsDefParseXMLSDL(def, node) < 0)
+ if (virDomainGraphicsDefParseXMLSDL(def, node, ctxt) < 0)
goto error;
break;
case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
@@ -25654,6 +25682,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
if (def->data.sdl.fullscreen)
virBufferAddLit(buf, " fullscreen='yes'");
+ if (!children && def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) {
+ virBufferAddLit(buf, ">\n");
+ virBufferAdjustIndent(buf, 2);
+ children = true;
+ }
+
+ if (def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) {
+ virBufferAsprintf(buf, "<gl enable='%s'",
+ virTristateBoolTypeToString(def->data.sdl.gl));
+ virBufferAddLit(buf, "/>\n");
+ }
+
break;
case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 15d228ba9..517278989 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1599,6 +1599,7 @@ struct _virDomainGraphicsDef {
char *display;
char *xauth;
bool fullscreen;
+ virTristateBool gl;
} sdl;
struct {
int port;
diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml b/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml
new file mode 100644
index 000000000..9dea73fbe
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml
@@ -0,0 +1,38 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</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>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2' cache='none'/>
+ <source file='/var/lib/libvirt/images/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='ide' index='0'/>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='sdl'>
+ <gl enable='yes'/>
+ </graphics>
+ <video>
+ <model type='virtio' heads='1'>
+ <acceleration accel3d='yes'/>
+ </model>
+ </video>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml b/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
new file mode 100644
index 000000000..da03dd5da
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
@@ -0,0 +1,45 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>1048576</memory>
+ <currentMemory unit='KiB'>1048576</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>
+ <disk type='file' device='disk'>
+ <driver name='qemu' type='qcow2' cache='none'/>
+ <source file='/var/lib/libvirt/images/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ </disk>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <graphics type='sdl'>
+ <gl enable='yes'/>
+ </graphics>
+ <video>
+ <model type='virtio' heads='1' primary='yes'>
+ <acceleration accel3d='yes'/>
+ </model>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
+ </video>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 53a26a0c3..9c4f8054e 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1096,6 +1096,7 @@ mymain(void)
DO_TEST("video-virtio-gpu-device", NONE);
DO_TEST("video-virtio-gpu-virgl", NONE);
DO_TEST("video-virtio-gpu-spice-gl", NONE);
+ DO_TEST("video-virtio-gpu-sdl-gl", NONE);
DO_TEST("virtio-input", NONE);
DO_TEST("virtio-input-passthrough", NONE);
--
2.11.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/10/2018 06:53 AM, Maciej Wolny wrote:
> Support OpenGL accelerated rendering when using SDL graphics in the
> domain config. Add associated test and documentation.
>
> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk>
> ---
> docs/formatdomain.html.in | 6 +++
> docs/schemas/domaincommon.rng | 8 ++++
> src/conf/domain_conf.c | 44 ++++++++++++++++++++-
> src/conf/domain_conf.h | 1 +
> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++
> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++
> tests/qemuxml2xmltest.c | 1 +
> 7 files changed, 141 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml
> create mode 100644 tests/qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml
>
[...]
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13448,11 +13448,18 @@ virDomainGraphicsDefParseXMLVNC(virDomainGraphicsDefPtr def,
>
> static int
> virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
> - xmlNodePtr node)
> + xmlNodePtr node,
> + xmlXPathContextPtr ctxt)
> {
> + xmlNodePtr save = ctxt->node;
> + char *enable;
Initialize = NULL, then...
> + int enableVal;
> + xmlNodePtr glNode;
> char *fullscreen = virXMLPropString(node, "fullscreen");
> int ret = -1;
>
> + ctxt->node = node;
> +
> if (fullscreen != NULL) {
> if (STREQ(fullscreen, "yes")) {
> def->data.sdl.fullscreen = true;
> @@ -13470,9 +13477,30 @@ virDomainGraphicsDefParseXMLSDL(virDomainGraphicsDefPtr def,
> def->data.sdl.xauth = virXMLPropString(node, "xauth");
> def->data.sdl.display = virXMLPropString(node, "display");
>
> + glNode = virXPathNode("./gl", ctxt);
> + if (glNode) {
> + enable = virXMLPropString(glNode, "enable");
> + if (!enable) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("sdl gl element missing enable"));
> + goto cleanup;
> + }
> +
> + enableVal = virTristateBoolTypeFromString(enable);
> + if (enableVal < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unknown enable value '%s'"), enable);
> + VIR_FREE(enable);
Move the VIR_FREE(enable) into cleanup and remove the one 3 lines later.
> + goto cleanup;
> + }
> + VIR_FREE(enable);
> + def->data.sdl.gl = enableVal;
> + }
> +
> ret = 0;
> cleanup:
> VIR_FREE(fullscreen);
> + ctxt->node = save;
> return ret;
> }
>
> @@ -13901,7 +13929,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
> goto error;
> break;
> case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> - if (virDomainGraphicsDefParseXMLSDL(def, node) < 0)
> + if (virDomainGraphicsDefParseXMLSDL(def, node, ctxt) < 0)
> goto error;
> break;
> case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> @@ -25654,6 +25682,18 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
> if (def->data.sdl.fullscreen)
> virBufferAddLit(buf, " fullscreen='yes'");
>
> + if (!children && def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) {
Well I certainly hope it cannot be true at this point; otherwise, the
compiler reorganized things on us ;-)... I'll leave it as is since it's
a bit of preventive coding...
> + virBufferAddLit(buf, ">\n");
> + virBufferAdjustIndent(buf, 2);
> + children = true;
> + }
> +
> + if (def->data.sdl.gl != VIR_TRISTATE_BOOL_ABSENT) {
> + virBufferAsprintf(buf, "<gl enable='%s'",
> + virTristateBoolTypeToString(def->data.sdl.gl));
> + virBufferAddLit(buf, "/>\n");
> + }
> +
> break;
>
I'll make the above adjustment to enable processing before pushing
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >Support OpenGL accelerated rendering when using SDL graphics in the >domain config. Add associated test and documentation. > >Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >--- > docs/formatdomain.html.in | 6 +++ > docs/schemas/domaincommon.rng | 8 ++++ > src/conf/domain_conf.c | 44 ++++++++++++++++++++- > src/conf/domain_conf.h | 1 + docs, conf and schemas fit together nicely, they should be in one patch, but. > tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ > .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ > tests/qemuxml2xmltest.c | 1 + this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:') For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/05/18 09:42, Martin Kletzander wrote: > On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >> Support OpenGL accelerated rendering when using SDL graphics in the >> domain config. Add associated test and documentation. >> >> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >> --- >> docs/formatdomain.html.in | 6 +++ >> docs/schemas/domaincommon.rng | 8 ++++ >> src/conf/domain_conf.c | 44 ++++++++++++++++++++- >> src/conf/domain_conf.h | 1 + > > docs, conf and schemas fit together nicely, they should be in one patch, but. > >> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ >> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ >> tests/qemuxml2xmltest.c | 1 + > > this has nothing to do with qemu (yet), also see Subject (I wouldn't say > 'qemu:' there, but rather something like 'docs, conf, schema:') > > For the XML tests above you can use genericxml2xmltest instead of the > QEMU-specific one. The option only makes sense in QEMU afaik, hence the naming. -- milloni -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >On 11/05/18 09:42, Martin Kletzander wrote: >> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>> Support OpenGL accelerated rendering when using SDL graphics in the >>> domain config. Add associated test and documentation. >>> >>> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >>> --- >>> docs/formatdomain.html.in | 6 +++ >>> docs/schemas/domaincommon.rng | 8 ++++ >>> src/conf/domain_conf.c | 44 ++++++++++++++++++++- >>> src/conf/domain_conf.h | 1 + >> >> docs, conf and schemas fit together nicely, they should be in one patch, but. >> >>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++++++++++++++++++ >>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++++++++++++++++++++++ >>> tests/qemuxml2xmltest.c | 1 + >> >> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >> 'qemu:' there, but rather something like 'docs, conf, schema:') >> >> For the XML tests above you can use genericxml2xmltest instead of the >> QEMU-specific one. > >The option only makes sense in QEMU afaik, hence the naming. > Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML. I hope that's clear. Have a nice day.-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 05/14/2018 07:24 AM, Martin Kletzander wrote: > On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >> On 11/05/18 09:42, Martin Kletzander wrote: >>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>>> Support OpenGL accelerated rendering when using SDL graphics in the >>>> domain config. Add associated test and documentation. >>>> >>>> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >>>> --- >>>> docs/formatdomain.html.in | 6 +++ >>>> docs/schemas/domaincommon.rng | 8 ++++ >>>> src/conf/domain_conf.c | 44 >>>> ++++++++++++++++++++- >>>> src/conf/domain_conf.h | 1 + >>> >>> docs, conf and schemas fit together nicely, they should be in one >>> patch, but. >>> >>>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>>> ++++++++++++++++++ >>>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>>> ++++++++++++++++++++++ >>>> tests/qemuxml2xmltest.c | 1 + >>> >>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>> >>> For the XML tests above you can use genericxml2xmltest instead of the >>> QEMU-specific one. >> >> The option only makes sense in QEMU afaik, hence the naming. >> > > Yes, for now. If someone who's building the code without QEMU driver > changes > the behaviour, the tests will pass if you keep it in qemuxml2xml, however > genericxml2xml will catch that. qemuxml2xml should be testing specifics > where > you behave based on some more information than just generic XML. > > I hope that's clear. > > Have a nice day. > However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired? Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". Could/should generification of the xml2xml tests be considered a "bite sized task"? John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: > > >On 05/14/2018 07:24 AM, Martin Kletzander wrote: >> On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >>> On 11/05/18 09:42, Martin Kletzander wrote: >>>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>>>> Support OpenGL accelerated rendering when using SDL graphics in the >>>>> domain config. Add associated test and documentation. >>>>> >>>>> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >>>>> --- >>>>> docs/formatdomain.html.in | 6 +++ >>>>> docs/schemas/domaincommon.rng | 8 ++++ >>>>> src/conf/domain_conf.c | 44 >>>>> ++++++++++++++++++++- >>>>> src/conf/domain_conf.h | 1 + >>>> >>>> docs, conf and schemas fit together nicely, they should be in one >>>> patch, but. >>>> >>>>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>>>> ++++++++++++++++++ >>>>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>>>> ++++++++++++++++++++++ >>>>> tests/qemuxml2xmltest.c | 1 + >>>> >>>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>>> >>>> For the XML tests above you can use genericxml2xmltest instead of the >>>> QEMU-specific one. >>> >>> The option only makes sense in QEMU afaik, hence the naming. >>> >> >> Yes, for now. If someone who's building the code without QEMU driver >> changes >> the behaviour, the tests will pass if you keep it in qemuxml2xml, however >> genericxml2xml will catch that. qemuxml2xml should be testing specifics >> where >> you behave based on some more information than just generic XML. >> >> I hope that's clear. >> >> Have a nice day. >> > >However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >then you'd have separate xml input and output files - is that what's >desired? > >Taking a quick look just now - see the graphics-vnc-socket - do we want >to duplicate having two input/output XML files which invariably will >diverge? Ironically the generic one has a domain type == qemu, an >emulator using qemu, and the socket path using QEMU - so while it's >generic in one sense, it's not in others. Even more ironic is the qemu >specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". > >Could/should generification of the xml2xml tests be considered a "bite >sized task"? > Oh, definitely. It's only some time ago that the tests started to be usable IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others could chime in as well so that I don't speak for others. I remember Pavel having some ideas for cleaner separation of those.-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 14/05/18 13:40, Martin Kletzander wrote: > On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: >> >> >> On 05/14/2018 07:24 AM, Martin Kletzander wrote: >>> On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >>>> On 11/05/18 09:42, Martin Kletzander wrote: >>>>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>>>>> Support OpenGL accelerated rendering when using SDL graphics in the >>>>>> domain config. Add associated test and documentation. >>>>>> >>>>>> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >>>>>> --- >>>>>> docs/formatdomain.html.in | 6 +++ >>>>>> docs/schemas/domaincommon.rng | 8 ++++ >>>>>> src/conf/domain_conf.c | 44 >>>>>> ++++++++++++++++++++- >>>>>> src/conf/domain_conf.h | 1 + >>>>> >>>>> docs, conf and schemas fit together nicely, they should be in one >>>>> patch, but. >>>>> >>>>>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>>>>> ++++++++++++++++++ >>>>>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>>>>> ++++++++++++++++++++++ >>>>>> tests/qemuxml2xmltest.c | 1 + >>>>> >>>>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>>>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>>>> >>>>> For the XML tests above you can use genericxml2xmltest instead of the >>>>> QEMU-specific one. >>>> >>>> The option only makes sense in QEMU afaik, hence the naming. >>>> >>> >>> Yes, for now. If someone who's building the code without QEMU driver >>> changes >>> the behaviour, the tests will pass if you keep it in qemuxml2xml, however >>> genericxml2xml will catch that. qemuxml2xml should be testing specifics >>> where >>> you behave based on some more information than just generic XML. >>> >>> I hope that's clear. >>> >>> Have a nice day. >>> >> >> However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >> then you'd have separate xml input and output files - is that what's >> desired? >> >> Taking a quick look just now - see the graphics-vnc-socket - do we want >> to duplicate having two input/output XML files which invariably will >> diverge? Ironically the generic one has a domain type == qemu, an >> emulator using qemu, and the socket path using QEMU - so while it's >> generic in one sense, it's not in others. Even more ironic is the qemu >> specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". >> >> Could/should generification of the xml2xml tests be considered a "bite >> sized task"? >> > > Oh, definitely. It's only some time ago that the tests started to be usable > IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others > could chime in as well so that I don't speak for others. I remember Pavel > having some ideas for cleaner separation of those. So, do you guys want to leave that for a separate patch set or do you want me to post a v3 with the changes Martin has requested? -- milloni -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 05/14/2018 09:18 AM, Maciej Wolny wrote: > On 14/05/18 13:40, Martin Kletzander wrote: >> On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: >>> >>> >>> On 05/14/2018 07:24 AM, Martin Kletzander wrote: >>>> On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >>>>> On 11/05/18 09:42, Martin Kletzander wrote: >>>>>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>>>>>> Support OpenGL accelerated rendering when using SDL graphics in the >>>>>>> domain config. Add associated test and documentation. >>>>>>> >>>>>>> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >>>>>>> --- >>>>>>> docs/formatdomain.html.in | 6 +++ >>>>>>> docs/schemas/domaincommon.rng | 8 ++++ >>>>>>> src/conf/domain_conf.c | 44 >>>>>>> ++++++++++++++++++++- >>>>>>> src/conf/domain_conf.h | 1 + >>>>>> >>>>>> docs, conf and schemas fit together nicely, they should be in one >>>>>> patch, but. >>>>>> >>>>>>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>>>>>> ++++++++++++++++++ >>>>>>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>>>>>> ++++++++++++++++++++++ >>>>>>> tests/qemuxml2xmltest.c | 1 + >>>>>> >>>>>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>>>>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>>>>> >>>>>> For the XML tests above you can use genericxml2xmltest instead of the >>>>>> QEMU-specific one. >>>>> >>>>> The option only makes sense in QEMU afaik, hence the naming. >>>>> >>>> >>>> Yes, for now. If someone who's building the code without QEMU driver >>>> changes >>>> the behaviour, the tests will pass if you keep it in qemuxml2xml, however >>>> genericxml2xml will catch that. qemuxml2xml should be testing specifics >>>> where >>>> you behave based on some more information than just generic XML. >>>> >>>> I hope that's clear. >>>> >>>> Have a nice day. >>>> >>> >>> However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >>> then you'd have separate xml input and output files - is that what's >>> desired? >>> >>> Taking a quick look just now - see the graphics-vnc-socket - do we want >>> to duplicate having two input/output XML files which invariably will >>> diverge? Ironically the generic one has a domain type == qemu, an >>> emulator using qemu, and the socket path using QEMU - so while it's >>> generic in one sense, it's not in others. Even more ironic is the qemu >>> specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". >>> >>> Could/should generification of the xml2xml tests be considered a "bite >>> sized task"? >>> >> >> Oh, definitely. It's only some time ago that the tests started to be usable >> IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others >> could chime in as well so that I don't speak for others. I remember Pavel >> having some ideas for cleaner separation of those. > > So, do you guys want to leave that for a separate patch set or do you want me to > post a v3 with the changes Martin has requested? > My opinion is leave it as is - hard to require you to do something we're not requiring other patches at this point. I'm not a fan of duplication - that is the task would be to essentially copy everything into the generic xml2xml test at this point. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, May 14, 2018 at 09:21:58AM -0400, John Ferlan wrote: > > >On 05/14/2018 09:18 AM, Maciej Wolny wrote: >> On 14/05/18 13:40, Martin Kletzander wrote: >>> On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: >>>> >>>> >>>> On 05/14/2018 07:24 AM, Martin Kletzander wrote: >>>>> On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >>>>>> On 11/05/18 09:42, Martin Kletzander wrote: >>>>>>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>>>>>>> Support OpenGL accelerated rendering when using SDL graphics in the >>>>>>>> domain config. Add associated test and documentation. >>>>>>>> >>>>>>>> Signed-off-by: Maciej Wolny <maciej.wolny@codethink.co.uk> >>>>>>>> --- >>>>>>>> docs/formatdomain.html.in | 6 +++ >>>>>>>> docs/schemas/domaincommon.rng | 8 ++++ >>>>>>>> src/conf/domain_conf.c | 44 >>>>>>>> ++++++++++++++++++++- >>>>>>>> src/conf/domain_conf.h | 1 + >>>>>>> >>>>>>> docs, conf and schemas fit together nicely, they should be in one >>>>>>> patch, but. >>>>>>> >>>>>>>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>>>>>>> ++++++++++++++++++ >>>>>>>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>>>>>>> ++++++++++++++++++++++ >>>>>>>> tests/qemuxml2xmltest.c | 1 + >>>>>>> >>>>>>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>>>>>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>>>>>> >>>>>>> For the XML tests above you can use genericxml2xmltest instead of the >>>>>>> QEMU-specific one. >>>>>> >>>>>> The option only makes sense in QEMU afaik, hence the naming. >>>>>> >>>>> >>>>> Yes, for now. If someone who's building the code without QEMU driver >>>>> changes >>>>> the behaviour, the tests will pass if you keep it in qemuxml2xml, however >>>>> genericxml2xml will catch that. qemuxml2xml should be testing specifics >>>>> where >>>>> you behave based on some more information than just generic XML. >>>>> >>>>> I hope that's clear. >>>>> >>>>> Have a nice day. >>>>> >>>> >>>> However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >>>> then you'd have separate xml input and output files - is that what's >>>> desired? >>>> >>>> Taking a quick look just now - see the graphics-vnc-socket - do we want >>>> to duplicate having two input/output XML files which invariably will >>>> diverge? Ironically the generic one has a domain type == qemu, an >>>> emulator using qemu, and the socket path using QEMU - so while it's >>>> generic in one sense, it's not in others. Even more ironic is the qemu >>>> specific file uses "<graphics type='vnc' socket='/tmp/foo.socket'>". >>>> >>>> Could/should generification of the xml2xml tests be considered a "bite >>>> sized task"? >>>> >>> >>> Oh, definitely. It's only some time ago that the tests started to be usable >>> IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others >>> could chime in as well so that I don't speak for others. I remember Pavel >>> having some ideas for cleaner separation of those. >> >> So, do you guys want to leave that for a separate patch set or do you want me to >> post a v3 with the changes Martin has requested? >> > >My opinion is leave it as is - hard to require you to do something we're >not requiring other patches at this point. I'm not a fan of duplication >- that is the task would be to essentially copy everything into the >generic xml2xml test at this point. > Well, I got reminded several times about this, but it is poosible that that was off-list (IRC or privately). I'm fine with leaving it as is. Mainly since there are soo many of similar ones from the past. So sorry for blocking you with this tiny, unimportant request. >John > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.