7:49 p.m.
This extends domain capabilities to report some bits apps will want
to determine if spice GL is supported:
<domainCapabilities>
<devices>
<video supported='yes'>
<enum name='modelType'/>
<enum name='virtioAccel3D'/>
</video>
<graphics supported='yes'>
<enum name='type'/>
<enum name='spiceGL'/>
</graphics>
</devices>
</domainCapabilities>
Cole Robinson (5):
domaincaps: Report graphics type enum
domaincaps: Report video modelType
domaincaps: Report graphics spiceGL
qemu: command: unconditionally allow accel3d='no'
domaincaps: Report graphics spiceGL
docs/formatdomaincaps.html.in | 76 +++++++++++++++++++++-
docs/schemas/domaincaps.rng | 16 +++++
src/conf/domain_capabilities.c | 28 ++++++++
src/conf/domain_capabilities.h | 21 ++++++
src/qemu/qemu_capabilities.c | 53 ++++++++++++++-
src/qemu/qemu_command.c | 2 +-
tests/domaincapsschemadata/domaincaps-basic.xml | 2 +
tests/domaincapsschemadata/domaincaps-full.xml | 31 +++++++++
.../domaincaps-qemu_1.6.50-1.xml | 21 ++++++
.../domaincaps-qemu_2.6.0-1.xml | 24 +++++++
.../domaincaps-qemu_2.6.0-2.xml | 19 ++++++
.../domaincaps-qemu_2.6.0-3.xml | 19 ++++++
.../domaincaps-qemu_2.6.0-4.xml | 19 ++++++
.../domaincaps-qemu_2.6.0-5.xml | 19 ++++++
tests/domaincapstest.c | 10 +++
15 files changed, 357 insertions(+), 3 deletions(-)
--
2.7.4
7:49 p.m.
New subject: [libvirt] [PATCH 1/5] domaincaps: Report graphics type enum
Requires adding the plumbing for <device><graphics>
Wire it up for qemu too
---
I stuck the <graphics> element between <disk> and <hostdev> to match
the ordering in <domain> XML
docs/formatdomaincaps.html.in | 30 +++++++++++++++++++++-
docs/schemas/domaincaps.rng | 8 ++++++
src/conf/domain_capabilities.c | 13 ++++++++++
src/conf/domain_capabilities.h | 8 ++++++
src/qemu/qemu_capabilities.c | 22 +++++++++++++++-
tests/domaincapsschemadata/domaincaps-basic.xml | 1 +
tests/domaincapsschemadata/domaincaps-full.xml | 9 +++++++
.../domaincaps-qemu_1.6.50-1.xml | 7 +++++
.../domaincaps-qemu_2.6.0-1.xml | 7 +++++
.../domaincaps-qemu_2.6.0-2.xml | 6 +++++
.../domaincaps-qemu_2.6.0-3.xml | 6 +++++
.../domaincaps-qemu_2.6.0-4.xml | 6 +++++
.../domaincaps-qemu_2.6.0-5.xml | 6 +++++
tests/domaincapstest.c | 4 +++
14 files changed, 131 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index edbb948..52e4463 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -175,7 +175,7 @@
<code>floppy</code>, or <code>lun</code>.</p>
<h4><a name="elementsDisks">Hard drives, floppy disks,
CDROMs</a></h4>
- <p>Disk capabilities are exposed under <code>disk</code> element.
For
+ <p>Disk capabilities are exposed under the <code>disk</code>
element. For
instance:</p>
<pre>
@@ -216,6 +216,34 @@
element for a <disk/>.</dd>
</dl>
+
+ <h4><a name="elementsGraphics">Graphical
framebuffers</a></h4>
+ <p>Graphics device capabilities are exposed under the
+ <code>graphics</code> element. For instance:</p>
+
+<pre>
+<domainCapabilities>
+ ...
+ <devices>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ <value>spice</value>
+ </enum>
+ </graphics>
+ ...
+ </devices>
+</domainCapabilities>
+</pre>
+
+ <dl>
+ <dt><code>type</code></dt>
+ <dd>Options for the <code>type</code> attribute of the
<graphics/>
+ element.</dd>
+ </dl>
+
+
<h4><a name="elementsHostDev">Host device
assignment</a></h4>
<p>Some host devices can be passed through to a guest (e.g. USB, PCI and
SCSI). Well, only if the following is enabled:</p>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 0d2777b..3e82b57 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -72,6 +72,7 @@
<element name='devices'>
<interleave>
<ref name='disk'/>
+ <ref name='graphics'/>
<ref name='hostdev'/>
</interleave>
</element>
@@ -84,6 +85,13 @@
</element>
</define>
+ <define name='graphics'>
+ <element name='graphics'>
+ <ref name='supported'/>
+ <ref name='enum'/>
+ </element>
+ </define>
+
<define name='hostdev'>
<element name='hostdev'>
<ref name='supported'/>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index eb880ae..232acd5 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -247,6 +247,18 @@ virDomainCapsDeviceDiskFormat(virBufferPtr buf,
static void
+virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
+ virDomainCapsDeviceGraphicsPtr const graphics)
+{
+ FORMAT_PROLOGUE(graphics);
+
+ ENUM_PROCESS(graphics, type, virDomainGraphicsTypeToString);
+
+ FORMAT_EPILOGUE(graphics);
+}
+
+
+static void
virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
virDomainCapsDeviceHostdevPtr const hostdev)
{
@@ -314,6 +326,7 @@ virDomainCapsFormatInternal(virBufferPtr buf,
virBufferAdjustIndent(buf, 2);
virDomainCapsDeviceDiskFormat(buf, &caps->disk);
+ virDomainCapsDeviceGraphicsFormat(buf, &caps->graphics);
virDomainCapsDeviceHostdevFormat(buf, &caps->hostdev);
virBufferAdjustIndent(buf, -2);
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 95afe5e..545ada7 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -69,6 +69,13 @@ struct _virDomainCapsDeviceDisk {
/* add new fields here */
};
+typedef struct _virDomainCapsDeviceGraphics virDomainCapsDeviceGraphics;
+typedef virDomainCapsDeviceGraphics *virDomainCapsDeviceGraphicsPtr;
+struct _virDomainCapsDeviceGraphics {
+ bool supported;
+ virDomainCapsEnum type; /* virDomainGraphicsType */
+};
+
typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
typedef virDomainCapsDeviceHostdev *virDomainCapsDeviceHostdevPtr;
struct _virDomainCapsDeviceHostdev {
@@ -101,6 +108,7 @@ struct _virDomainCaps {
virDomainCapsOS os;
virDomainCapsDeviceDisk disk;
+ virDomainCapsDeviceGraphics graphics;
virDomainCapsDeviceHostdev hostdev;
/* add new domain devices here */
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 30dc33a..c675f9f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4171,6 +4171,23 @@ virQEMUCapsFillDomainDeviceDiskCaps(virQEMUCapsPtr qemuCaps,
static int
+virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUCapsPtr qemuCaps,
+ virDomainCapsDeviceGraphicsPtr dev)
+{
+ dev->supported = true;
+
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SDL))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_SDL);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_VNC);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_SPICE);
+
+ return 0;
+}
+
+
+static int
virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
virDomainCapsDeviceHostdevPtr hostdev)
{
@@ -4281,13 +4298,16 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
virDomainCapsOSPtr os = &domCaps->os;
virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev;
+ virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics;
int maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine);
domCaps->maxvcpus = maxvcpus;
if (virQEMUCapsFillDomainOSCaps(qemuCaps, os,
loader, nloader) < 0 ||
- virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, domCaps->machine, disk) < 0
||
+ virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps,
+ domCaps->machine, disk) < 0 ||
+ virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics) < 0 ||
virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
return -1;
diff --git a/tests/domaincapsschemadata/domaincaps-basic.xml
b/tests/domaincapsschemadata/domaincaps-basic.xml
index 6587d56..f0f0864 100644
--- a/tests/domaincapsschemadata/domaincaps-basic.xml
+++ b/tests/domaincapsschemadata/domaincaps-basic.xml
@@ -6,6 +6,7 @@
<os supported='no'/>
<devices>
<disk supported='no'/>
+ <graphics supported='no'/>
<hostdev supported='no'/>
</devices>
<features>
diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
index d4f91fa..b3b8855 100644
--- a/tests/domaincapsschemadata/domaincaps-full.xml
+++ b/tests/domaincapsschemadata/domaincaps-full.xml
@@ -39,6 +39,15 @@
<value>sd</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ <value>rdp</value>
+ <value>desktop</value>
+ <value>spice</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
index 1e73ff1..147424d 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
@@ -34,6 +34,13 @@
<value>usb</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ <value>spice</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
index 244700a..f8f7465 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
@@ -34,6 +34,13 @@
<value>usb</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ <value>spice</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
index 411c6b7..7913703 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
@@ -34,6 +34,12 @@
<value>usb</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
index 21c59a8..6f30819 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
@@ -34,6 +34,12 @@
<value>usb</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
index c972d5e..6845e92 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
@@ -34,6 +34,12 @@
<value>usb</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
index e4b8c3a..68d88d1 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
@@ -32,6 +32,12 @@
<value>usb</value>
</enum>
</disk>
+ <graphics supported='yes'>
+ <enum name='type'>
+ <value>sdl</value>
+ <value>vnc</value>
+ </enum>
+ </graphics>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index ee78555..6bef682 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -61,6 +61,7 @@ fillAllCaps(virDomainCapsPtr domCaps)
virDomainCapsOSPtr os = &domCaps->os;
virDomainCapsLoaderPtr loader = &os->loader;
virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
+ virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics;
virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev;
domCaps->maxvcpus = 255;
@@ -79,6 +80,9 @@ fillAllCaps(virDomainCapsPtr domCaps)
SET_ALL_BITS(disk->diskDevice);
SET_ALL_BITS(disk->bus);
+ graphics->supported = true;
+ SET_ALL_BITS(graphics->type);
+
hostdev->supported = true;
SET_ALL_BITS(hostdev->mode);
SET_ALL_BITS(hostdev->startupPolicy);
--
2.7.4
11:27 a.m.
New subject: [libvirt] [PATCH 1/5] domaincaps: Report graphics type enum
On 08.05.2016 19:49, Cole Robinson wrote:
Requires adding the plumbing for <device><graphics>
Wire it up for qemu too
---
I stuck the <graphics> element between <disk> and <hostdev> to match
the ordering in <domain> XML
docs/formatdomaincaps.html.in | 30 +++++++++++++++++++++-
docs/schemas/domaincaps.rng | 8 ++++++
src/conf/domain_capabilities.c | 13 ++++++++++
src/conf/domain_capabilities.h | 8 ++++++
src/qemu/qemu_capabilities.c | 22 +++++++++++++++-
tests/domaincapsschemadata/domaincaps-basic.xml | 1 +
tests/domaincapsschemadata/domaincaps-full.xml | 9 +++++++
.../domaincaps-qemu_1.6.50-1.xml | 7 +++++
.../domaincaps-qemu_2.6.0-1.xml | 7 +++++
.../domaincaps-qemu_2.6.0-2.xml | 6 +++++
.../domaincaps-qemu_2.6.0-3.xml | 6 +++++
.../domaincaps-qemu_2.6.0-4.xml | 6 +++++
.../domaincaps-qemu_2.6.0-5.xml | 6 +++++
tests/domaincapstest.c | 4 +++
14 files changed, 131 insertions(+), 2 deletions(-)
ACK
Michal
7:49 p.m.
New subject: [libvirt] [PATCH 2/5] domaincaps: Report video modelType
Requires adding the plumbing for <device><video>
The value is <enum name='modelType'> to match the associated domain
XML of <video><model type='XXX'/>
Wire it up for qemu too
---
docs/formatdomaincaps.html.in | 29 ++++++++++++++++++++++
docs/schemas/domaincaps.rng | 8 ++++++
src/conf/domain_capabilities.c | 13 ++++++++++
src/conf/domain_capabilities.h | 10 ++++++++
src/qemu/qemu_capabilities.c | 23 +++++++++++++++++
tests/domaincapsschemadata/domaincaps-basic.xml | 1 +
tests/domaincapsschemadata/domaincaps-full.xml | 12 +++++++++
.../domaincaps-qemu_1.6.50-1.xml | 8 ++++++
.../domaincaps-qemu_2.6.0-1.xml | 9 +++++++
.../domaincaps-qemu_2.6.0-2.xml | 7 ++++++
.../domaincaps-qemu_2.6.0-3.xml | 7 ++++++
.../domaincaps-qemu_2.6.0-4.xml | 7 ++++++
.../domaincaps-qemu_2.6.0-5.xml | 7 ++++++
tests/domaincapstest.c | 4 +++
14 files changed, 145 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 52e4463..d5a8414 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -244,6 +244,35 @@
</dl>
+ <h4><a name="elementsVideo">Video device</a></h4>
+ <p>Video device capabilities are exposed under the
+ <code>video</code> element. For instance:</p>
+
+<pre>
+<domainCapabilities>
+ ...
+ <devices>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>cirrus</value>
+ <value>vmvga</value>
+ <value>qxl</value>
+ <value>virtio</value>
+ </enum>
+ </video>
+ ...
+ </devices>
+</domainCapabilities>
+</pre>
+
+ <dl>
+ <dt><code>modelType</code></dt>
+ <dd>Options for the <code>type</code> attribute of the
+ <video><model> element.</dd>
+ </dl>
+
+
<h4><a name="elementsHostDev">Host device
assignment</a></h4>
<p>Some host devices can be passed through to a guest (e.g. USB, PCI and
SCSI). Well, only if the following is enabled:</p>
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index 3e82b57..97da41f 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -73,6 +73,7 @@
<interleave>
<ref name='disk'/>
<ref name='graphics'/>
+ <ref name='video'/>
<ref name='hostdev'/>
</interleave>
</element>
@@ -92,6 +93,13 @@
</element>
</define>
+ <define name='video'>
+ <element name='video'>
+ <ref name='supported'/>
+ <ref name='enum'/>
+ </element>
+ </define>
+
<define name='hostdev'>
<element name='hostdev'>
<ref name='supported'/>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 232acd5..1676f0e 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -259,6 +259,18 @@ virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
static void
+virDomainCapsDeviceVideoFormat(virBufferPtr buf,
+ virDomainCapsDeviceVideoPtr const video)
+{
+ FORMAT_PROLOGUE(video);
+
+ ENUM_PROCESS(video, modelType, virDomainVideoTypeToString);
+
+ FORMAT_EPILOGUE(video);
+}
+
+
+static void
virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
virDomainCapsDeviceHostdevPtr const hostdev)
{
@@ -327,6 +339,7 @@ virDomainCapsFormatInternal(virBufferPtr buf,
virDomainCapsDeviceDiskFormat(buf, &caps->disk);
virDomainCapsDeviceGraphicsFormat(buf, &caps->graphics);
+ virDomainCapsDeviceVideoFormat(buf, &caps->video);
virDomainCapsDeviceHostdevFormat(buf, &caps->hostdev);
virBufferAdjustIndent(buf, -2);
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 545ada7..d0ca009 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -76,6 +76,15 @@ struct _virDomainCapsDeviceGraphics {
virDomainCapsEnum type; /* virDomainGraphicsType */
};
+typedef struct _virDomainCapsDeviceVideo virDomainCapsDeviceVideo;
+typedef virDomainCapsDeviceVideo *virDomainCapsDeviceVideoPtr;
+struct _virDomainCapsDeviceVideo {
+ bool supported;
+ virDomainCapsEnum modelType; /* virDomainVideoType */
+};
+
+typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
+
typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
typedef virDomainCapsDeviceHostdev *virDomainCapsDeviceHostdevPtr;
struct _virDomainCapsDeviceHostdev {
@@ -109,6 +118,7 @@ struct _virDomainCaps {
virDomainCapsOS os;
virDomainCapsDeviceDisk disk;
virDomainCapsDeviceGraphics graphics;
+ virDomainCapsDeviceVideo video;
virDomainCapsDeviceHostdev hostdev;
/* add new domain devices here */
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c675f9f..1bddf43 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4188,6 +4188,27 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUCapsPtr qemuCaps,
static int
+virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps,
+ virDomainCapsDeviceVideoPtr dev)
+{
+ dev->supported = true;
+
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_VGA);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_CIRRUS);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_VMVGA);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_QXL);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_VIRTIO);
+
+ return 0;
+}
+
+
+static int
virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
virDomainCapsDeviceHostdevPtr hostdev)
{
@@ -4299,6 +4320,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev;
virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics;
+ virDomainCapsDeviceVideoPtr video = &domCaps->video;
int maxvcpus = virQEMUCapsGetMachineMaxCpus(qemuCaps, domCaps->machine);
domCaps->maxvcpus = maxvcpus;
@@ -4308,6 +4330,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps,
domCaps->machine, disk) < 0 ||
virQEMUCapsFillDomainDeviceGraphicsCaps(qemuCaps, graphics) < 0 ||
+ virQEMUCapsFillDomainDeviceVideoCaps(qemuCaps, video) < 0 ||
virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0 ||
virQEMUCapsFillDomainFeatureGICCaps(qemuCaps, domCaps) < 0)
return -1;
diff --git a/tests/domaincapsschemadata/domaincaps-basic.xml
b/tests/domaincapsschemadata/domaincaps-basic.xml
index f0f0864..5513f99 100644
--- a/tests/domaincapsschemadata/domaincaps-basic.xml
+++ b/tests/domaincapsschemadata/domaincaps-basic.xml
@@ -7,6 +7,7 @@
<devices>
<disk supported='no'/>
<graphics supported='no'/>
+ <video supported='no'/>
<hostdev supported='no'/>
</devices>
<features>
diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
index b3b8855..2f529ff 100644
--- a/tests/domaincapsschemadata/domaincaps-full.xml
+++ b/tests/domaincapsschemadata/domaincaps-full.xml
@@ -48,6 +48,18 @@
<value>spice</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>cirrus</value>
+ <value>vmvga</value>
+ <value>xen</value>
+ <value>vbox</value>
+ <value>qxl</value>
+ <value>parallels</value>
+ <value>virtio</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
index 147424d..161d0ab 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
@@ -41,6 +41,14 @@
<value>spice</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>cirrus</value>
+ <value>vmvga</value>
+ <value>qxl</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
index f8f7465..f42d239 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
@@ -41,6 +41,15 @@
<value>spice</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>cirrus</value>
+ <value>vmvga</value>
+ <value>qxl</value>
+ <value>virtio</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
index 7913703..4e87cd2 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
@@ -40,6 +40,13 @@
<value>vnc</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>qxl</value>
+ <value>virtio</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
index 6f30819..f5f0f1c 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
@@ -40,6 +40,13 @@
<value>vnc</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>qxl</value>
+ <value>virtio</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
index 6845e92..1ae8172 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
@@ -40,6 +40,13 @@
<value>vnc</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>qxl</value>
+ <value>virtio</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
index 68d88d1..583fdf0 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
@@ -38,6 +38,13 @@
<value>vnc</value>
</enum>
</graphics>
+ <video supported='yes'>
+ <enum name='modelType'>
+ <value>vga</value>
+ <value>qxl</value>
+ <value>virtio</value>
+ </enum>
+ </video>
<hostdev supported='yes'>
<enum name='mode'>
<value>subsystem</value>
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 6bef682..6ae3f35 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -62,6 +62,7 @@ fillAllCaps(virDomainCapsPtr domCaps)
virDomainCapsLoaderPtr loader = &os->loader;
virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
virDomainCapsDeviceGraphicsPtr graphics = &domCaps->graphics;
+ virDomainCapsDeviceVideoPtr video = &domCaps->video;
virDomainCapsDeviceHostdevPtr hostdev = &domCaps->hostdev;
domCaps->maxvcpus = 255;
@@ -83,6 +84,9 @@ fillAllCaps(virDomainCapsPtr domCaps)
graphics->supported = true;
SET_ALL_BITS(graphics->type);
+ video->supported = true;
+ SET_ALL_BITS(video->modelType);
+
hostdev->supported = true;
SET_ALL_BITS(hostdev->mode);
SET_ALL_BITS(hostdev->startupPolicy);
--
2.7.4
11:27 a.m.
New subject: [libvirt] [PATCH 2/5] domaincaps: Report video modelType
On 08.05.2016 19:49, Cole Robinson wrote:
Requires adding the plumbing for <device><video>
The value is <enum name='modelType'> to match the associated domain
XML of <video><model type='XXX'/>
Wire it up for qemu too
---
docs/formatdomaincaps.html.in | 29 ++++++++++++++++++++++
docs/schemas/domaincaps.rng | 8 ++++++
src/conf/domain_capabilities.c | 13 ++++++++++
src/conf/domain_capabilities.h | 10 ++++++++
src/qemu/qemu_capabilities.c | 23 +++++++++++++++++
tests/domaincapsschemadata/domaincaps-basic.xml | 1 +
tests/domaincapsschemadata/domaincaps-full.xml | 12 +++++++++
.../domaincaps-qemu_1.6.50-1.xml | 8 ++++++
.../domaincaps-qemu_2.6.0-1.xml | 9 +++++++
.../domaincaps-qemu_2.6.0-2.xml | 7 ++++++
.../domaincaps-qemu_2.6.0-3.xml | 7 ++++++
.../domaincaps-qemu_2.6.0-4.xml | 7 ++++++
.../domaincaps-qemu_2.6.0-5.xml | 7 ++++++
tests/domaincapstest.c | 4 +++
14 files changed, 145 insertions(+)
ACK
Michal
7:49 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
Reports a tristate enum value for acceptable graphics type=spice
<gl enable=XXX/>.
Wire it up for qemu too. 'no' is always a valid value, so we
unconditionally report it.
---
docs/formatdomaincaps.html.in | 8 ++++++++
src/conf/domain_capabilities.c | 1 +
src/conf/domain_capabilities.h | 1 +
src/qemu/qemu_capabilities.c | 4 ++++
tests/domaincapsschemadata/domaincaps-full.xml | 5 +++++
tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml | 4 ++++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml | 3 +++
tests/domaincapstest.c | 1 +
12 files changed, 39 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index d5a8414..c424107 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -231,6 +231,10 @@
<value>vnc</value>
<value>spice</value>
</enum>
+ <enum name='spiceGL'>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</graphics>
...
</devices>
@@ -241,6 +245,10 @@
<dt><code>type</code></dt>
<dd>Options for the <code>type</code> attribute of the
<graphics/>
element.</dd>
+
+ <dt><code>spiceGL</code></dt>
+ <dd>Options for the <code>enable</code> attribute of the
+ <graphics type='spice'><gl/>
element.</dd>
</dl>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 1676f0e..344955c 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -253,6 +253,7 @@ virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
FORMAT_PROLOGUE(graphics);
ENUM_PROCESS(graphics, type, virDomainGraphicsTypeToString);
+ ENUM_PROCESS(graphics, spiceGL, virTristateBoolTypeToString);
FORMAT_EPILOGUE(graphics);
}
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index d0ca009..916dba0 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -74,6 +74,7 @@ typedef virDomainCapsDeviceGraphics *virDomainCapsDeviceGraphicsPtr;
struct _virDomainCapsDeviceGraphics {
bool supported;
virDomainCapsEnum type; /* virDomainGraphicsType */
+ virDomainCapsEnum spiceGL; /* type=spice <gl enable=X/> tristate */
};
typedef struct _virDomainCapsDeviceVideo virDomainCapsDeviceVideo;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 1bddf43..f228f4f 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4183,6 +4183,10 @@ virQEMUCapsFillDomainDeviceGraphicsCaps(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE))
VIR_DOMAIN_CAPS_ENUM_SET(dev->type, VIR_DOMAIN_GRAPHICS_TYPE_SPICE);
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->spiceGL, VIR_TRISTATE_BOOL_NO);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPICE_GL))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->spiceGL, VIR_TRISTATE_BOOL_YES);
+
return 0;
}
diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
index 2f529ff..4eb5637 100644
--- a/tests/domaincapsschemadata/domaincaps-full.xml
+++ b/tests/domaincapsschemadata/domaincaps-full.xml
@@ -47,6 +47,11 @@
<value>desktop</value>
<value>spice</value>
</enum>
+ <enum name='spiceGL'>
+ <value>default</value>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
index 161d0ab..6213297 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
@@ -40,6 +40,9 @@
<value>vnc</value>
<value>spice</value>
</enum>
+ <enum name='spiceGL'>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
index f42d239..2a1477f 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
@@ -40,6 +40,10 @@
<value>vnc</value>
<value>spice</value>
</enum>
+ <enum name='spiceGL'>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
index 4e87cd2..4349998 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
@@ -39,6 +39,9 @@
<value>sdl</value>
<value>vnc</value>
</enum>
+ <enum name='spiceGL'>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
index f5f0f1c..27173c4 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
@@ -39,6 +39,9 @@
<value>sdl</value>
<value>vnc</value>
</enum>
+ <enum name='spiceGL'>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
index 1ae8172..d7d81b5 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
@@ -39,6 +39,9 @@
<value>sdl</value>
<value>vnc</value>
</enum>
+ <enum name='spiceGL'>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
index 583fdf0..51c5615 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
@@ -37,6 +37,9 @@
<value>sdl</value>
<value>vnc</value>
</enum>
+ <enum name='spiceGL'>
+ <value>no</value>
+ </enum>
</graphics>
<video supported='yes'>
<enum name='modelType'>
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 6ae3f35..1b62781 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -83,6 +83,7 @@ fillAllCaps(virDomainCapsPtr domCaps)
graphics->supported = true;
SET_ALL_BITS(graphics->type);
+ SET_ALL_BITS(graphics->spiceGL);
video->supported = true;
SET_ALL_BITS(video->modelType);
--
2.7.4
11:26 a.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 08.05.2016 19:49, Cole Robinson wrote:
Reports a tristate enum value for acceptable graphics type=spice
<gl enable=XXX/>.
This ^^ is part of graphics element as follows:
<graphics>
<gl enable='yes'/>
</graphics> ..
Wire it up for qemu too. 'no' is always a valid value, so we
unconditionally report it.
---
docs/formatdomaincaps.html.in | 8 ++++++++
src/conf/domain_capabilities.c | 1 +
src/conf/domain_capabilities.h | 1 +
src/qemu/qemu_capabilities.c | 4 ++++
tests/domaincapsschemadata/domaincaps-full.xml | 5 +++++
tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml | 4 ++++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml | 3 +++
tests/domaincapstest.c | 1 +
12 files changed, 39 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index d5a8414..c424107 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -231,6 +231,10 @@
<value>vnc</value>
<value>spice</value>
</enum>
+ <enum name='spiceGL'>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</graphics>
.. and while it's true that just Spice supports OpenGL currently, I'd
vote for a less specific naming. BTW: wasn't virgl implemented for SDL
initially? I know we don't support it, but my point is, what if XYZ
graphics type gains support for something equivalent to virgl? Would we
end up having yet another 'xyzGL' enum? Well, on the other hand, we
already have pciBackend enum to hostdev elem where we report VFIO/KVM
caps for hostdev[@type='pci']/driver/@name.
So I guess it's your call whether to change this or not. The only name I
can suggest right now (if you're out of ideas) is just 'gl'.
Otherwise the code looks good.
Michal
3:35 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 05/09/2016 05:26 AM, Michal Privoznik wrote:
On 08.05.2016 19:49, Cole Robinson wrote:
> Reports a tristate enum value for acceptable graphics type=spice
> <gl enable=XXX/>.
This ^^ is part of graphics element as follows:
<graphics>
<gl enable='yes'/>
</graphics> ..
>
> Wire it up for qemu too. 'no' is always a valid value, so we
> unconditionally report it.
> ---
> docs/formatdomaincaps.html.in | 8 ++++++++
> src/conf/domain_capabilities.c | 1 +
> src/conf/domain_capabilities.h | 1 +
> src/qemu/qemu_capabilities.c | 4 ++++
> tests/domaincapsschemadata/domaincaps-full.xml | 5 +++++
> tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml | 4 ++++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml | 3 +++
> tests/domaincapstest.c | 1 +
> 12 files changed, 39 insertions(+)
>
> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> index d5a8414..c424107 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -231,6 +231,10 @@
> <value>vnc</value>
> <value>spice</value>
> </enum>
> + <enum name='spiceGL'>
> + <value>yes</value>
> + <value>no</value>
> + </enum>
> </graphics>
.. and while it's true that just Spice supports OpenGL currently, I'd
vote for a less specific naming. BTW: wasn't virgl implemented for SDL
initially? I know we don't support it, but my point is, what if XYZ
graphics type gains support for something equivalent to virgl? Would we
end up having yet another 'xyzGL' enum? Well, on the other hand, we
already have pciBackend enum to hostdev elem where we report VFIO/KVM
caps for hostdev[@type='pci']/driver/@name.
So I guess it's your call whether to change this or not. The only name I
can suggest right now (if you're out of ideas) is just 'gl'.
I think just reporting <enum name='gl'/> gives insufficient info to apps.
What
if hypothetically VNC starts support 3D... then apps have no way of
distinguishing which libvirt+qemu combos support spice + 3D vs the new VNC +
3D, and they are stuck in the current position of trying to guess what is
supported by checking libvirt version numbers.
So IMO the domcaps need to report this fine grained value some how. Either
this way or the XML suggested by Pavel, please give your thoughts there as well
Thanks,
Cole
1:30 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Sun, May 08, 2016 at 01:49:06PM -0400, Cole Robinson wrote:
Reports a tristate enum value for acceptable graphics type=spice
<gl enable=XXX/>.
Wire it up for qemu too. 'no' is always a valid value, so we
unconditionally report it.
---
[...]
diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
index 2f529ff..4eb5637 100644
--- a/tests/domaincapsschemadata/domaincaps-full.xml
+++ b/tests/domaincapsschemadata/domaincaps-full.xml
@@ -47,6 +47,11 @@
<value>desktop</value>
<value>spice</value>
</enum>
+ <enum name='spiceGL'>
+ <value>default</value>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
Ewww, this doesn't look good and I agree with Michal that what if we add
support for another graphics device and what if one graphics device will have
more than one capability?
I would suggest something like this:
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
<type name='spice'>
<gl supported='yes'/>
</type>
</graphics>
or even do something like this:
<graphics supported='yes'>
<type name='spice'>
<gl supported='yes'/>
</type>
<type name='vnc'/>
<type name='sdl'/>
</graphics>
Pavel
3:30 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 05/09/2016 07:30 AM, Pavel Hrdina wrote:
On Sun, May 08, 2016 at 01:49:06PM -0400, Cole Robinson wrote:
> Reports a tristate enum value for acceptable graphics type=spice
> <gl enable=XXX/>.
>
> Wire it up for qemu too. 'no' is always a valid value, so we
> unconditionally report it.
> ---
[...]
> diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
> index 2f529ff..4eb5637 100644
> --- a/tests/domaincapsschemadata/domaincaps-full.xml
> +++ b/tests/domaincapsschemadata/domaincaps-full.xml
> @@ -47,6 +47,11 @@
> <value>desktop</value>
> <value>spice</value>
> </enum>
> + <enum name='spiceGL'>
> + <value>default</value>
> + <value>yes</value>
> + <value>no</value>
> + </enum>
Ewww, this doesn't look good and I agree with Michal that what if we add
support for another graphics device and what if one graphics device will have
more than one capability?
I would suggest something like this:
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
<type name='spice'>
<gl supported='yes'/>
</type>
</graphics>
or even do something like this:
<graphics supported='yes'>
<type name='spice'>
<gl supported='yes'/>
</type>
<type name='vnc'/>
<type name='sdl'/>
</graphics>
Really wish someone would have chimed in with this to my (unresponded) RFC
email about these things...
http://www.redhat.com/archives/libvir-list/2016-April/msg01839.html
That pattern is fine in this case, but what about the case of ports= element
that is only supported with a controller type=usb model=nec-xhci ? Do we have
new XML like:
<controller supported='yes'>
<type name='usb'>
<model name='nec-xhci'/>
<param name='ports' supported='yes'/>
</model>
</type>
</controller>
It seems like these types of relationships could grow deeply nested, so I
opted for following the limited existing convention of adding unique enum
names to represent the relationship.
I'm not really opposed to the nesting, but I'd like others on the list to
state their preference before I rework this
Thanks,
Cole
3:41 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Mon, May 09, 2016 at 09:30:30 -0400, Cole Robinson wrote:
On 05/09/2016 07:30 AM, Pavel Hrdina wrote:
> On Sun, May 08, 2016 at 01:49:06PM -0400, Cole Robinson wrote:
Really wish someone would have chimed in with this to my (unresponded) RFC
email about these things...
http://www.redhat.com/archives/libvir-list/2016-April/msg01839.html
That pattern is fine in this case, but what about the case of ports= element
that is only supported with a controller type=usb model=nec-xhci ? Do we have
new XML like:
<controller supported='yes'>
<type name='usb'>
<model name='nec-xhci'/>
<param name='ports' supported='yes'/>
</model>
</type>
</controller>
It seems like these types of relationships could grow deeply nested, so I
opted for following the limited existing convention of adding unique enum
names to represent the relationship.
Neither of those is really human friendly, thus I prefer aggregating the
information to avoid having to collate them from snippets out of place.
3:48 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Mon, May 09, 2016 at 09:30:30AM -0400, Cole Robinson wrote:
On 05/09/2016 07:30 AM, Pavel Hrdina wrote:
> On Sun, May 08, 2016 at 01:49:06PM -0400, Cole Robinson wrote:
>> Reports a tristate enum value for acceptable graphics type=spice
>> <gl enable=XXX/>.
>>
>> Wire it up for qemu too. 'no' is always a valid value, so we
>> unconditionally report it.
>> ---
>
> [...]
>
>> diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
>> index 2f529ff..4eb5637 100644
>> --- a/tests/domaincapsschemadata/domaincaps-full.xml
>> +++ b/tests/domaincapsschemadata/domaincaps-full.xml
>> @@ -47,6 +47,11 @@
>> <value>desktop</value>
>> <value>spice</value>
>> </enum>
>> + <enum name='spiceGL'>
>> + <value>default</value>
>> + <value>yes</value>
>> + <value>no</value>
>> + </enum>
>
> Ewww, this doesn't look good and I agree with Michal that what if we add
> support for another graphics device and what if one graphics device will have
> more than one capability?
>
> I would suggest something like this:
>
> <graphics supported='yes'>
> <enum name='type'>
> <value>spice</value>
> <value>vnc</value>
> <value>sdl</value>
> </enum>
> <type name='spice'>
> <gl supported='yes'/>
> </type>
> </graphics>
>
> or even do something like this:
>
> <graphics supported='yes'>
> <type name='spice'>
> <gl supported='yes'/>
> </type>
> <type name='vnc'/>
> <type name='sdl'/>
> </graphics>
>
Really wish someone would have chimed in with this to my (unresponded) RFC
email about these things...
http://www.redhat.com/archives/libvir-list/2016-April/msg01839.html
That pattern is fine in this case, but what about the case of ports= element
that is only supported with a controller type=usb model=nec-xhci ? Do we have
new XML like:
<controller supported='yes'>
<type name='usb'>
<model name='nec-xhci'/>
<param name='ports' supported='yes'/>
</model>
</type>
</controller>
It seems like these types of relationships could grow deeply nested, so I
opted for following the limited existing convention of adding unique enum
names to represent the relationship.
Yeah, I find flat modelling quite desirable, because the relationships
between attributes will certainly grow quite complicate, and they do
not neccessarily form a nice simple tree relationship.
ie, whether foo=bar is permitted may depend on whether wizz=eek *AND*
when lala=ooh, and you can't have the enum for 'foo' nested underneath
the enum for 'wizz' & 'lala' at the same time. Also with nesting, if
you want the same values reported for multiple options, we end up
repeating the same information multiple times which is not good.
At the same time the flat modelling with "spiceGL" also doesn't scale
as you have to invent new names for each one, and again it doesn't
work if the 'gl' enum depended on type="spice" *and* something else
at the same time.
So I think we need a more flexible way to express dependancies here.
We could let the <enum> be repeated multiple times and express conditional
rules against each instance of the enum
So for example
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
<enum name='gl'>
<condition>
<enum name="type" value="spice">
</condition>
<value>default</value>
<value>yes</value>
<value>no</value>
</enum>
<enum name='gl'>
<value>default</value>
<value>no</value>
</enum>
</graphics>
This would express that the first <enum type="gl"> entry only applies
when the @type == spice. If that doesn't match them fallback to the
second <enum type="gl">. The nice thing about this is that we can
make the conditions arbitrarly complex
For example:
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
<enum name='gl'>
<condition op="and">
<enum name="type" value="spice">
<condition op="or">
<enum name="type" value="vnc">
<enum name="wibble" value="eek">
</condition>
</condition>
<value>default</value>
<value>yes</value>
<value>no</value>
</enum>
<enum name='gl'>
<value>default</value>
<value>no</value>
</enum>
</graphics>
This shows how the "gl" option can be used with VNC, but only if you
also have the @wibble attribute set to "eek".
Of course complex conditions like this become quite tedious & verbose
so obviously we should strive to keep them as simple as possible and
not use nested conditions unless absolutely needed.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:53 a.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
On Mon, May 09, 2016 at 09:30:30AM -0400, Cole Robinson wrote:
> On 05/09/2016 07:30 AM, Pavel Hrdina wrote:
>> On Sun, May 08, 2016 at 01:49:06PM -0400, Cole Robinson wrote:
>>> Reports a tristate enum value for acceptable graphics type=spice
>>> <gl enable=XXX/>.
>>>
>>> Wire it up for qemu too. 'no' is always a valid value, so we
>>> unconditionally report it.
>>> ---
>>
>> [...]
>>
>>> diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
>>> index 2f529ff..4eb5637 100644
>>> --- a/tests/domaincapsschemadata/domaincaps-full.xml
>>> +++ b/tests/domaincapsschemadata/domaincaps-full.xml
>>> @@ -47,6 +47,11 @@
>>> <value>desktop</value>
>>> <value>spice</value>
>>> </enum>
>>> + <enum name='spiceGL'>
>>> + <value>default</value>
>>> + <value>yes</value>
>>> + <value>no</value>
>>> + </enum>
>>
>> Ewww, this doesn't look good and I agree with Michal that what if we add
>> support for another graphics device and what if one graphics device will have
>> more than one capability?
>>
>> I would suggest something like this:
>>
>> <graphics supported='yes'>
>> <enum name='type'>
>> <value>spice</value>
>> <value>vnc</value>
>> <value>sdl</value>
>> </enum>
>> <type name='spice'>
>> <gl supported='yes'/>
>> </type>
>> </graphics>
>>
>> or even do something like this:
>>
>> <graphics supported='yes'>
>> <type name='spice'>
>> <gl supported='yes'/>
>> </type>
>> <type name='vnc'/>
>> <type name='sdl'/>
>> </graphics>
>>
>
> Really wish someone would have chimed in with this to my (unresponded) RFC
> email about these things...
>
> http://www.redhat.com/archives/libvir-list/2016-April/msg01839.html
>
> That pattern is fine in this case, but what about the case of ports= element
> that is only supported with a controller type=usb model=nec-xhci ? Do we have
> new XML like:
>
> <controller supported='yes'>
> <type name='usb'>
> <model name='nec-xhci'/>
> <param name='ports' supported='yes'/>
> </model>
> </type>
> </controller>
>
> It seems like these types of relationships could grow deeply nested, so I
> opted for following the limited existing convention of adding unique enum
> names to represent the relationship.
Yeah, I find flat modelling quite desirable, because the relationships
between attributes will certainly grow quite complicate, and they do
not neccessarily form a nice simple tree relationship.
ie, whether foo=bar is permitted may depend on whether wizz=eek *AND*
when lala=ooh, and you can't have the enum for 'foo' nested underneath
the enum for 'wizz' & 'lala' at the same time. Also with nesting,
if
you want the same values reported for multiple options, we end up
repeating the same information multiple times which is not good.
At the same time the flat modelling with "spiceGL" also doesn't scale
as you have to invent new names for each one, and again it doesn't
work if the 'gl' enum depended on type="spice" *and* something else
at the same time.
So I think we need a more flexible way to express dependancies here.
We could let the <enum> be repeated multiple times and express conditional
rules against each instance of the enum
So for example
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
<enum name='gl'>
<condition>
<enum name="type" value="spice">
</condition>
<value>default</value>
<value>yes</value>
<value>no</value>
</enum>
<enum name='gl'>
<value>default</value>
<value>no</value>
</enum>
</graphics>
This would express that the first <enum type="gl"> entry only applies
when the @type == spice. If that doesn't match them fallback to the
second <enum type="gl">. The nice thing about this is that we can
make the conditions arbitrarly complex
For example:
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
<enum name='gl'>
<condition op="and">
<enum name="type" value="spice">
<condition op="or">
<enum name="type" value="vnc">
<enum name="wibble" value="eek">
</condition>
</condition>
<value>default</value>
<value>yes</value>
<value>no</value>
</enum>
<enum name='gl'>
<value>default</value>
<value>no</value>
</enum>
</graphics>
This shows how the "gl" option can be used with VNC, but only if you
also have the @wibble attribute set to "eek".
Of course complex conditions like this become quite tedious & verbose
so obviously we should strive to keep them as simple as possible and
not use nested conditions unless absolutely needed.
Writing a parser for this type of XML, that maintains behavior in a future
proof way, is going to be a lot of work. Hypothetical example:
<video supported='yes'>
<enum name='modelType'/>
<value>cirrus</value>
<value>qxl</value>
</enum>
<enum name='accel2d'/>
<value>no</value>
</enum>
</video>
6 months later, qemu gets some accel2d support for model=qxl
<video supported='yes'>
<enum name='modelType'/>
<value>cirrus</value>
<value>qxl</value>
</enum>
<enum name='accel2d'/>
<value>no</value>
</enum>
<enum name='accel2d'/>
<condition>
<enum name="modelType" value="qxl">
</condition>
<value>yes</value>
<value>no</value>
</enum>
</video>
Another 6 months later, qemu gets accel2d support for cirrus only, but it
requires <address type='pci'/> and doesn't work for <address
type='isa'/>
<video supported='yes'>
<enum name='modelType'/>
<value>cirrus</value>
<value>qxl</value>
</enum>
<enum name='addressType'/>
<value>pci</value>
<value>isa</value>
</enum>
<enum name='accel2d'/>
<value>no</value>
</enum>
<enum name='accel2d'/>
<condition>
<enum name="modelType" value="qxl">
</condition>
<value>yes</value>
<value>no</value>
</enum>
<enum name='accel2d'/>
<condition>
<enum name="modelType" value="cirrus">
<enum name="addressType" value="pci">
</condition>
<value>yes</value>
<value>no</value>
</enum>
</video>
The app wants to answer the question 'can I specify accel2d by default for
model=cirrus?' (note, cirrus, and not qxl)
For the first example, the code parses the accel2d enum, doesn't see 'yes',
so
it doesn't enable accel2d. The code will need to contain some knowledge that
enum name='accel2d' == ./video/acceleration/@accel2d but that's presently
unavoidable.
For the second example, code will need to be added to parse the basic
condition, see that modelType=cirrus (./video/model/@type) doesn't match the
condition, and so again accel2d=no
For the third example, code hits the modelType=cirrus condition, but then also
needs to check that addressType (./video/address/@type) == pci, and if so,
sets accel2d=yes
So, that's all achievable. But what kind of parser does the app write at each
step that may fall over with the next XML iteration?
What if the parser in step 1 doesn't expect multiple <enum
name='accel2d'/> ?
If it naively parses only the first instance, then nothing will regress in
step #2 or step #3. step #3's conclusion will be wrong, but that's okay. Note
this depends on libvirt being smart about appending conditional enums, and not
putting them first.
However if it parses the last instance, such as by iterating over every XML
node and taking the last one that matches name='accel2d' (virtinst used to
have a pattern like this when parsing capabilities XML), on step #2 the code
will think the <enum> with the 'qxl' condition is the only instance. The
parser doesn't know about <condition> yet, will assume accel2d is supported
for everyone, and set accel2d=on for cirrus which will probably fail.
Okay, starting from a working step #2. The parser knows about multiple enums,
and basic <condition> statements. Upgrade to step #3. If the parser was dumb,
maybe it didn't know about multiple <condition> enums, which may cause an
incorrect match, setting accel2d=yes, and the config fails.
Assume the parser is smart enough to know about multiple <condition> enums. It
sees addressType, but it doesn't know what XML value it maps to. Does the app
just ignore it and hope the config works, possibly generating an error? Does
the app completely abandon the check even though it may work correctly? The
latter is certainly the safest, but coding errors/naive impls could make
mistakes or possible even throw other errors.
Now consider that pattern expanding out with conditional or/and/not operators,
maybe even numerical comparisons like <, >, etc. The only way for the parser
from step #1 to not make a fatal error in step #3 is to implement <condition>
parsing from the start and code very defensively. That's a lot of upfront
overhead. And I think it will be tempting for apps to try and get by _without_
implementing the whole comparison engine and potentially set themselves up for
future pain.
Also having a very interconnected schema like this means that if we ever need
to extend the XML format we need to think _very_ hard about how parsers are
likely handing the existing XML, and how our additions might screw things up,
because we are requiring users to perform some kind of comparison on the
result. This is much different than extending the domain XML which is largely
just a config file. <capabilities> XML is the closet analog but even that
requires very little processing, the only tricky bit is handling that <domain
type='kvm'> has its own <machine> list
And even after all that, the XML is not going to be completely introspectable
unless we find a way to describe addressType=./video/address/@type in the XML.
So any general parser is still going to need to be extended regularly to
handle new enum names. So we end up with XML that
appears-introspectable-but-not-quite
So let's redo the example with the unique string names and flat namespace:
Step #1 is identical
Step #2 addition is
<enum name='qxlAccel2D'/>
<value>yes</value>
<value>no</value>
</enum>
Step #3 addition is
<enum name='cirrusAccel2D'/>
<value>yes</value>
<value>no</value>
</enum>
And for step #3 we document that it's only valid for address type=pci, If
address type=isa grows support later, we add a new top level string if we
think it's worth it. The type=pci vs type=isa is a misleading example here, if
it was some more obscure option that cirrus accel2d depended on, maybe we
would want to put that in the string name.
The misinterpretation problems for the app are non-existent. Yes it will need
to be extended at step #3 to support cirrus accel2d=yes but that's completely
fine IMO.
If the next day, xen supports domcaps and wants to expose cirrusAccel2D but
with sufficiently different semantics, then we would have to add a new string.
Yes that sucks, but it's the safest way by far for apps.
Benefits:
- The app XML parser is much simpler
- The app XML parser is much quicker to bring up. Important because I bet most
apps will only want to check a small set of support checks
- I suspect extending the libvirt code will be much simpler
- We can always come up with a way to add a comparison engine later if this
approach gets too out of hand
Downsides:
- Much less generalizable (but as I said above I don't think this there _is_ a
generalizable domcaps XML schema, nor should it be the goal)
- Apps/readers need to look to the docs to understand the enum semantics
- The app's parser requires more maintenance in relation to how many features
it wants to parse
I think the guide here should largely be exposing the types of things that we
already track with qemu capabilities, and at that granularity. Give that
information to the user (which they presently can't find out reliably on their
own), and leave the rest of the bits like interdependence of XML options up to
them to figure out. So, 'qemu supports spiceGL', and not 'a working spiceGL
config is spiceGL with listen=none or listen=unix and video virtio with accel3D'
All that said, I'm probably missing some examples where this flat modeling
especially sucks too :) So please throw them my way
Thanks,
Cole
11:10 a.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
> Yeah, I find flat modelling quite desirable, because the relationships
> between attributes will certainly grow quite complicate, and they do
> not neccessarily form a nice simple tree relationship.
>
> ie, whether foo=bar is permitted may depend on whether wizz=eek *AND*
> when lala=ooh, and you can't have the enum for 'foo' nested underneath
> the enum for 'wizz' & 'lala' at the same time. Also with
nesting, if
> you want the same values reported for multiple options, we end up
> repeating the same information multiple times which is not good.
>
> At the same time the flat modelling with "spiceGL" also doesn't scale
> as you have to invent new names for each one, and again it doesn't
> work if the 'gl' enum depended on type="spice" *and* something
else
> at the same time.
>
> So I think we need a more flexible way to express dependancies here.
>
> We could let the <enum> be repeated multiple times and express conditional
> rules against each instance of the enum
>
> So for example
>
> <graphics supported='yes'>
> <enum name='type'>
> <value>spice</value>
> <value>vnc</value>
> <value>sdl</value>
> </enum>
> <enum name='gl'>
> <condition>
> <enum name="type" value="spice">
> </condition>
> <value>default</value>
> <value>yes</value>
> <value>no</value>
> </enum>
> <enum name='gl'>
> <value>default</value>
> <value>no</value>
> </enum>
> </graphics>
>
>
> This would express that the first <enum type="gl"> entry only
applies
> when the @type == spice. If that doesn't match them fallback to the
> second <enum type="gl">. The nice thing about this is that we can
> make the conditions arbitrarly complex
>
> For example:
>
> <graphics supported='yes'>
> <enum name='type'>
> <value>spice</value>
> <value>vnc</value>
> <value>sdl</value>
> </enum>
> <enum name='gl'>
> <condition op="and">
> <enum name="type" value="spice">
> <condition op="or">
> <enum name="type" value="vnc">
> <enum name="wibble" value="eek">
> </condition>
> </condition>
> <value>default</value>
> <value>yes</value>
> <value>no</value>
> </enum>
> <enum name='gl'>
> <value>default</value>
> <value>no</value>
> </enum>
> </graphics>
>
>
> This shows how the "gl" option can be used with VNC, but only if you
> also have the @wibble attribute set to "eek".
>
> Of course complex conditions like this become quite tedious & verbose
> so obviously we should strive to keep them as simple as possible and
> not use nested conditions unless absolutely needed.
>
Writing a parser for this type of XML, that maintains behavior in a future
proof way, is going to be a lot of work. Hypothetical example:
<video supported='yes'>
<enum name='modelType'/>
<value>cirrus</value>
<value>qxl</value>
</enum>
<enum name='accel2d'/>
<value>no</value>
</enum>
</video>
6 months later, qemu gets some accel2d support for model=qxl
<video supported='yes'>
<enum name='modelType'/>
<value>cirrus</value>
<value>qxl</value>
</enum>
<enum name='accel2d'/>
<value>no</value>
</enum>
<enum name='accel2d'/>
<condition>
<enum name="modelType" value="qxl">
</condition>
<value>yes</value>
<value>no</value>
</enum>
</video>
Another 6 months later, qemu gets accel2d support for cirrus only, but it
requires <address type='pci'/> and doesn't work for <address
type='isa'/>
<video supported='yes'>
<enum name='modelType'/>
<value>cirrus</value>
<value>qxl</value>
</enum>
<enum name='addressType'/>
<value>pci</value>
<value>isa</value>
</enum>
<enum name='accel2d'/>
<value>no</value>
</enum>
<enum name='accel2d'/>
<condition>
<enum name="modelType" value="qxl">
</condition>
<value>yes</value>
<value>no</value>
</enum>
<enum name='accel2d'/>
<condition>
<enum name="modelType" value="cirrus">
<enum name="addressType" value="pci">
</condition>
<value>yes</value>
<value>no</value>
</enum>
</video>
The app wants to answer the question 'can I specify accel2d by default for
model=cirrus?' (note, cirrus, and not qxl)
For the first example, the code parses the accel2d enum, doesn't see 'yes',
so
it doesn't enable accel2d. The code will need to contain some knowledge that
enum name='accel2d' == ./video/acceleration/@accel2d but that's presently
unavoidable.
For the second example, code will need to be added to parse the basic
condition, see that modelType=cirrus (./video/model/@type) doesn't match the
condition, and so again accel2d=no
For the third example, code hits the modelType=cirrus condition, but then also
needs to check that addressType (./video/address/@type) == pci, and if so,
sets accel2d=yes
So, that's all achievable. But what kind of parser does the app write at each
step that may fall over with the next XML iteration?
What if the parser in step 1 doesn't expect multiple <enum
name='accel2d'/> ?
If it naively parses only the first instance, then nothing will regress in
step #2 or step #3. step #3's conclusion will be wrong, but that's okay. Note
this depends on libvirt being smart about appending conditional enums, and not
putting them first.
However if it parses the last instance, such as by iterating over every XML
node and taking the last one that matches name='accel2d' (virtinst used to
have a pattern like this when parsing capabilities XML), on step #2 the code
will think the <enum> with the 'qxl' condition is the only instance. The
parser doesn't know about <condition> yet, will assume accel2d is supported
for everyone, and set accel2d=on for cirrus which will probably fail.
This is all simply a matter of documentation - if we're to allow multiple
<enum> elements, then clearly we need to document the rules that a parser
should follow so that its behaviour is consistent. IOW, we would have to
document whether the default (no conditional) enum always comes first or
last, or whether the application should explicitly look for an enum without
any <condition> rules. Any one of those is a viable option - we just need
to document which. Or if we're really needing to avoid causing trouble
for pre-existing parsers, we give the non-default enums which permit use
of <condition> a different element name, eg <enumVariant>.
Okay, starting from a working step #2. The parser knows about
multiple enums,
and basic <condition> statements. Upgrade to step #3. If the parser was dumb,
maybe it didn't know about multiple <condition> enums, which may cause an
incorrect match, setting accel2d=yes, and the config fails.
Again you're just describing a case where we need to document the intended
semantics of the information. If apps using the data ignore the documented
semantics that's their problem. We also need to be extra careful about
how we extend the information in the future such that we don't add extra
elements can could confuse existing parsers. This is a little more strict
that we've been with XML extension in general, but totally managable.
Assume the parser is smart enough to know about multiple
<condition> enums. It
sees addressType, but it doesn't know what XML value it maps to. Does the app
just ignore it and hope the config works, possibly generating an error? Does
the app completely abandon the check even though it may work correctly? The
latter is certainly the safest, but coding errors/naive impls could make
mistakes or possible even throw other errors.
We would document that if an application doesn't understand the particular
condition, then it should ignore that whole enum block. It would thus use
the next/previous best-matching enum block it found.
Now consider that pattern expanding out with conditional or/and/not
operators,
maybe even numerical comparisons like <, >, etc. The only way for the parser
from step #1 to not make a fatal error in step #3 is to implement <condition>
parsing from the start and code very defensively. That's a lot of upfront
overhead. And I think it will be tempting for apps to try and get by _without_
implementing the whole comparison engine and potentially set themselves up for
future pain.
Again apps should explicitly ignore enums with comparison operators that they
don't know about, and fallback to the next best matching enum they can find.
Also having a very interconnected schema like this means that if we
ever need
to extend the XML format we need to think _very_ hard about how parsers are
likely handing the existing XML, and how our additions might screw things up,
because we are requiring users to perform some kind of comparison on the
result. This is much different than extending the domain XML which is largely
just a config file. <capabilities> XML is the closet analog but even that
requires very little processing, the only tricky bit is handling that <domain
type='kvm'> has its own <machine> list
And even after all that, the XML is not going to be completely introspectable
unless we find a way to describe addressType=./video/address/@type in the XML.
So any general parser is still going to need to be extended regularly to
handle new enum names. So we end up with XML that
appears-introspectable-but-not-quite
So let's redo the example with the unique string names and flat
namespace:
Step #1 is identical
Step #2 addition is
<enum name='qxlAccel2D'/>
<value>yes</value>
<value>no</value>
</enum>
Step #3 addition is
<enum name='cirrusAccel2D'/>
<value>yes</value>
<value>no</value>
</enum>
And for step #3 we document that it's only valid for address type=pci, If
address type=isa grows support later, we add a new top level string if we
think it's worth it. The type=pci vs type=isa is a misleading example here, if
it was some more obscure option that cirrus accel2d depended on, maybe we
would want to put that in the string name.
The misinterpretation problems for the app are non-existent. Yes it will need
to be extended at step #3 to support cirrus accel2d=yes but that's completely
fine IMO.
IMHO it is a total failure if we require the application to extend its
parser every time we add a new enum to the domain capabilities. We have
the ability to design something that is data driven - we should not build
something it is forced to be code driven with code changes for every
libvirt addition.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:26 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
IMHO it is a total failure if we require the application to extend its
parser every time we add a new enum to the domain capabilities. We have
the ability to design something that is data driven - we should not build
something it is forced to be code driven with code changes for every
libvirt addition.
This is ignoring the point I made previously that the schema is not already
fully introspectable. Most of the current enums cannot be programmatically
consumed anyways, because there's no way to map an enum name to its domain XML
property. So if that's our goal to be data driven, we should address that
issue first.
I can't really think of a good way to represent that without nesting deeply or
using specially formatted strings. Do you have a suggestion for that?
Thanks,
Cole
5:50 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Tue, May 10, 2016 at 11:26:29AM -0400, Cole Robinson wrote:
On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
> On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
>> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
>
> IMHO it is a total failure if we require the application to extend its
> parser every time we add a new enum to the domain capabilities. We have
> the ability to design something that is data driven - we should not build
> something it is forced to be code driven with code changes for every
> libvirt addition.
This is ignoring the point I made previously that the schema is not already
fully introspectable. Most of the current enums cannot be programmatically
consumed anyways, because there's no way to map an enum name to its domain XML
property. So if that's our goal to be data driven, we should address that
issue first.
Knowing the mapping of the capabilities enums to the domain schema is
a pre-requisite for consuming the capabilities data, no matter which
approach discussed in this thread is chosen. Assuming the app knows
that mapping, then the enum conditionals can be programmatically
handled with the approach I describe.
Providing a way for the app to automatically determine the mapping
from capabilities enums to the domain schema would be a nice
addition, but it isn't a pre-requisite blocker for what's discussed
here. It is something that can be deal with in parallel.
I can't really think of a good way to represent that without
nesting
deeply or using specially formatted strings. Do you have a suggestion
for that?
Probably the best thing is to use a very simplified xpath style notation.
If we add a 'mapping' attribute to the <enum> that expresses the attribute
or element it is associated with, relative to the parent container element.
eg, consider the tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
data file with the mapping info:
<domainCapabilities>
<path>/usr/bin/qemu-system-x86_64</path>
<domain>kvm</domain>
<machine>pc-1.2</machine>
<arch>x86_64</arch>
<os supported='yes'>
<loader supported='yes'>
<value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
<value>/usr/share/OVMF/OVMF_CODE.fd</value>
<enum name='type' mapping="@type">
<value>rom</value>
<value>pflash</value>
</enum>
<enum name='readonly' mapping="@readonly">
<value>yes</value>
<value>no</value>
</enum>
</loader>
</os>
<devices>
<disk supported='yes'>
<enum name='diskDevice' mapping="@device">
<value>disk</value>
<value>cdrom</value>
<value>floppy</value>
<value>lun</value>
</enum>
<enum name='bus' mapping="target/@bus">
<value>ide</value>
<value>fdc</value>
<value>scsi</value>
<value>virtio</value>
<value>usb</value>
</enum>
</disk>
<hostdev supported='yes'>
<enum name='mode' mapping="@mode">
<value>subsystem</value>
</enum>
<enum name='startupPolicy' mapping="source/@startupPolicy">
<value>default</value>
<value>mandatory</value>
<value>requisite</value>
<value>optional</value>
</enum>
<enum name='subsysType' mapping="@type">
<value>usb</value>
<value>pci</value>
<value>scsi</value>
</enum>
<enum name='capsType' mapping="@type"/>
<enum name='pciBackend' mapping="driver/@name">
<value>default</value>
<value>kvm</value>
<value>vfio</value>
</enum>
</hostdev>
</devices>
<features>
<gic supported='no'/>
</features>
</domainCapabilities>
NB, with my proposed conditionals, the hostdev enums above would actually
want to be different. eg it would want to look like this:
<enum name='mode' mapping="@mode">
<value>subsystem</value>
</enum>
<enum name='startupPolicy' mapping="source/@startupPolicy">
<value>default</value>
<value>mandatory</value>
<value>requisite</value>
<value>optional</value>
</enum>
<enum name='type' mapping="@type">
<condition>
<enum name='mode' value='subsystem'/>
</condition>
<value>usb</value>
<value>pci</value>
<value>scsi</value>
</enum>
<enum name='driver' mapping="driver/@name">
<condition>
<enum name='mode' value='subsystem'/>
<enum name='type' value='pci'/>
</condition>
<value>default</value>
<value>kvm</value>
<value>vfio</value>
</enum>
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:15 a.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 05/10/2016 11:50 AM, Daniel P. Berrange wrote:
On Tue, May 10, 2016 at 11:26:29AM -0400, Cole Robinson wrote:
> On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
>> On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
>>> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
>>
>> IMHO it is a total failure if we require the application to extend its
>> parser every time we add a new enum to the domain capabilities. We have
>> the ability to design something that is data driven - we should not build
>> something it is forced to be code driven with code changes for every
>> libvirt addition.
>
> This is ignoring the point I made previously that the schema is not already
> fully introspectable. Most of the current enums cannot be programmatically
> consumed anyways, because there's no way to map an enum name to its domain XML
> property. So if that's our goal to be data driven, we should address that
> issue first.
Knowing the mapping of the capabilities enums to the domain schema is
a pre-requisite for consuming the capabilities data, no matter which
approach discussed in this thread is chosen. Assuming the app knows
that mapping, then the enum conditionals can be programmatically
handled with the approach I describe.
Providing a way for the app to automatically determine the mapping
from capabilities enums to the domain schema would be a nice
addition, but it isn't a pre-requisite blocker for what's discussed
here. It is something that can be deal with in parallel.
> I can't really think of a good way to represent that without nesting
> deeply or using specially formatted strings. Do you have a suggestion
> for that?
Probably the best thing is to use a very simplified xpath style notation.
If we add a 'mapping' attribute to the <enum> that expresses the attribute
or element it is associated with, relative to the parent container element.
eg, consider the tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
data file with the mapping info:
<domainCapabilities>
<path>/usr/bin/qemu-system-x86_64</path>
<domain>kvm</domain>
<machine>pc-1.2</machine>
<arch>x86_64</arch>
<os supported='yes'>
<loader supported='yes'>
<value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
<value>/usr/share/OVMF/OVMF_CODE.fd</value>
<enum name='type' mapping="@type">
<value>rom</value>
<value>pflash</value>
</enum>
<enum name='readonly' mapping="@readonly">
<value>yes</value>
<value>no</value>
</enum>
</loader>
</os>
<devices>
<disk supported='yes'>
<enum name='diskDevice' mapping="@device">
<value>disk</value>
<value>cdrom</value>
<value>floppy</value>
<value>lun</value>
</enum>
<enum name='bus' mapping="target/@bus">
<value>ide</value>
<value>fdc</value>
<value>scsi</value>
<value>virtio</value>
<value>usb</value>
</enum>
</disk>
<hostdev supported='yes'>
<enum name='mode' mapping="@mode">
<value>subsystem</value>
</enum>
<enum name='startupPolicy'
mapping="source/@startupPolicy">
<value>default</value>
<value>mandatory</value>
<value>requisite</value>
<value>optional</value>
</enum>
<enum name='subsysType' mapping="@type">
<value>usb</value>
<value>pci</value>
<value>scsi</value>
</enum>
<enum name='capsType' mapping="@type"/>
<enum name='pciBackend' mapping="driver/@name">
<value>default</value>
<value>kvm</value>
<value>vfio</value>
</enum>
</hostdev>
</devices>
<features>
<gic supported='no'/>
</features>
</domainCapabilities>
NB, with my proposed conditionals, the hostdev enums above would actually
want to be different. eg it would want to look like this:
<enum name='mode' mapping="@mode">
<value>subsystem</value>
</enum>
<enum name='startupPolicy'
mapping="source/@startupPolicy">
<value>default</value>
<value>mandatory</value>
<value>requisite</value>
<value>optional</value>
</enum>
<enum name='type' mapping="@type">
<condition>
<enum name='mode' value='subsystem'/>
</condition>
<value>usb</value>
<value>pci</value>
<value>scsi</value>
</enum>
<enum name='driver' mapping="driver/@name">
<condition>
<enum name='mode' value='subsystem'/>
<enum name='type' value='pci'/>
</condition>
<value>default</value>
<value>kvm</value>
<value>vfio</value>
</enum>
Yeah that mapping= bit makes sense to me.
I don't have time to see pick this up now though, so I've stuffed it in a bug:
https://bugzilla.redhat.com/show_bug.cgi?id=1334958
Thanks,
Cole
10:49 a.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Tue, May 10, 2016 at 07:15:01PM -0400, Cole Robinson wrote:
On 05/10/2016 11:50 AM, Daniel P. Berrange wrote:
> On Tue, May 10, 2016 at 11:26:29AM -0400, Cole Robinson wrote:
>> On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
>>> On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
>>>> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
>>>
>>> IMHO it is a total failure if we require the application to extend its
>>> parser every time we add a new enum to the domain capabilities. We have
>>> the ability to design something that is data driven - we should not build
>>> something it is forced to be code driven with code changes for every
>>> libvirt addition.
>>
>> This is ignoring the point I made previously that the schema is not already
>> fully introspectable. Most of the current enums cannot be programmatically
>> consumed anyways, because there's no way to map an enum name to its domain
XML
>> property. So if that's our goal to be data driven, we should address that
>> issue first.
>
> Knowing the mapping of the capabilities enums to the domain schema is
> a pre-requisite for consuming the capabilities data, no matter which
> approach discussed in this thread is chosen. Assuming the app knows
> that mapping, then the enum conditionals can be programmatically
> handled with the approach I describe.
>
> Providing a way for the app to automatically determine the mapping
> from capabilities enums to the domain schema would be a nice
> addition, but it isn't a pre-requisite blocker for what's discussed
> here. It is something that can be deal with in parallel.
>
>> I can't really think of a good way to represent that without nesting
>> deeply or using specially formatted strings. Do you have a suggestion
>> for that?
>
> Probably the best thing is to use a very simplified xpath style notation.
> If we add a 'mapping' attribute to the <enum> that expresses the
attribute
> or element it is associated with, relative to the parent container element.
>
> eg, consider the tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
> data file with the mapping info:
>
> <domainCapabilities>
> <path>/usr/bin/qemu-system-x86_64</path>
> <domain>kvm</domain>
> <machine>pc-1.2</machine>
> <arch>x86_64</arch>
> <os supported='yes'>
> <loader supported='yes'>
> <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
> <value>/usr/share/OVMF/OVMF_CODE.fd</value>
> <enum name='type' mapping="@type">
> <value>rom</value>
> <value>pflash</value>
> </enum>
> <enum name='readonly' mapping="@readonly">
> <value>yes</value>
> <value>no</value>
> </enum>
> </loader>
> </os>
> <devices>
> <disk supported='yes'>
> <enum name='diskDevice' mapping="@device">
> <value>disk</value>
> <value>cdrom</value>
> <value>floppy</value>
> <value>lun</value>
> </enum>
> <enum name='bus' mapping="target/@bus">
> <value>ide</value>
> <value>fdc</value>
> <value>scsi</value>
> <value>virtio</value>
> <value>usb</value>
> </enum>
> </disk>
> <hostdev supported='yes'>
> <enum name='mode' mapping="@mode">
> <value>subsystem</value>
> </enum>
> <enum name='startupPolicy'
mapping="source/@startupPolicy">
> <value>default</value>
> <value>mandatory</value>
> <value>requisite</value>
> <value>optional</value>
> </enum>
> <enum name='subsysType' mapping="@type">
> <value>usb</value>
> <value>pci</value>
> <value>scsi</value>
> </enum>
> <enum name='capsType' mapping="@type"/>
> <enum name='pciBackend' mapping="driver/@name">
> <value>default</value>
> <value>kvm</value>
> <value>vfio</value>
> </enum>
> </hostdev>
> </devices>
> <features>
> <gic supported='no'/>
> </features>
> </domainCapabilities>
>
>
> NB, with my proposed conditionals, the hostdev enums above would actually
> want to be different. eg it would want to look like this:
>
> <enum name='mode' mapping="@mode">
> <value>subsystem</value>
> </enum>
> <enum name='startupPolicy'
mapping="source/@startupPolicy">
> <value>default</value>
> <value>mandatory</value>
> <value>requisite</value>
> <value>optional</value>
> </enum>
> <enum name='type' mapping="@type">
> <condition>
> <enum name='mode' value='subsystem'/>
> </condition>
> <value>usb</value>
> <value>pci</value>
> <value>scsi</value>
> </enum>
> <enum name='driver' mapping="driver/@name">
> <condition>
> <enum name='mode' value='subsystem'/>
> <enum name='type' value='pci'/>
> </condition>
> <value>default</value>
> <value>kvm</value>
> <value>vfio</value>
> </enum>
>
Yeah that mapping= bit makes sense to me.
I don't have time to see pick this up now though, so I've stuffed it in a bug:
Thinking about this some more last night, I realize that once we have
the 'mapping' attribute, then the choice of values in the 'name'
attribute
becomes totally irrelevant.
IOW, we could use your suggestion for giving each enum a unique name,
eg 'spiceGL', 'vncGL', etc, etc. So apps which want to consume this
data have a choice between just matching on explicit enum names and
ignoring the <condition> rules, or they can match on the 'mapping'
attribute and use the <condition> rules.. So we get the best of both
ideas.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:25 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 05/11/2016 04:49 AM, Daniel P. Berrange wrote:
On Tue, May 10, 2016 at 07:15:01PM -0400, Cole Robinson wrote:
> On 05/10/2016 11:50 AM, Daniel P. Berrange wrote:
>> On Tue, May 10, 2016 at 11:26:29AM -0400, Cole Robinson wrote:
>>> On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
>>>> On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
>>>>> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
>>>>
>>>> IMHO it is a total failure if we require the application to extend its
>>>> parser every time we add a new enum to the domain capabilities. We have
>>>> the ability to design something that is data driven - we should not
build
>>>> something it is forced to be code driven with code changes for every
>>>> libvirt addition.
>>>
>>> This is ignoring the point I made previously that the schema is not already
>>> fully introspectable. Most of the current enums cannot be programmatically
>>> consumed anyways, because there's no way to map an enum name to its
domain XML
>>> property. So if that's our goal to be data driven, we should address
that
>>> issue first.
>>
>> Knowing the mapping of the capabilities enums to the domain schema is
>> a pre-requisite for consuming the capabilities data, no matter which
>> approach discussed in this thread is chosen. Assuming the app knows
>> that mapping, then the enum conditionals can be programmatically
>> handled with the approach I describe.
>>
>> Providing a way for the app to automatically determine the mapping
>> from capabilities enums to the domain schema would be a nice
>> addition, but it isn't a pre-requisite blocker for what's discussed
>> here. It is something that can be deal with in parallel.
>>
>>> I can't really think of a good way to represent that without nesting
>>> deeply or using specially formatted strings. Do you have a suggestion
>>> for that?
>>
>> Probably the best thing is to use a very simplified xpath style notation.
>> If we add a 'mapping' attribute to the <enum> that expresses the
attribute
>> or element it is associated with, relative to the parent container element.
>>
>> eg, consider the tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
>> data file with the mapping info:
>>
>> <domainCapabilities>
>> <path>/usr/bin/qemu-system-x86_64</path>
>> <domain>kvm</domain>
>> <machine>pc-1.2</machine>
>> <arch>x86_64</arch>
>> <os supported='yes'>
>> <loader supported='yes'>
>> <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
>> <value>/usr/share/OVMF/OVMF_CODE.fd</value>
>> <enum name='type' mapping="@type">
>> <value>rom</value>
>> <value>pflash</value>
>> </enum>
>> <enum name='readonly' mapping="@readonly">
>> <value>yes</value>
>> <value>no</value>
>> </enum>
>> </loader>
>> </os>
>> <devices>
>> <disk supported='yes'>
>> <enum name='diskDevice' mapping="@device">
>> <value>disk</value>
>> <value>cdrom</value>
>> <value>floppy</value>
>> <value>lun</value>
>> </enum>
>> <enum name='bus' mapping="target/@bus">
>> <value>ide</value>
>> <value>fdc</value>
>> <value>scsi</value>
>> <value>virtio</value>
>> <value>usb</value>
>> </enum>
>> </disk>
>> <hostdev supported='yes'>
>> <enum name='mode' mapping="@mode">
>> <value>subsystem</value>
>> </enum>
>> <enum name='startupPolicy'
mapping="source/@startupPolicy">
>> <value>default</value>
>> <value>mandatory</value>
>> <value>requisite</value>
>> <value>optional</value>
>> </enum>
>> <enum name='subsysType' mapping="@type">
>> <value>usb</value>
>> <value>pci</value>
>> <value>scsi</value>
>> </enum>
>> <enum name='capsType' mapping="@type"/>
>> <enum name='pciBackend' mapping="driver/@name">
>> <value>default</value>
>> <value>kvm</value>
>> <value>vfio</value>
>> </enum>
>> </hostdev>
>> </devices>
>> <features>
>> <gic supported='no'/>
>> </features>
>> </domainCapabilities>
>>
>>
>> NB, with my proposed conditionals, the hostdev enums above would actually
>> want to be different. eg it would want to look like this:
>>
>> <enum name='mode' mapping="@mode">
>> <value>subsystem</value>
>> </enum>
>> <enum name='startupPolicy'
mapping="source/@startupPolicy">
>> <value>default</value>
>> <value>mandatory</value>
>> <value>requisite</value>
>> <value>optional</value>
>> </enum>
>> <enum name='type' mapping="@type">
>> <condition>
>> <enum name='mode' value='subsystem'/>
>> </condition>
>> <value>usb</value>
>> <value>pci</value>
>> <value>scsi</value>
>> </enum>
>> <enum name='driver' mapping="driver/@name">
>> <condition>
>> <enum name='mode' value='subsystem'/>
>> <enum name='type' value='pci'/>
>> </condition>
>> <value>default</value>
>> <value>kvm</value>
>> <value>vfio</value>
>> </enum>
>>
>
> Yeah that mapping= bit makes sense to me.
>
> I don't have time to see pick this up now though, so I've stuffed it in a
bug:
Thinking about this some more last night, I realize that once we have
the 'mapping' attribute, then the choice of values in the 'name'
attribute
becomes totally irrelevant.
IOW, we could use your suggestion for giving each enum a unique name,
eg 'spiceGL', 'vncGL', etc, etc. So apps which want to consume this
data have a choice between just matching on explicit enum names and
ignoring the <condition> rules, or they can match on the 'mapping'
attribute and use the <condition> rules.. So we get the best of both
ideas.
Hmm, but does that mean that once we assign an enum name, its semantics are
locked in stone? Can we add new conditions to an existing enum to clarify
semantics, or does extending the condition list always mandate a new enum?
- Cole
5:30 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On Wed, May 11, 2016 at 11:25:50AM -0400, Cole Robinson wrote:
On 05/11/2016 04:49 AM, Daniel P. Berrange wrote:
> On Tue, May 10, 2016 at 07:15:01PM -0400, Cole Robinson wrote:
>> On 05/10/2016 11:50 AM, Daniel P. Berrange wrote:
>>> On Tue, May 10, 2016 at 11:26:29AM -0400, Cole Robinson wrote:
>>>> On 05/10/2016 05:10 AM, Daniel P. Berrange wrote:
>>>>> On Mon, May 09, 2016 at 06:53:01PM -0400, Cole Robinson wrote:
>>>>>> On 05/09/2016 09:48 AM, Daniel P. Berrange wrote:
>>>>>
>>>>> IMHO it is a total failure if we require the application to extend
its
>>>>> parser every time we add a new enum to the domain capabilities. We
have
>>>>> the ability to design something that is data driven - we should not
build
>>>>> something it is forced to be code driven with code changes for
every
>>>>> libvirt addition.
>>>>
>>>> This is ignoring the point I made previously that the schema is not
already
>>>> fully introspectable. Most of the current enums cannot be
programmatically
>>>> consumed anyways, because there's no way to map an enum name to its
domain XML
>>>> property. So if that's our goal to be data driven, we should address
that
>>>> issue first.
>>>
>>> Knowing the mapping of the capabilities enums to the domain schema is
>>> a pre-requisite for consuming the capabilities data, no matter which
>>> approach discussed in this thread is chosen. Assuming the app knows
>>> that mapping, then the enum conditionals can be programmatically
>>> handled with the approach I describe.
>>>
>>> Providing a way for the app to automatically determine the mapping
>>> from capabilities enums to the domain schema would be a nice
>>> addition, but it isn't a pre-requisite blocker for what's discussed
>>> here. It is something that can be deal with in parallel.
>>>
>>>> I can't really think of a good way to represent that without
nesting
>>>> deeply or using specially formatted strings. Do you have a suggestion
>>>> for that?
>>>
>>> Probably the best thing is to use a very simplified xpath style notation.
>>> If we add a 'mapping' attribute to the <enum> that expresses
the attribute
>>> or element it is associated with, relative to the parent container element.
>>>
>>> eg, consider the tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
>>> data file with the mapping info:
>>>
>>> <domainCapabilities>
>>> <path>/usr/bin/qemu-system-x86_64</path>
>>> <domain>kvm</domain>
>>> <machine>pc-1.2</machine>
>>> <arch>x86_64</arch>
>>> <os supported='yes'>
>>> <loader supported='yes'>
>>> <value>/usr/share/AAVMF/AAVMF_CODE.fd</value>
>>> <value>/usr/share/OVMF/OVMF_CODE.fd</value>
>>> <enum name='type' mapping="@type">
>>> <value>rom</value>
>>> <value>pflash</value>
>>> </enum>
>>> <enum name='readonly' mapping="@readonly">
>>> <value>yes</value>
>>> <value>no</value>
>>> </enum>
>>> </loader>
>>> </os>
>>> <devices>
>>> <disk supported='yes'>
>>> <enum name='diskDevice' mapping="@device">
>>> <value>disk</value>
>>> <value>cdrom</value>
>>> <value>floppy</value>
>>> <value>lun</value>
>>> </enum>
>>> <enum name='bus' mapping="target/@bus">
>>> <value>ide</value>
>>> <value>fdc</value>
>>> <value>scsi</value>
>>> <value>virtio</value>
>>> <value>usb</value>
>>> </enum>
>>> </disk>
>>> <hostdev supported='yes'>
>>> <enum name='mode' mapping="@mode">
>>> <value>subsystem</value>
>>> </enum>
>>> <enum name='startupPolicy'
mapping="source/@startupPolicy">
>>> <value>default</value>
>>> <value>mandatory</value>
>>> <value>requisite</value>
>>> <value>optional</value>
>>> </enum>
>>> <enum name='subsysType' mapping="@type">
>>> <value>usb</value>
>>> <value>pci</value>
>>> <value>scsi</value>
>>> </enum>
>>> <enum name='capsType' mapping="@type"/>
>>> <enum name='pciBackend'
mapping="driver/@name">
>>> <value>default</value>
>>> <value>kvm</value>
>>> <value>vfio</value>
>>> </enum>
>>> </hostdev>
>>> </devices>
>>> <features>
>>> <gic supported='no'/>
>>> </features>
>>> </domainCapabilities>
>>>
>>>
>>> NB, with my proposed conditionals, the hostdev enums above would actually
>>> want to be different. eg it would want to look like this:
>>>
>>> <enum name='mode' mapping="@mode">
>>> <value>subsystem</value>
>>> </enum>
>>> <enum name='startupPolicy'
mapping="source/@startupPolicy">
>>> <value>default</value>
>>> <value>mandatory</value>
>>> <value>requisite</value>
>>> <value>optional</value>
>>> </enum>
>>> <enum name='type' mapping="@type">
>>> <condition>
>>> <enum name='mode' value='subsystem'/>
>>> </condition>
>>> <value>usb</value>
>>> <value>pci</value>
>>> <value>scsi</value>
>>> </enum>
>>> <enum name='driver' mapping="driver/@name">
>>> <condition>
>>> <enum name='mode' value='subsystem'/>
>>> <enum name='type' value='pci'/>
>>> </condition>
>>> <value>default</value>
>>> <value>kvm</value>
>>> <value>vfio</value>
>>> </enum>
>>>
>>
>> Yeah that mapping= bit makes sense to me.
>>
>> I don't have time to see pick this up now though, so I've stuffed it in
a bug:
>
> Thinking about this some more last night, I realize that once we have
> the 'mapping' attribute, then the choice of values in the 'name'
attribute
> becomes totally irrelevant.
>
> IOW, we could use your suggestion for giving each enum a unique name,
> eg 'spiceGL', 'vncGL', etc, etc. So apps which want to consume this
> data have a choice between just matching on explicit enum names and
> ignoring the <condition> rules, or they can match on the 'mapping'
> attribute and use the <condition> rules.. So we get the best of both
> ideas.
Hmm, but does that mean that once we assign an enum name, its semantics are
locked in stone? Can we add new conditions to an existing enum to clarify
semantics, or does extending the condition list always mandate a new enum?
Hmm, that's a good question - to too sure really, but I'd probably be
inclined to say the latter.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:05 p.m.
New subject: [libvirt] [PATCH 3/5] domaincaps: Report graphics spiceGL
On 09.05.2016 13:30, Pavel Hrdina wrote:
On Sun, May 08, 2016 at 01:49:06PM -0400, Cole Robinson wrote:
> Reports a tristate enum value for acceptable graphics type=spice
> <gl enable=XXX/>.
>
> Wire it up for qemu too. 'no' is always a valid value, so we
> unconditionally report it.
> ---
[...]
> diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
> index 2f529ff..4eb5637 100644
> --- a/tests/domaincapsschemadata/domaincaps-full.xml
> +++ b/tests/domaincapsschemadata/domaincaps-full.xml
> @@ -47,6 +47,11 @@
> <value>desktop</value>
> <value>spice</value>
> </enum>
> + <enum name='spiceGL'>
> + <value>default</value>
> + <value>yes</value>
> + <value>no</value>
> + </enum>
Ewww, this doesn't look good and I agree with Michal that what if we add
support for another graphics device and what if one graphics device will have
more than one capability?
I would suggest something like this:
<graphics supported='yes'>
<enum name='type'>
<value>spice</value>
<value>vnc</value>
<value>sdl</value>
</enum>
I think we should have this enum in as it's easier for mgmt apps to
create XPath to query for supported values. In the case below they would
need to special case between supported graphics and the rest. That's why
I ACKed 1/5.
<type name='spice'>
<gl supported='yes'/>
</type>
I like this. The only problem I see with this approach is when we would
want to express multidimensional values. I mean what if <gl/> would not
depend just on graphics type='spice' but also on another attribute
having specific value? It is more visible in a general case: feature C
can be enabled iff A && B:
<tag A='yes' B='yes'>
<C enabled='yes'/>
</tag>
How would this be represented in domcaps?
> </graphics>
>
> or even do something like this:
>
> <graphics supported='yes'>
<type name='spice'>
<gl supported='yes'/>
</type>
> <type name='vnc'/>
> <type name='sdl'/>
> </graphics>
>
Michal
7:49 p.m.
New subject: [libvirt] [PATCH 4/5] qemu: command: unconditionally allow accel3d='no'
This matches how we handle spice gl='no' even if spice GL isn't
supported. Not too interesting in practice but I figure we should
be consistent
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2966b07..c26715f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4125,7 +4125,7 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias);
if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
- if (video->accel && video->accel->accel3d) {
+ if (video->accel && video->accel->accel3d ==
VIR_TRISTATE_SWITCH_ON) {
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("virtio-gpu 3d acceleration is not
supported"));
--
2.7.4
11:27 a.m.
New subject: [libvirt] [PATCH 4/5] qemu: command: unconditionally allow accel3d='no'
On 08.05.2016 19:49, Cole Robinson wrote:
This matches how we handle spice gl='no' even if spice GL
isn't
supported. Not too interesting in practice but I figure we should
be consistent
---
src/qemu/qemu_command.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2966b07..c26715f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4125,7 +4125,7 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias);
if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
- if (video->accel && video->accel->accel3d) {
+ if (video->accel && video->accel->accel3d ==
VIR_TRISTATE_SWITCH_ON) {
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
"%s", _("virtio-gpu 3d acceleration is not
supported"));
In fact, this is important in practice. Because if in accel3d had been
'no' we would ignore it and execute the if() statement body. Both
VIR_TRISTATE_SWITCH_ON and _OFF have non zero values.
ACK
Michal
7:49 p.m.
New subject: [libvirt] [PATCH 5/5] domaincaps: Report graphics spiceGL
Reports a tristate enum value for acceptable video model=virtio
<acceleration accel3d=XXX/>.
Wire it up for qemu too. 'no' is always a valid value, so we
unconditionally report it.
---
docs/formatdomaincaps.html.in | 9 +++++++++
src/conf/domain_capabilities.c | 1 +
src/conf/domain_capabilities.h | 2 ++
src/qemu/qemu_capabilities.c | 4 ++++
tests/domaincapsschemadata/domaincaps-full.xml | 5 +++++
tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml | 4 ++++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml | 3 +++
tests/domaincapstest.c | 1 +
12 files changed, 41 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index c424107..b87a45a 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -268,6 +268,10 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</video>
...
</devices>
@@ -278,6 +282,11 @@
<dt><code>modelType</code></dt>
<dd>Options for the <code>type</code> attribute of the
<video><model> element.</dd>
+
+ <dt><code>spiceGL</code></dt>
+ <dd>Options for the <code>accel3D</code> attribute of the
+ <video><model
type='virtio'/><acceleration/>
+ element.</dd>
</dl>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 344955c..b5715d4 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -266,6 +266,7 @@ virDomainCapsDeviceVideoFormat(virBufferPtr buf,
FORMAT_PROLOGUE(video);
ENUM_PROCESS(video, modelType, virDomainVideoTypeToString);
+ ENUM_PROCESS(video, virtioAccel3D, virTristateBoolTypeToString);
FORMAT_EPILOGUE(video);
}
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 916dba0..0501e99 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -82,6 +82,8 @@ typedef virDomainCapsDeviceVideo *virDomainCapsDeviceVideoPtr;
struct _virDomainCapsDeviceVideo {
bool supported;
virDomainCapsEnum modelType; /* virDomainVideoType */
+ virDomainCapsEnum virtioAccel3D; /* tristate for type=virtio
+ <acceleration accel3d=X/> */
};
typedef struct _virDomainCapsDeviceHostdev virDomainCapsDeviceHostdev;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f228f4f..511faee 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -4208,6 +4208,10 @@ virQEMUCapsFillDomainDeviceVideoCaps(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU))
VIR_DOMAIN_CAPS_ENUM_SET(dev->modelType, VIR_DOMAIN_VIDEO_TYPE_VIRTIO);
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->virtioAccel3D, VIR_TRISTATE_BOOL_NO);
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL))
+ VIR_DOMAIN_CAPS_ENUM_SET(dev->virtioAccel3D, VIR_TRISTATE_BOOL_YES);
+
return 0;
}
diff --git a/tests/domaincapsschemadata/domaincaps-full.xml
b/tests/domaincapsschemadata/domaincaps-full.xml
index 4eb5637..568f452 100644
--- a/tests/domaincapsschemadata/domaincaps-full.xml
+++ b/tests/domaincapsschemadata/domaincaps-full.xml
@@ -64,6 +64,11 @@
<value>parallels</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>default</value>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
index 6213297..f287a22 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
@@ -51,6 +51,9 @@
<value>vmvga</value>
<value>qxl</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
index 2a1477f..7182b3d 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml
@@ -53,6 +53,10 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
index 4349998..64d4c03 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml
@@ -49,6 +49,9 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
index 27173c4..c019ac4 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml
@@ -49,6 +49,9 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
index d7d81b5..539545b 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml
@@ -49,6 +49,9 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
index 51c5615..454233f 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml
@@ -47,6 +47,9 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>no</value>
+ </enum>
</video>
<hostdev supported='yes'>
<enum name='mode'>
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 1b62781..ba34a08 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -87,6 +87,7 @@ fillAllCaps(virDomainCapsPtr domCaps)
video->supported = true;
SET_ALL_BITS(video->modelType);
+ SET_ALL_BITS(video->virtioAccel3D);
hostdev->supported = true;
SET_ALL_BITS(hostdev->mode);
--
2.7.4
11:27 a.m.
New subject: [libvirt] [PATCH 5/5] domaincaps: Report graphics spiceGL
On 08.05.2016 19:49, Cole Robinson wrote:
Reports a tristate enum value for acceptable video model=virtio
<acceleration accel3d=XXX/>.
Wire it up for qemu too. 'no' is always a valid value, so we
unconditionally report it.
---
docs/formatdomaincaps.html.in | 9 +++++++++
src/conf/domain_capabilities.c | 1 +
src/conf/domain_capabilities.h | 2 ++
src/qemu/qemu_capabilities.c | 4 ++++
tests/domaincapsschemadata/domaincaps-full.xml | 5 +++++
tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml | 4 ++++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml | 3 +++
tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml | 3 +++
tests/domaincapstest.c | 1 +
12 files changed, 41 insertions(+)
diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index c424107..b87a45a 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -268,6 +268,10 @@
<value>qxl</value>
<value>virtio</value>
</enum>
+ <enum name='virtioAccel3D'>
+ <value>yes</value>
+ <value>no</value>
+ </enum>
</video>
...
</devices>
@@ -278,6 +282,11 @@
<dt><code>modelType</code></dt>
<dd>Options for the <code>type</code> attribute of the
<video><model> element.</dd>
+
+ <dt><code>spiceGL</code></dt>
Copy paste error. This should have been virtioAccel3D. This is more or
less the same problem (maybe too strong term though) that I've raised in
3/5. Just accel3D perhaps? Otherwise the code looks good.
Michal
10:15 p.m.
New subject: [libvirt] [PATCH 5/5] domaincaps: Report graphics spiceGL
On 05/09/2016 05:27 AM, Michal Privoznik wrote:
On 08.05.2016 19:49, Cole Robinson wrote:
> Reports a tristate enum value for acceptable video model=virtio
> <acceleration accel3d=XXX/>.
>
> Wire it up for qemu too. 'no' is always a valid value, so we
> unconditionally report it.
> ---
> docs/formatdomaincaps.html.in | 9 +++++++++
> src/conf/domain_capabilities.c | 1 +
> src/conf/domain_capabilities.h | 2 ++
> src/qemu/qemu_capabilities.c | 4 ++++
> tests/domaincapsschemadata/domaincaps-full.xml | 5 +++++
> tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-1.xml | 4 ++++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-2.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-3.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-4.xml | 3 +++
> tests/domaincapsschemadata/domaincaps-qemu_2.6.0-5.xml | 3 +++
> tests/domaincapstest.c | 1 +
> 12 files changed, 41 insertions(+)
>
> diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
> index c424107..b87a45a 100644
> --- a/docs/formatdomaincaps.html.in
> +++ b/docs/formatdomaincaps.html.in
> @@ -268,6 +268,10 @@
> <value>qxl</value>
> <value>virtio</value>
> </enum>
> + <enum name='virtioAccel3D'>
> + <value>yes</value>
> + <value>no</value>
> + </enum>
> </video>
> ...
> </devices>
> @@ -278,6 +282,11 @@
> <dt><code>modelType</code></dt>
> <dd>Options for the <code>type</code> attribute of the
> <video><model> element.</dd>
> +
> + <dt><code>spiceGL</code></dt>
Copy paste error. This should have been virtioAccel3D. This is more or
less the same problem (maybe too strong term though) that I've raised in
3/5. Just accel3D perhaps? Otherwise the code looks good.
Fixed locally (and the subject... not sure how that happened)
I've pushed patches 1, 2, and 4 since you ACKd and they don't seem
controversial. Thanks!
- Cole
3055
days inactive
3058
days old
25 comments
5 participants
participants (5)
-
Cole Robinson -
Daniel P. Berrange -
Michal Privoznik -
Pavel Hrdina -
Peter Krempa