From: Julio Faracco <jcfaracco(a)gmail.com&gt; This commit adds resolution element with parameters 'x' and 'y' into video XML domain group definition. Both, properties were added into an element called 'resolution' and it was added inside 'model' element. They are set as optional. This element does not follow QEMU properties 'xres' and 'yres' format. Both HTML documentation and schema were changed too. This commit includes a simple test case to cover resolution for QEMU video models. The new XML format for resolution looks like: <model ...> <resolution x='800' y='600'/> </model> Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com&gt; --- docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 10 +++ src/conf/domain_conf.c | 63 ++++++++++++++++++- src/conf/domain_conf.h | 5 ++ src/conf/virconftypes.h | 3 + .../video-qxl-resolution.args | 32 ++++++++++ .../qemuxml2argvdata/video-qxl-resolution.xml | 40 ++++++++++++ tests/qemuxml2argvtest.c | 4 ++ .../video-qxl-resolution.xml | 40 ++++++++++++ tests/qemuxml2xmltest.c | 1 + 10 files changed, 201 insertions(+), 2 deletions(-) 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/docs/formatdomain.html.in b/docs/formatdomain.html.in index 500f114f41..962766b792 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7077,7 +7077,10 @@ qemu-kvm -net nic,model=? /dev/null <code>vgamem</code> (<span class="since">since 1.2.11</span>) to set the size of VGA framebuffer for fallback mode of QXL device. Attribute <code>vram64</code> (<span class="since">since 1.3.3</span>) - extends secondary bar and makes it addressable as 64bit memory. + extends secondary bar and makes it addressable as 64bit memory. For + resolution settings, there are <code>x</code> and <code>y</code> + (<span class="since">since 5.9.0</span>) optional attributes to set + minimum resolution for model. </p> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index ead5a25068..e06f892da3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3656,6 +3656,16 @@ </optional> </element> </optional> + <optional> + <element name="resolution"> + <attribute name="x"> + <ref name="unsignedInt"/> + </attribute> + <attribute name="y"> + <ref name="unsignedInt"/> + </attribute> + </element> + </optional> </element> </optional> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2e6a113de3..88e93f6fb8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15345,6 +15345,52 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node) return def; } +static virDomainVideoResolutionDefPtr +virDomainVideoResolutionDefParseXML(xmlNodePtr node) +{ + xmlNodePtr cur; + virDomainVideoResolutionDefPtr def; + g_autofree char *x = NULL; + g_autofree char *y = NULL; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE) { + if (!x && !y && + virXMLNodeNameEqual(cur, "resolution")) { + x = virXMLPropString(cur, "x"); + y = virXMLPropString(cur, "y"); + } + } + cur = cur->next; + } + + if (!x || !y) + return NULL; + + if (VIR_ALLOC(def) < 0) + goto cleanup; + + if (x) { + if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot parse video x-resolution '%s'"), x); + goto cleanup; + } + } + + if (y) { + if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot parse video y-resolution '%s'"), y); + goto cleanup; + } + } + + cleanup: + return def; +} + static virDomainVideoDriverDefPtr virDomainVideoDriverDefParseXML(xmlNodePtr node) { @@ -15424,6 +15470,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, } def->accel = virDomainVideoAccelDefParseXML(cur); + def->res = virDomainVideoResolutionDefParseXML(cur); } if (virXMLNodeNameEqual(cur, "driver")) { if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) @@ -26515,6 +26562,18 @@ virDomainVideoAccelDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } +static void +virDomainVideoResolutionDefFormat(virBufferPtr buf, + virDomainVideoResolutionDefPtr def) +{ + virBufferAddLit(buf, "<resolution"); + if (def->x && def->y) { + virBufferAsprintf(buf, " x='%u' y='%u'", + def->x, def->y); + } + virBufferAddLit(buf, "/>\n"); +} + static int virDomainVideoDefFormat(virBufferPtr buf, virDomainVideoDefPtr def, @@ -26562,11 +26621,13 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " heads='%u'", def->heads); if (def->primary) virBufferAddLit(buf, " primary='yes'"); - if (def->accel) { + if (def->accel || def->res) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); if (def->accel) virDomainVideoAccelDefFormat(buf, def->accel); + if (def->res) + virDomainVideoResolutionDefFormat(buf, def->res); virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</model>\n"); } else { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index edac6250e4..b33e5334f4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1421,6 +1421,10 @@ struct _virDomainVideoAccelDef { char *rendernode; }; +struct _virDomainVideoResolutionDef { + unsigned int x; + unsigned int y; +}; struct _virDomainVideoDriverDef { virDomainVideoVGAConf vgaconf; @@ -1438,6 +1442,7 @@ struct _virDomainVideoDef { unsigned int heads; bool primary; virDomainVideoAccelDefPtr accel; + virDomainVideoResolutionDefPtr res; virDomainVideoDriverDefPtr driver; virDomainDeviceInfo info; virDomainVirtioOptionsPtr virtio; diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h index a15cfb5f9e..462842f324 100644 --- a/src/conf/virconftypes.h +++ b/src/conf/virconftypes.h @@ -324,6 +324,9 @@ typedef virDomainVcpuDef *virDomainVcpuDefPtr; typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef; typedef virDomainVideoAccelDef *virDomainVideoAccelDefPtr; +typedef struct _virDomainVideoResolutionDef virDomainVideoResolutionDef; +typedef virDomainVideoResolutionDef *virDomainVideoResolutionDefPtr; + typedef struct _virDomainVideoDef virDomainVideoDef; typedef virDomainVideoDef *virDomainVideoDefPtr; diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.args b/tests/qemuxml2argvdata/video-qxl-resolution.args new file mode 100644 index 0000000000..1dbcd660f1 --- /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,vgamem_mb=8,\ +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..6ba2817002 --- /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' 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 5212ce50bd..90edd7a844 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2056,6 +2056,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_VGAMEM); 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..6ba2817002 --- /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' 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 b9364f942f..4c7ba98367 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -1210,6 +1210,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 Thu, 2019-10-17 at 14:12 -0500, Jonathon Jongsma wrote: Oops, I sent the review just as I noticed another small issue. In this parse function, you parse the x and the y resolution values separately, which implies that it's valid to specify only one or the other. In other words, this function will not report an error for the following configuration: <model ...> <resolution x='800'/> </model> However, below in virDomainVideoDefFormat(), you refuse to format the resolution values unless *both* x and y are non-zero. (similarly, in the following patch, you only generate the qemu commandline arguments if both parameters are non-zero in qemuBuildDeviceVideoStr()). If both x and y are required, you should probably enforce that in this function. Jonathon

At least, I need to fix the leak problem. ;-) I can grab other problems too. I will submit a fix/patch soon. -- Julio Faracco Em qui, 17 de out de 2019 às 17:33, Cole Robinson <crobinso(a)redhat.com&gt; escreveu:

From: Julio Faracco <jcfaracco(a)gmail.com&gt; This commit let QEMU command line define 'xres' and 'yres' properties if XML contains both properties from video model: based on resolution fields 'x' and 'y'. There is a conditional structure inside qemuDomainDeviceDefValidateVideo() that validates if video model supports this feature. This commit includes the necessary changes to cover resolution for 'video-qxl-resolution' test cases too. Signed-off-by: Julio Faracco <jcfaracco(a)gmail.com&gt; --- src/qemu/qemu_command.c | 5 +++++ src/qemu/qemu_domain.c | 11 +++++++++++ tests/qemuxml2argvdata/video-qxl-resolution.args | 2 +- tests/qemuxml2argvdata/video-qxl-resolution.xml | 4 +++- tests/qemuxml2xmloutdata/video-qxl-resolution.xml | 4 +++- 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e98195b1d7..b579b994f3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4598,6 +4598,11 @@ qemuBuildDeviceVideoStr(const virDomainDef *def, virBufferAsprintf(&buf, ",vgamem=%uk", video->vram); } + if (video->res && video->res->x && video->res->y) { + /* QEMU accepts resolution xres and yres. */ + virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->res->x, video->res->y); + } + if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) return NULL; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 09b6c9a570..a97bf65e7f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5774,6 +5774,17 @@ qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video) } } + if (video->type != VIR_DOMAIN_VIDEO_TYPE_VGA && + video->type != VIR_DOMAIN_VIDEO_TYPE_QXL && + video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO && + video->type != VIR_DOMAIN_VIDEO_TYPE_BOCHS) { + if (video->res) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("model resolution is not supported")); + return -1; + } + } + if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA || video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA) { if (video->vram && video->vram < 1024) { diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.args b/tests/qemuxml2argvdata/video-qxl-resolution.args index 1dbcd660f1..ed2e4422da 100644 --- a/tests/qemuxml2argvdata/video-qxl-resolution.args +++ b/tests/qemuxml2argvdata/video-qxl-resolution.args @@ -28,5 +28,5 @@ server,nowait \ -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,vgamem_mb=8,\ -bus=pci.0,addr=0x2 \ +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 index 6ba2817002..1bc415f3f8 100644 --- a/tests/qemuxml2argvdata/video-qxl-resolution.xml +++ b/tests/qemuxml2argvdata/video-qxl-resolution.xml @@ -30,7 +30,9 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <video> - <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'/> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'> + <resolution x='1280' y='720'/> + </model> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> diff --git a/tests/qemuxml2xmloutdata/video-qxl-resolution.xml b/tests/qemuxml2xmloutdata/video-qxl-resolution.xml index 6ba2817002..1bc415f3f8 100644 --- a/tests/qemuxml2xmloutdata/video-qxl-resolution.xml +++ b/tests/qemuxml2xmloutdata/video-qxl-resolution.xml @@ -30,7 +30,9 @@ <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> <video> - <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'/> + <model type='qxl' ram='65536' vram='65536' vgamem='8192' heads='1' primary='yes'> + <resolution x='1280' y='720'/> + </model> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> </video> <memballoon model='virtio'> -- 2.20.1