[libvirt] [PATCH 0/3] implement vram64 attribute for QXL video device
Pavel Hrdina (3): qemu_capabilities: introduce QEMU_CAPS_QXL(_VGA)_VRAM64 docs/formatdomain: rewrite video documentation qemu: introduce vram64 attribute for QXL video device docs/formatdomain.html.in | 18 +++++++----- docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 34 ++++++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 4 +++ src/qemu/qemu_capabilities.h | 2 ++ src/qemu/qemu_command.c | 13 +++++++++ src/qemu/qemu_monitor_json.c | 8 +++++ tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 2 ++ .../qemuxml2argv-video-qxl-device-vram64.args | 25 ++++++++++++++++ .../qemuxml2argv-video-qxl-device-vram64.xml | 29 ++++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.args | 27 +++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.xml | 32 ++++++++++++++++++++ 22 files changed, 204 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml -- 2.7.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_capabilities.c | 4 ++++ src/qemu/qemu_capabilities.h | 2 ++ tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.4.0-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.5.0-1.caps | 2 ++ tests/qemucapabilitiesdata/caps_2.6.0-1.caps | 2 ++ 12 files changed, 26 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 2b953ea..28d6b7a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -315,6 +315,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "vserport-change-event", /* 210 */ "virtio-balloon-pci.deflate-on-oom", "mptsas1068", + "qxl.vram64_size_mb", + "qxl-vga.vram64_size_mb", ); @@ -1652,10 +1654,12 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVmwareSvga[] = { static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxl[] = { { "vgamem_mb", QEMU_CAPS_QXL_VGAMEM }, + { "vram64_size_mb", QEMU_CAPS_QXL_VRAM64 }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = { { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM }, + { "vram64_size_mb", QEMU_CAPS_QXL_VGA_VRAM64 }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b73c529..5cb2e63 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -344,6 +344,8 @@ typedef enum { QEMU_CAPS_VIRTIO_BALLOON_AUTODEFLATE, /* virtio-balloon-{device,pci,ccw}. * deflate-on-oom */ QEMU_CAPS_SCSI_MPTSAS1068, /* -device mptsas1068 */ + QEMU_CAPS_QXL_VRAM64, /* -device qxl.vram64_size_mb */ + QEMU_CAPS_QXL_VGA_VRAM64, /* -device qxl-vga.vram64_size_mb */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps index 34ddd80..2e452ea 100644 --- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps @@ -112,4 +112,6 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps index cb8eac9..5ad56aa 100644 --- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps @@ -127,4 +127,6 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps index 86982f1..d0341fd 100644 --- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps @@ -128,4 +128,6 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps index 5401d65..93ea687 100644 --- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps @@ -137,4 +137,6 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps index 63536a7..c25b076 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps @@ -143,4 +143,6 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps index 6717a94..30b70e9 100644 --- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps +++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps @@ -143,4 +143,6 @@ <flag name='rtl8139'/> <flag name='e1000'/> <flag name='virtio-net'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps index 332b85a..fd2639d 100644 --- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps @@ -160,4 +160,6 @@ <flag name='e1000'/> <flag name='virtio-net'/> <flag name='vserport-change-event'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps index e411542..4b61fc1 100644 --- a/tests/qemucapabilitiesdata/caps_2.4.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.4.0-1.caps @@ -171,4 +171,6 @@ <flag name='ich9-disable-s4'/> <flag name='vserport-change-event'/> <flag name='virtio-balloon-pci.deflate-on-oom'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps index 931bc4f..2061d0e 100644 --- a/tests/qemucapabilitiesdata/caps_2.5.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.5.0-1.caps @@ -172,4 +172,6 @@ <flag name='ich9-disable-s4'/> <flag name='vserport-change-event'/> <flag name='virtio-balloon-pci.deflate-on-oom'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps index f32d5aa..8d78b82 100644 --- a/tests/qemucapabilitiesdata/caps_2.6.0-1.caps +++ b/tests/qemucapabilitiesdata/caps_2.6.0-1.caps @@ -174,4 +174,6 @@ <flag name='ich9-disable-s4'/> <flag name='vserport-change-event'/> <flag name='virtio-balloon-pci.deflate-on-oom'/> + <flag name='qxl.vram64_size_mb'/> + <flag name='qxl-vga.vram64_size_mb'/> </qemuCaps> -- 2.7.1
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a524c17..3fcd728 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5182,15 +5182,15 @@ qemu-kvm -net nic,model=? /dev/null supported only for guests type of "vz", "kvm", "vbox" and "vmx". </p> <p> - For guest type of kvm the optional attribute <code>ram</code> - (<span class="since">since 1.0.2</span>) is allowed for "qxl" type - only and specifies the size of the primary bar, while the optional + For guest type of "kvm" or "qemu" and model type "qxl" there are + optional attributes. Attribute <code>ram</code> (<span class="since"> + since 1.0.2</span>) specifies the size of the primary bar, while the attribute <code>vram</code> specifies the secondary bar size. - If "ram" or "vram" are not supplied a default value is used. The ram - should also be rounded to power of two as vram. There is also optional - attribute <code>vgamem</code> (<span class="since">since 1.2.11 (QEMU - only)</span>) to set the size of VGA framebuffer for fallback mode of - QXL device. + If <code>ram</code> or <code>vram</code> are not supplied a default + value is used. The <code>ram</code> should also be rounded to power of + two as <code>vram</code>. There is also optional attribute + <code>vgamem</code> (<span class="since">since 1.2.11</span>) to set + the size of VGA framebuffer for fallback mode of QXL device. </p> </dd> -- 2.7.1
This attribute is used to extend secondary PCI bar and expose it to the guest as 64bit memory. It works like this: attribute vram is there to set size of secondary PCI bar and guest sees it as 32bit memory, attribute vram64 can extend this secondary PCI bar. If both attributes are used, guest sees two memory bars, both address the same memory, with the difference that the 32bit bar can address only the first part of the whole memory. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260749 Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 2 ++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 34 ++++++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++ src/qemu/qemu_monitor_json.c | 8 +++++ .../qemuxml2argv-video-qxl-device-vram64.args | 25 ++++++++++++++++ .../qemuxml2argv-video-qxl-device-vram64.xml | 29 ++++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.args | 27 +++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.xml | 32 ++++++++++++++++++++ 10 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3fcd728..318ffd9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5191,6 +5191,8 @@ qemu-kvm -net nic,model=? /dev/null two as <code>vram</code>. There is also optional attribute <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.2</span>) + extends secondary bar and makes it addressable as 64bit memory. </p> </dd> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67af93a..fe5eaf0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2938,6 +2938,11 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="vram64"> + <ref name="unsignedInt"/> + </attribute> + </optional> </group> </choice> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56bd1aa..76fc52a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11898,6 +11898,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *vram64 = NULL; char *ram = NULL; char *vgamem = NULL; char *primary = NULL; @@ -11913,6 +11914,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, type = virXMLPropString(cur, "type"); ram = virXMLPropString(cur, "ram"); vram = virXMLPropString(cur, "vram"); + vram64 = virXMLPropString(cur, "vram64"); vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads"); @@ -11967,6 +11969,19 @@ virDomainVideoDefParseXML(xmlNodePtr node, def->vram = virDomainVideoDefaultRAM(dom, def->type); } + if (vram64) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("vram64 attribute only supported for type of qxl")); + goto error; + } + if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video vram64 '%s'"), vram64); + goto error; + } + } + if (vgamem) { if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -11993,9 +12008,11 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error; + cleanup: VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); + VIR_FREE(vram64); VIR_FREE(vgamem); VIR_FREE(heads); @@ -12003,12 +12020,8 @@ virDomainVideoDefParseXML(xmlNodePtr node, error: virDomainVideoDefFree(def); - VIR_FREE(type); - VIR_FREE(ram); - VIR_FREE(vram); - VIR_FREE(vgamem); - VIR_FREE(heads); - return NULL; + def = NULL; + goto cleanup; } static virDomainHostdevDefPtr @@ -17023,6 +17036,13 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, return false; } + if (src->vram64 != dst->vram64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target video card vram64 %u does not match source %u"), + dst->vram64, src->vram64); + return false; + } + if (src->vgamem != dst->vgamem) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card vgamem %u does not match source %u"), @@ -20708,6 +20728,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " ram='%u'", def->ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); + if (def->vram64) + virBufferAsprintf(buf, " vram64='%u'", def->vram64); if (def->vgamem) virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1de3be3..c3a7386 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1399,6 +1399,7 @@ struct _virDomainVideoDef { int type; unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ + unsigned int vram64; /* kibibytes (multiples of 1024) */ unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; bool primary; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 78423e7..24ddd8a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3347,6 +3347,12 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); } + if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) || + (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VRAM64))) { + /* QEMU accepts mebibytes for vram64_size_mb. */ + virBufferAsprintf(&buf, ",vram64_size_mb=%u", video->vram64 / 1024); + } + if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) || (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGAMEM))) { /* QEMU accepts mebibytes for vgamem_mb. */ @@ -8254,6 +8260,7 @@ qemuBuildCommandLine(virConnectPtr conn, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { unsigned int ram = def->videos[0]->ram; unsigned int vram = def->videos[0]->vram; + unsigned int vram64 = def->videos[0]->vram64; unsigned int vgamem = def->videos[0]->vgamem; if (vram > (UINT_MAX / 1024)) { @@ -8279,6 +8286,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "%s.vram_size=%u", dev, vram * 1024); } + if (vram64 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram64_size_mb=%u", + dev, vram64 / 1024); + } if (vgamem && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { virCommandAddArg(cmd, "-global"); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2b641f..34efd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1436,6 +1436,14 @@ qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, return -1; } video->vram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, + "vram64_size_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Object '%s' has no property 'vram64_size_mb'"), + path); + return -1; + } + video->vram64 = prop.val.ul / 1024; if (qemuMonitorJSONGetObjectProperty(mon, path, "ram_size", &prop) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("QOM Object '%s' has no property 'ram_size'"), diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args new file mode 100644 index 0000000..b9e65ea --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 1024 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\ +id=drive-ide0-0-0,cache=none \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ +vram64_size_mb=128,vgamem_mb=16,bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml new file mode 100644 index 0000000..1e89d06 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='x86_64' 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-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='qxl' vram64='131072' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args new file mode 100644 index 0000000..fadc3ed --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 1024 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\ +id=drive-ide0-0-0,cache=none \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vgamem_mb=16,\ +bus=pci.0,addr=0x2 \ +-device qxl,id=video1,ram_size=67108864,vram_size=67108864,vram64_size_mb=128,\ +vgamem_mb=16,bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml new file mode 100644 index 0000000..72e8bad --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu>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</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='qxl' heads='1'/> + </video> + <video> + <model type='qxl' vram64='131072' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> -- 2.7.1
On Mon, Feb 22, 2016 at 02:34:14PM +0100, Pavel Hrdina wrote:
This attribute is used to extend secondary PCI bar and expose it to the guest as 64bit memory. It works like this: attribute vram is there to set size of secondary PCI bar and guest sees it as 32bit memory, attribute vram64 can extend this secondary PCI bar. If both attributes are used, guest sees two memory bars, both address the same memory, with the difference that the 32bit bar can address only the first part of the whole memory.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260749
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 2 ++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 34 ++++++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++ src/qemu/qemu_monitor_json.c | 8 +++++ .../qemuxml2argv-video-qxl-device-vram64.args | 25 ++++++++++++++++ .../qemuxml2argv-video-qxl-device-vram64.xml | 29 ++++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.args | 27 +++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.xml | 32 ++++++++++++++++++++ 10 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3fcd728..318ffd9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5191,6 +5191,8 @@ qemu-kvm -net nic,model=? /dev/null two as <code>vram</code>. There is also optional attribute <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.2</span>) + extends secondary bar and makes it addressable as 64bit memory. </p> </dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67af93a..fe5eaf0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2938,6 +2938,11 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="vram64"> + <ref name="unsignedInt"/> + </attribute> + </optional> </group> </choice> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56bd1aa..76fc52a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11898,6 +11898,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *vram64 = NULL; char *ram = NULL; char *vgamem = NULL; char *primary = NULL; @@ -11913,6 +11914,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, type = virXMLPropString(cur, "type"); ram = virXMLPropString(cur, "ram"); vram = virXMLPropString(cur, "vram"); + vram64 = virXMLPropString(cur, "vram64"); vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads");
@@ -11967,6 +11969,19 @@ virDomainVideoDefParseXML(xmlNodePtr node, def->vram = virDomainVideoDefaultRAM(dom, def->type); }
+ if (vram64) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("vram64 attribute only supported for type of qxl")); + goto error; + } + if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video vram64 '%s'"), vram64); + goto error; + } + } + if (vgamem) { if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -11993,9 +12008,11 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error;
+ cleanup: VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); + VIR_FREE(vram64); VIR_FREE(vgamem); VIR_FREE(heads);
@@ -12003,12 +12020,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
error: virDomainVideoDefFree(def); - VIR_FREE(type); - VIR_FREE(ram); - VIR_FREE(vram); - VIR_FREE(vgamem); - VIR_FREE(heads); - return NULL; + def = NULL; + goto cleanup; }
static virDomainHostdevDefPtr @@ -17023,6 +17036,13 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, return false; }
+ if (src->vram64 != dst->vram64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target video card vram64 %u does not match source %u"), + dst->vram64, src->vram64); + return false; + } +
Does this check make sense if we're updating that value after QEMU starts?
if (src->vgamem != dst->vgamem) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card vgamem %u does not match source %u"), @@ -20708,6 +20728,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " ram='%u'", def->ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); + if (def->vram64) + virBufferAsprintf(buf, " vram64='%u'", def->vram64); if (def->vgamem) virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1de3be3..c3a7386 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1399,6 +1399,7 @@ struct _virDomainVideoDef { int type; unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ + unsigned int vram64; /* kibibytes (multiples of 1024) */ unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; bool primary; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 78423e7..24ddd8a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3347,6 +3347,12 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); }
+ if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) || + (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VRAM64))) { + /* QEMU accepts mebibytes for vram64_size_mb. */ + virBufferAsprintf(&buf, ",vram64_size_mb=%u", video->vram64 / 1024); + } +
Why do we both set the size here (for both primary and secondary card), ...
if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) || (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGAMEM))) { /* QEMU accepts mebibytes for vgamem_mb. */ @@ -8254,6 +8260,7 @@ qemuBuildCommandLine(virConnectPtr conn, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { unsigned int ram = def->videos[0]->ram; unsigned int vram = def->videos[0]->vram; + unsigned int vram64 = def->videos[0]->vram64; unsigned int vgamem = def->videos[0]->vgamem;
if (vram > (UINT_MAX / 1024)) { @@ -8279,6 +8286,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "%s.vram_size=%u", dev, vram * 1024); } + if (vram64 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram64_size_mb=%u", + dev, vram64 / 1024); + }
... and also set it here? And mostly why do we check only for primary card capability here? I remember that this was needed for some parameters when QEMU_CAPS_DEVICE was a variable (before we switched to supporting only QEMU_CAPS_DEVICE-enabled versions). Although the code does the opposite thing (uses '-global' only for qemu with '-device' supported). I must be clearly on a bad track here.
if (vgamem && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { virCommandAddArg(cmd, "-global"); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2b641f..34efd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1436,6 +1436,14 @@ qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, return -1; } video->vram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, + "vram64_size_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Object '%s' has no property 'vram64_size_mb'"), + path); + return -1; + } + video->vram64 = prop.val.ul / 1024;
And a last question: why don't we just skip the vram64_size_mb if it's not available and move on to the next one? What if it's a bit older QEMU? For all the questions a short explanation might be enough as all the other properties are coded following the same fashion. That doesn't make them correct, though, so the explanation is still needed.
if (qemuMonitorJSONGetObjectProperty(mon, path, "ram_size", &prop) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("QOM Object '%s' has no property 'ram_size'"), diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args new file mode 100644 index 0000000..b9e65ea --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args @@ -0,0 +1,25 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 1024 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\ +id=drive-ide0-0-0,cache=none \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ +vram64_size_mb=128,vgamem_mb=16,bus=pci.0,addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml new file mode 100644 index 0000000..1e89d06 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu>1</vcpu> + <os> + <type arch='x86_64' 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-x86_64</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='qxl' vram64='131072' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args new file mode 100644 index 0000000..fadc3ed --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-m 1024 \ +-smp 1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/var/lib/libvirt/images/QEMUGuest1,format=qcow2,if=none,\ +id=drive-ide0-0-0,cache=none \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vgamem_mb=16,\ +bus=pci.0,addr=0x2 \ +-device qxl,id=video1,ram_size=67108864,vram_size=67108864,vram64_size_mb=128,\ +vgamem_mb=16,bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml new file mode 100644 index 0000000..72e8bad --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu>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</emulator> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2' cache='none'/> + <source file='/var/lib/libvirt/images/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <video> + <model type='qxl' heads='1'/> + </video> + <video> + <model type='qxl' vram64='131072' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> -- 2.7.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 22, 2016 at 04:16:49PM +0100, Martin Kletzander wrote:
On Mon, Feb 22, 2016 at 02:34:14PM +0100, Pavel Hrdina wrote:
This attribute is used to extend secondary PCI bar and expose it to the guest as 64bit memory. It works like this: attribute vram is there to set size of secondary PCI bar and guest sees it as 32bit memory, attribute vram64 can extend this secondary PCI bar. If both attributes are used, guest sees two memory bars, both address the same memory, with the difference that the 32bit bar can address only the first part of the whole memory.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260749
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 2 ++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 34 ++++++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++ src/qemu/qemu_monitor_json.c | 8 +++++ .../qemuxml2argv-video-qxl-device-vram64.args | 25 ++++++++++++++++ .../qemuxml2argv-video-qxl-device-vram64.xml | 29 ++++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.args | 27 +++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.xml | 32 ++++++++++++++++++++ 10 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3fcd728..318ffd9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5191,6 +5191,8 @@ qemu-kvm -net nic,model=? /dev/null two as <code>vram</code>. There is also optional attribute <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.2</span>) + extends secondary bar and makes it addressable as 64bit memory. </p> </dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67af93a..fe5eaf0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2938,6 +2938,11 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="vram64"> + <ref name="unsignedInt"/> + </attribute> + </optional> </group> </choice> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56bd1aa..76fc52a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11898,6 +11898,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *vram64 = NULL; char *ram = NULL; char *vgamem = NULL; char *primary = NULL; @@ -11913,6 +11914,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, type = virXMLPropString(cur, "type"); ram = virXMLPropString(cur, "ram"); vram = virXMLPropString(cur, "vram"); + vram64 = virXMLPropString(cur, "vram64"); vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads");
@@ -11967,6 +11969,19 @@ virDomainVideoDefParseXML(xmlNodePtr node, def->vram = virDomainVideoDefaultRAM(dom, def->type); }
+ if (vram64) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("vram64 attribute only supported for type of qxl")); + goto error; + } + if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video vram64 '%s'"), vram64); + goto error; + } + } + if (vgamem) { if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -11993,9 +12008,11 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error;
+ cleanup: VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); + VIR_FREE(vram64); VIR_FREE(vgamem); VIR_FREE(heads);
@@ -12003,12 +12020,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
error: virDomainVideoDefFree(def); - VIR_FREE(type); - VIR_FREE(ram); - VIR_FREE(vram); - VIR_FREE(vgamem); - VIR_FREE(heads); - return NULL; + def = NULL; + goto cleanup; }
static virDomainHostdevDefPtr @@ -17023,6 +17036,13 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, return false; }
+ if (src->vram64 != dst->vram64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target video card vram64 %u does not match source %u"), + dst->vram64, src->vram64); + return false; + } +
Does this check make sense if we're updating that value after QEMU starts?
Yes, it makes sense, this will ensure, that user doesn't change this value via qemu hook.
if (src->vgamem != dst->vgamem) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card vgamem %u does not match source %u"), @@ -20708,6 +20728,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " ram='%u'", def->ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); + if (def->vram64) + virBufferAsprintf(buf, " vram64='%u'", def->vram64); if (def->vgamem) virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1de3be3..c3a7386 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1399,6 +1399,7 @@ struct _virDomainVideoDef { int type; unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ + unsigned int vram64; /* kibibytes (multiples of 1024) */ unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; bool primary; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 78423e7..24ddd8a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3347,6 +3347,12 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); }
+ if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) || + (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VRAM64))) { + /* QEMU accepts mebibytes for vram64_size_mb. */ + virBufferAsprintf(&buf, ",vram64_size_mb=%u", video->vram64 / 1024); + } +
Why do we both set the size here (for both primary and secondary card), ...
This code is a part of for cycle for all video devices. This means, if current video device is primary, we need to check different flag than for secondary video device. Note: Both secondary and primary qxl video device are internally represented by the same device model in qemu, so both of them will ether have this feature or not.
if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) || (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGAMEM))) { /* QEMU accepts mebibytes for vgamem_mb. */ @@ -8254,6 +8260,7 @@ qemuBuildCommandLine(virConnectPtr conn, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { unsigned int ram = def->videos[0]->ram; unsigned int vram = def->videos[0]->vram; + unsigned int vram64 = def->videos[0]->vram64; unsigned int vgamem = def->videos[0]->vgamem;
if (vram > (UINT_MAX / 1024)) { @@ -8279,6 +8286,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "%s.vram_size=%u", dev, vram * 1024); } + if (vram64 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram64_size_mb=%u", + dev, vram64 / 1024); + }
... and also set it here? And mostly why do we check only for primary card capability here? I remember that this was needed for some parameters when QEMU_CAPS_DEVICE was a variable (before we switched to supporting only QEMU_CAPS_DEVICE-enabled versions). Although the code does the opposite thing (uses '-global' only for qemu with '-device' supported). I must be clearly on a bad track here.
This is probably a dead code since we've removed support for qemu that doesn't have -device. Without -device it was possible only have one video device. This should be removed, but not as part of this series.
if (vgamem && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { virCommandAddArg(cmd, "-global"); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2b641f..34efd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1436,6 +1436,14 @@ qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, return -1; } video->vram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, + "vram64_size_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Object '%s' has no property 'vram64_size_mb'"), + path); + return -1; + } + video->vram64 = prop.val.ul / 1024;
And a last question: why don't we just skip the vram64_size_mb if it's not available and move on to the next one? What if it's a bit older QEMU?
I'm not sure about this, but if a device supports some attribute, it should be also present in QOM Object. If this happens, something is really wrong :) Pavel
For all the questions a short explanation might be enough as all the other properties are coded following the same fashion. That doesn't make them correct, though, so the explanation is still needed.
On Mon, Feb 22, 2016 at 07:26:42PM +0100, Pavel Hrdina wrote:
On Mon, Feb 22, 2016 at 04:16:49PM +0100, Martin Kletzander wrote:
On Mon, Feb 22, 2016 at 02:34:14PM +0100, Pavel Hrdina wrote:
This attribute is used to extend secondary PCI bar and expose it to the guest as 64bit memory. It works like this: attribute vram is there to set size of secondary PCI bar and guest sees it as 32bit memory, attribute vram64 can extend this secondary PCI bar. If both attributes are used, guest sees two memory bars, both address the same memory, with the difference that the 32bit bar can address only the first part of the whole memory.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260749
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 2 ++ docs/schemas/domaincommon.rng | 5 ++++ src/conf/domain_conf.c | 34 ++++++++++++++++++---- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 13 +++++++++ src/qemu/qemu_monitor_json.c | 8 +++++ .../qemuxml2argv-video-qxl-device-vram64.args | 25 ++++++++++++++++ .../qemuxml2argv-video-qxl-device-vram64.xml | 29 ++++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.args | 27 +++++++++++++++++ .../qemuxml2argv-video-qxl-sec-device-vram64.xml | 32 ++++++++++++++++++++ 10 files changed, 170 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-device-vram64.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-video-qxl-sec-device-vram64.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3fcd728..318ffd9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5191,6 +5191,8 @@ qemu-kvm -net nic,model=? /dev/null two as <code>vram</code>. There is also optional attribute <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.2</span>) + extends secondary bar and makes it addressable as 64bit memory. </p> </dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 67af93a..fe5eaf0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2938,6 +2938,11 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="vram64"> + <ref name="unsignedInt"/> + </attribute> + </optional> </group> </choice> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 56bd1aa..76fc52a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11898,6 +11898,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *vram64 = NULL; char *ram = NULL; char *vgamem = NULL; char *primary = NULL; @@ -11913,6 +11914,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, type = virXMLPropString(cur, "type"); ram = virXMLPropString(cur, "ram"); vram = virXMLPropString(cur, "vram"); + vram64 = virXMLPropString(cur, "vram64"); vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads");
@@ -11967,6 +11969,19 @@ virDomainVideoDefParseXML(xmlNodePtr node, def->vram = virDomainVideoDefaultRAM(dom, def->type); }
+ if (vram64) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("vram64 attribute only supported for type of qxl")); + goto error; + } + if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video vram64 '%s'"), vram64); + goto error; + } + } + if (vgamem) { if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -11993,9 +12008,11 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0) goto error;
+ cleanup: VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); + VIR_FREE(vram64); VIR_FREE(vgamem); VIR_FREE(heads);
@@ -12003,12 +12020,8 @@ virDomainVideoDefParseXML(xmlNodePtr node,
error: virDomainVideoDefFree(def); - VIR_FREE(type); - VIR_FREE(ram); - VIR_FREE(vram); - VIR_FREE(vgamem); - VIR_FREE(heads); - return NULL; + def = NULL; + goto cleanup; }
static virDomainHostdevDefPtr @@ -17023,6 +17036,13 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, return false; }
+ if (src->vram64 != dst->vram64) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target video card vram64 %u does not match source %u"), + dst->vram64, src->vram64); + return false; + } +
Does this check make sense if we're updating that value after QEMU starts?
Yes, it makes sense, this will ensure, that user doesn't change this value via qemu hook.
if (src->vgamem != dst->vgamem) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card vgamem %u does not match source %u"), @@ -20708,6 +20728,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " ram='%u'", def->ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); + if (def->vram64) + virBufferAsprintf(buf, " vram64='%u'", def->vram64); if (def->vgamem) virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1de3be3..c3a7386 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1399,6 +1399,7 @@ struct _virDomainVideoDef { int type; unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ + unsigned int vram64; /* kibibytes (multiples of 1024) */ unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; bool primary; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 78423e7..24ddd8a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3347,6 +3347,12 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); }
+ if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) || + (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VRAM64))) { + /* QEMU accepts mebibytes for vram64_size_mb. */ + virBufferAsprintf(&buf, ",vram64_size_mb=%u", video->vram64 / 1024); + } +
Why do we both set the size here (for both primary and secondary card), ...
This code is a part of for cycle for all video devices. This means, if current video device is primary, we need to check different flag than for secondary video device.
Note: Both secondary and primary qxl video device are internally represented by the same device model in qemu, so both of them will ether have this feature or not.
Yes, I know that, it was just an introduction to ...
if ((primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) || (!primary && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGAMEM))) { /* QEMU accepts mebibytes for vgamem_mb. */ @@ -8254,6 +8260,7 @@ qemuBuildCommandLine(virConnectPtr conn, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { unsigned int ram = def->videos[0]->ram; unsigned int vram = def->videos[0]->vram; + unsigned int vram64 = def->videos[0]->vram64; unsigned int vgamem = def->videos[0]->vgamem;
if (vram > (UINT_MAX / 1024)) { @@ -8279,6 +8286,12 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgFormat(cmd, "%s.vram_size=%u", dev, vram * 1024); } + if (vram64 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VRAM64)) { + virCommandAddArg(cmd, "-global"); + virCommandAddArgFormat(cmd, "%s.vram64_size_mb=%u", + dev, vram64 / 1024); + }
... and also set it here? And mostly why do we check only for primary card capability here? I remember that this was needed for some parameters when QEMU_CAPS_DEVICE was a variable (before we switched to supporting only QEMU_CAPS_DEVICE-enabled versions). Although the code does the opposite thing (uses '-global' only for qemu with '-device' supported). I must be clearly on a bad track here.
This is probably a dead code since we've removed support for qemu that doesn't have -device. Without -device it was possible only have one video device.
This should be removed, but not as part of this series.
... this question. Good that we're on the same page, I just wanted to make sure I understand it the same way you do.
if (vgamem && virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_VGAMEM)) { virCommandAddArg(cmd, "-global"); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2b641f..34efd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1436,6 +1436,14 @@ qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, return -1; } video->vram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, + "vram64_size_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Object '%s' has no property 'vram64_size_mb'"), + path); + return -1; + } + video->vram64 = prop.val.ul / 1024;
And a last question: why don't we just skip the vram64_size_mb if it's not available and move on to the next one? What if it's a bit older QEMU?
I'm not sure about this, but if a device supports some attribute, it should be also present in QOM Object. If this happens, something is really wrong :)
Yes, but we are not checking that it supports that attribute anywhere. Or are you saying that whenever QEMU supports qxl, it has this property?
Pavel
For all the questions a short explanation might be enough as all the other properties are coded following the same fashion. That doesn't make them correct, though, so the explanation is still needed.
On Tue, Feb 23, 2016 at 08:55:29AM +0100, Martin Kletzander wrote:
On Mon, Feb 22, 2016 at 07:26:42PM +0100, Pavel Hrdina wrote:
On Mon, Feb 22, 2016 at 04:16:49PM +0100, Martin Kletzander wrote:
On Mon, Feb 22, 2016 at 02:34:14PM +0100, Pavel Hrdina wrote:
This attribute is used to extend secondary PCI bar and expose it to the guest as 64bit memory. It works like this: attribute vram is there to set size of secondary PCI bar and guest sees it as 32bit memory, attribute vram64 can extend this secondary PCI bar. If both attributes are used, guest sees two memory bars, both address the same memory, with the difference that the 32bit bar can address only the first part of the whole memory.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260749
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
[...]
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d2b641f..34efd4f 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1436,6 +1436,14 @@ qemuMonitorJSONUpdateVideoMemorySize(qemuMonitorPtr mon, return -1; } video->vram = prop.val.ul / 1024; + if (qemuMonitorJSONGetObjectProperty(mon, path, + "vram64_size_mb", &prop) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("QOM Object '%s' has no property 'vram64_size_mb'"), + path); + return -1; + } + video->vram64 = prop.val.ul / 1024;
And a last question: why don't we just skip the vram64_size_mb if it's not available and move on to the next one? What if it's a bit older QEMU?
I'm not sure about this, but if a device supports some attribute, it should be also present in QOM Object. If this happens, something is really wrong :)
Yes, but we are not checking that it supports that attribute anywhere. Or are you saying that whenever QEMU supports qxl, it has this property?
Oh, I see. I need to rework this as the qemuMonitorJSONUpdateVideoMemorySize was originally meant to update only vgamem and we check for capabilities in qemu_process before we call this function. Good catch. Pavel
participants (2)
-
Martin Kletzander -
Pavel Hrdina