[libvirt] [PATCH 0/4] Adding resolution properties for QXL device
From: Julio Faracco <jcfaracco@gmail.com> This serie adds 'xres' and 'yres' properties into XML definition for QXL video device to specify a default resolution. This serie covers a simple test case too. Julio Faracco (4): docs: Adding 'xres' and 'yres' into qxl XML definition conf: Adding XML support for 'xres' and 'yres' qemu: Generate 'xres' and 'yres' for qxl device. tests: Add separate tests for 'xres' and 'yres' docs/schemas/domaincommon.rng | 10 +++++ src/conf/domain_conf.c | 26 ++++++++++++ src/conf/domain_conf.h | 2 + src/qemu/qemu_command.c | 4 ++ .../video-qxl-resolution.args | 32 +++++++++++++++ .../qemuxml2argvdata/video-qxl-resolution.xml | 40 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../video-qxl-resolution.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 9 files changed, 159 insertions(+) create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.args create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.xml create mode 100644 tests/qemuxml2xmloutdata/video-qxl-resolution.xml -- 2.20.1
From: Julio Faracco <jcfaracco@gmail.com> This commit adds 'xres' and 'yres' into qxl XML Domain group definition. Both, properties were added into properties group inside qxl model and they are set as optional. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/schemas/domaincommon.rng | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a0771da45b..8d95948595 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3606,6 +3606,16 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="xres"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="yres"> + <ref name="unsignedInt"/> + </attribute> + </optional> </group> </choice> <optional> -- 2.20.1
On Sun, Aug 04, 2019 at 10:21:18PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
This commit adds 'xres' and 'yres' into qxl XML Domain group definition. Both, properties were added into properties group inside qxl model and they are set as optional.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- docs/schemas/domaincommon.rng | 10 ++++++++++ 1 file changed, 10 insertions(+)
Changes to the HTML docs are missing.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a0771da45b..8d95948595 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3606,6 +3606,16 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="xres"> + <ref name="unsignedInt"/> + </attribute> + </optional> + <optional> + <attribute name="yres"> + <ref name="unsignedInt"/> + </attribute> + </optional>
Both attributes should be enclosed by a single <optional> element, there's IMO no compelling reason to allow usage and therefore validation of one without the other, is there? Erik
From: Julio Faracco <jcfaracco@gmail.com> XML need to support both properties together. This commit adds XML support for QXL model if they are set. Domain configuration is able to parse this properties. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 2 files changed, 28 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 441eb1a5a2..120c6ccf5f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15360,6 +15360,8 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) ram = NULL; VIR_AUTOFREE(char *) vgamem = NULL; VIR_AUTOFREE(char *) primary = NULL; + VIR_AUTOFREE(char *) xres = NULL; + VIR_AUTOFREE(char *) yres = NULL; if (!(def = virDomainVideoDefNew())) return NULL; @@ -15377,6 +15379,8 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, vram64 = virXMLPropString(cur, "vram64"); vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads"); + xres = virXMLPropString(cur, "xres"); + yres = virXMLPropString(cur, "yres"); if ((primary = virXMLPropString(cur, "primary")) != NULL) { if (STREQ(primary, "yes")) @@ -15459,6 +15463,24 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, } } + if (xres && yres) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("xres and yres attribute only supported for type of qxl")); + goto error; + } + if (virStrToLong_uip(xres, NULL, 10, &def->xres) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video xres '%s'"), xres); + goto error; + } + if (virStrToLong_uip(yres, NULL, 10, &def->yres) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video yres '%s'"), yres); + goto error; + } + } + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error; @@ -26427,6 +26449,10 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) virBufferAsprintf(buf, " heads='%u'", def->heads); + if (def->xres) + virBufferAsprintf(buf, " xres='%u'", def->xres); + if (def->yres) + virBufferAsprintf(buf, " yres='%u'", def->yres); if (def->primary) virBufferAddLit(buf, " primary='yes'"); if (def->accel) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8a4425df64..bfee86efcf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1419,6 +1419,8 @@ struct _virDomainVideoDef { unsigned int vram64; /* kibibytes (multiples of 1024) */ unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; + unsigned int xres; + unsigned int yres; bool primary; virDomainVideoAccelDefPtr accel; virDomainVideoDriverDefPtr driver; -- 2.20.1
On Mon, Aug 5, 2019 at 9:25 AM <jcfaracco@gmail.com> wrote:
From: Julio Faracco <jcfaracco@gmail.com>
XML need to support both properties together. This commit adds XML support for QXL model if they are set. Domain configuration is able to parse this properties.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 2 files changed, 28 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 441eb1a5a2..120c6ccf5f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15360,6 +15360,8 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) ram = NULL; VIR_AUTOFREE(char *) vgamem = NULL; VIR_AUTOFREE(char *) primary = NULL; + VIR_AUTOFREE(char *) xres = NULL; + VIR_AUTOFREE(char *) yres = NULL;
if (!(def = virDomainVideoDefNew())) return NULL; @@ -15377,6 +15379,8 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, vram64 = virXMLPropString(cur, "vram64"); vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads"); + xres = virXMLPropString(cur, "xres"); + yres = virXMLPropString(cur, "yres");
if ((primary = virXMLPropString(cur, "primary")) != NULL) { if (STREQ(primary, "yes")) @@ -15459,6 +15463,24 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, } }
+ if (xres && yres) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("xres and yres attribute only supported for type of qxl"));
The error msg is not right here. Not only qxl type support xres&yres, bus also VGA, virtio, bochs types. How about a write a general function for qemu hypervisor driver to verify if a video type support xres&yres, and then add xres&yres attribute to all the supported video types? BTW, there is a bug to track this feature: https://bugzilla.redhat.com/show_bug.cgi?id=1485793 Please mention the bug in your commit msg. Thank you
+ goto error; + } + if (virStrToLong_uip(xres, NULL, 10, &def->xres) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video xres '%s'"), xres); + goto error; + } + if (virStrToLong_uip(yres, NULL, 10, &def->yres) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video yres '%s'"), yres); + goto error; + } + } + if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0) goto error;
@@ -26427,6 +26449,10 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) virBufferAsprintf(buf, " heads='%u'", def->heads); + if (def->xres) + virBufferAsprintf(buf, " xres='%u'", def->xres); + if (def->yres) + virBufferAsprintf(buf, " yres='%u'", def->yres); if (def->primary) virBufferAddLit(buf, " primary='yes'"); if (def->accel) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 8a4425df64..bfee86efcf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1419,6 +1419,8 @@ struct _virDomainVideoDef { unsigned int vram64; /* kibibytes (multiples of 1024) */ unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; + unsigned int xres; + unsigned int yres; bool primary; virDomainVideoAccelDefPtr accel; virDomainVideoDriverDefPtr driver; -- 2.20.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Best regards, ----------------------------------- Han Han Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
On Sun, Aug 04, 2019 at 10:21:19PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
XML need to support both properties together. This commit adds XML support for QXL model if they are set. Domain configuration is able to
The commit message should show an example of the XML that is being added. Also, the XML->XML test should be a part of this commit. Jano
parse this properties.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 2 files changed, 28 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Mon, Aug 05, 2019 at 12:39:58PM +0200, Ján Tomko wrote:
On Sun, Aug 04, 2019 at 10:21:19PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
XML need to support both properties together. This commit adds XML support for QXL model if they are set. Domain configuration is able to
The commit message should show an example of the XML that is being added. Also, the XML->XML test should be a part of this commit.
Jano
parse this properties.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ 2 files changed, 28 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Oops, ignore this, I pressed the wrong key combination when sending the e-mail.
Jano
From: Julio Faracco <jcfaracco@gmail.com> Now, QEMU command line can define 'xres' and 'yres' if XML contains both properties from qxl model. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_command.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fee51158a9..82430e7e98 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4713,6 +4713,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (video->heads) virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); } + + if (video->xres && video->yres) + virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->xres, video->yres); + } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS)) { if (video->heads) -- 2.20.1
On Sun, Aug 04, 2019 at 10:21:20PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
Now, QEMU command line can define 'xres' and 'yres' if XML contains both properties from qxl model.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_command.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fee51158a9..82430e7e98 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4713,6 +4713,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (video->heads) virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); } + + if (video->xres && video->yres) + virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->xres, video->yres);
Capabilities changes are needed. The support for specifying video resolution was added in qemu 2.10.0, libvirt supports as old as 1.5.0. Unfortunately, device properties listing isn't QAPIfied yet, so we can't probe this properly. Erik
On Mon, Aug 05, 2019 at 08:43:14AM +0200, Erik Skultety wrote:
On Sun, Aug 04, 2019 at 10:21:20PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
Now, QEMU command line can define 'xres' and 'yres' if XML contains both properties from qxl model.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_command.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fee51158a9..82430e7e98 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4713,6 +4713,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (video->heads) virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); } + + if (video->xres && video->yres) + virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->xres, video->yres);
Capabilities changes are needed. The support for specifying video resolution was added in qemu 2.10.0, libvirt supports as old as 1.5.0. Unfortunately, device properties listing isn't QAPIfied yet, so we can't probe this properly.
Not sure how QAPI comes into play here, but we already do probe for qxl's other properties and git grep "xres" tests does show hits from replies files from 2.10.0 on. Jano
Erik
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Aug 05, 2019 at 12:30:48PM +0200, Ján Tomko wrote:
On Mon, Aug 05, 2019 at 08:43:14AM +0200, Erik Skultety wrote:
On Sun, Aug 04, 2019 at 10:21:20PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
Now, QEMU command line can define 'xres' and 'yres' if XML contains both properties from qxl model.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/qemu/qemu_command.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fee51158a9..82430e7e98 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4713,6 +4713,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, if (video->heads) virBufferAsprintf(&buf, ",max_outputs=%u", video->heads); } + + if (video->xres && video->yres) + virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->xres, video->yres);
Capabilities changes are needed. The support for specifying video resolution was added in qemu 2.10.0, libvirt supports as old as 1.5.0. Unfortunately, device properties listing isn't QAPIfied yet, so we can't probe this properly.
Not sure how QAPI comes into play here, but we already do probe for qxl's other properties and git grep "xres" tests does show hits from replies files from 2.10.0 on.
I stand corrected, I must have overlooked, in which case, there shouldn't really be any problem in querying the property properly. Erik
From: Julio Faracco <jcfaracco@gmail.com> New tests to verify resolution properties of a simple qxl video. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- .../video-qxl-resolution.args | 32 +++++++++++++++ .../qemuxml2argvdata/video-qxl-resolution.xml | 40 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../video-qxl-resolution.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 117 insertions(+) create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.args create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.xml create mode 100644 tests/qemuxml2xmloutdata/video-qxl-resolution.xml diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.args b/tests/qemuxml2argvdata/video-qxl-resolution.args new file mode 100644 index 0000000000..71370ff735 --- /dev/null +++ b/tests/qemuxml2argvdata/video-qxl-resolution.args @@ -0,0 +1,32 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name QEMUGuest1 \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,max_outputs=1,\ +xres=1280,yres=720,bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.xml b/tests/qemuxml2argvdata/video-qxl-resolution.xml new file mode 100644 index 0000000000..c6275c1bc5 --- /dev/null +++ b/tests/qemuxml2argvdata/video-qxl-resolution.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' xres='1280' yres='720' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c166fd18d6..b67ba8f135 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2003,6 +2003,10 @@ mymain(void) QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_QXL_MAX_OUTPUTS); + DO_TEST("video-qxl-resolution", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS); DO_TEST("video-virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); diff --git a/tests/qemuxml2xmloutdata/video-qxl-resolution.xml b/tests/qemuxml2xmloutdata/video-qxl-resolution.xml new file mode 100644 index 0000000000..c6275c1bc5 --- /dev/null +++ b/tests/qemuxml2xmloutdata/video-qxl-resolution.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' xres='1280' yres='720' primary='yes'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 525eb9a740..62485e89d9 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1189,6 +1189,7 @@ mymain(void) QEMU_CAPS_DEVICE_QXL); DO_TEST("video-qxl-heads", NONE); DO_TEST("video-qxl-noheads", NONE); + DO_TEST("video-qxl-resolution", NONE); DO_TEST("video-virtio-gpu-secondary", NONE); DO_TEST("video-virtio-gpu-ccw", QEMU_CAPS_CCW, -- 2.20.1
On Sun, Aug 04, 2019 at 10:21:21PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
New tests to verify resolution properties of a simple qxl video.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- .../video-qxl-resolution.args | 32 +++++++++++++++
xml->argv test should be introduced along with the qemu_command changes.
.../qemuxml2argvdata/video-qxl-resolution.xml | 40 +++++++++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../video-qxl-resolution.xml | 40 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 5 files changed, 117 insertions(+) create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.args create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.xml create mode 100644 tests/qemuxml2xmloutdata/video-qxl-resolution.xml
diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.args b/tests/qemuxml2argvdata/video-qxl-resolution.args new file mode 100644 index 0000000000..71370ff735 diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.xml b/tests/qemuxml2argvdata/video-qxl-resolution.xml new file mode 100644 index 0000000000..c6275c1bc5 --- /dev/null +++ b/tests/qemuxml2argvdata/video-qxl-resolution.xml @@ -0,0 +1,40 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-i686</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk>
No need for a disk for testing graphics devices.
+ <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <video> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' xres='1280' yres='720' primary='yes'/>
I'm not a fan of blindly copying qemu's laconic arguments. Also, for other devices, we'd put such arguments into the <driver> element, not <model>. So I propose either: <model> <resolution x='1280' y='720'/> </model> or <driver> <resolution x='1280' y='720'/> </driver>
+ <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c166fd18d6..b67ba8f135 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2003,6 +2003,10 @@ mymain(void) QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_QXL_MAX_OUTPUTS); + DO_TEST("video-qxl-resolution", + QEMU_CAPS_DEVICE_VIDEO_PRIMARY, + QEMU_CAPS_DEVICE_QXL, + QEMU_CAPS_QXL_MAX_OUTPUTS);
Please use DO_TEST_CAPS_LATEST which runs the test with the latest QEMU capabilities we have collected from a real QEMU instance. With manually enumerating the capabilities there's a risk of missing some. If you want to test it against an older QEMU version which lacked some capability, there's DO_TEST_CAPS_VER Jano
DO_TEST("video-virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
On Sun, Aug 04, 2019 at 10:21:17PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
This serie adds 'xres' and 'yres' properties into XML definition for QXL video device to specify a default resolution. This serie covers a simple test case too.
Why limit this to qxl? virtio and stdvga support xres+yres too. Note: on stdvga this requires edid support (available since 3.1, enabled by default since 4.1). cheers, Gerd
On Mon, Aug 05, 2019 at 09:12:26AM +0200, Gerd Hoffmann wrote:
On Sun, Aug 04, 2019 at 10:21:17PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
This serie adds 'xres' and 'yres' properties into XML definition for QXL video device to specify a default resolution. This serie covers a simple test case too.
Why limit this to qxl?
virtio and stdvga support xres+yres too.
Note: on stdvga this requires edid support (available since 3.1, enabled by default since 4.1).
Gerd, I'd already responded by the time you had a look, so just to confirm that I'd looked at the schema properly, it isn't possible to query support for this from QAPI, is it? Regards, Erik
On Mon, Aug 05, 2019 at 11:17:35AM +0200, Erik Skultety wrote:
On Mon, Aug 05, 2019 at 09:12:26AM +0200, Gerd Hoffmann wrote:
On Sun, Aug 04, 2019 at 10:21:17PM -0300, jcfaracco@gmail.com wrote:
From: Julio Faracco <jcfaracco@gmail.com>
This serie adds 'xres' and 'yres' properties into XML definition for QXL video device to specify a default resolution. This serie covers a simple test case too.
Why limit this to qxl?
virtio and stdvga support xres+yres too.
Note: on stdvga this requires edid support (available since 3.1, enabled by default since 4.1).
Gerd, I'd already responded by the time you had a look, so just to confirm that I'd looked at the schema properly, it isn't possible to query support for this from QAPI, is it?
You can check this using "qemu -device VGA,help". I'm not sure whenever there is a QAPI version of that. cheers, Gerd
participants (5)
-
Erik Skultety -
Gerd Hoffmann -
Han Han -
jcfaracco＠gmail.com -
Ján Tomko