Re: [libvirt] [PATCH V3 2/3] qemu: Introduce vgamem attribute for video model

From: Zeng Junliang <zengjunliang(a)huawei.com&gt; QEMU suopports to specifie the size of the framebuffer portion of the "ram" region for vga, vmvga and qxl through the qemu command line parameter "vgamem_mb". This patch introduces vgamem attribute for video model, makes the "vgamem_mb" value configured in libvirt xml. Also, add test cases and descriptions for it. Libvirt xml configuration sample(based on VGA): <video> <model type='vga' vgamem='16384' heads='1'/> </video> The resulting qemu command line change is the addition of: -vga std -global VGA.vgamem_mb=16 or -device VGA,id=video0,vgamem_mb=16,bus=pci.0,addr=0x2 If 'vgamem' is not configured by user, a default value will be assigned by libvirt. Assume that user wants to start a VM without 'vgamem' configuration in xml, if QEMU lacks the ability of vgamem, libvirt will only report a warning rather than make starting failed even though libvirt has assigned a default value to vgamem. It could avoid a regression. It's less confusing to introduce the new vgamem attribute. Prior to the change: model libvirt-attribute(xml) qemu-attribute qxl vram vram_size qxl none vgamem_mb vga vram QEMU has no attribute named vram* vga none vgamem_mb vmvga vram QEMU has no attribute named vram* vmvga none vgamem_mb After the change: model libvirt attribute(xml) QEMU attribute qxl vram vram_size qxl vgamem vgamem_mb vga vram QEMU has no attribute named vram* vga vgamem vgamem_mb vmvga vram QEMU has no attribute named vram* vmvga vgamem vgamem_mb Signed-off-by: Zeng Junliang <zengjunliang(a)huawei.com&gt; Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com&gt; --- docs/formatdomain.html.in | 35 ++++--- docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 52 +++++++++- src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 15 +++ src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_command.c | 105 ++++++++++++++++----- tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 3 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 3 + tests/qemuhelptest.c | 10 +- .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +- ...emuxml2argv-graphics-spice-agent-file-xfer.args | 5 +- ...qemuxml2argv-graphics-spice-agent-file-xfer.xml | 4 +- .../qemuxml2argv-graphics-spice-compression.args | 4 +- .../qemuxml2argv-graphics-spice-compression.xml | 4 +- .../qemuxml2argv-graphics-spice-listen-network.xml | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.args | 4 +- .../qemuxml2argv-graphics-spice-qxl-vga.xml | 4 +- .../qemuxml2argv-graphics-spice-sasl.args | 3 +- .../qemuxml2argv-graphics-spice-sasl.xml | 2 +- .../qemuxml2argv-graphics-spice-timeout.xml | 2 +- .../qemuxml2argv-graphics-spice.args | 5 +- .../qemuxml2argv-graphics-spice.xml | 4 +- .../qemuxml2argv-graphics-vnc-std-vga.args | 4 + .../qemuxml2argv-graphics-vnc-std-vga.xml | 36 +++++++ .../qemuxml2argv-graphics-vnc-vmware-svga.args | 4 + .../qemuxml2argv-graphics-vnc-vmware-svga.xml | 36 +++++++ .../qemuxml2argv-net-bandwidth.xml | 2 +- .../qemuxml2argv-pcihole64-q35.args | 3 +- .../qemuxml2argv-pcihole64-q35.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 2 +- .../qemuxml2argv-serial-spiceport.args | 4 +- .../qemuxml2argv-serial-spiceport.xml | 2 +- .../qemuxml2argv-video-device-pciaddr-default.args | 9 +- .../qemuxml2argv-video-device-pciaddr-default.xml | 6 +- tests/qemuxml2argvtest.c | 22 ++++- .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +- tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 2 +- tests/qemuxml2xmltest.c | 2 + 45 files changed, 357 insertions(+), 77 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-std-vga.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-std-vga.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-vmware-svga.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-vmware-svga.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3012e3c..a0d15c4 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4419,7 +4419,7 @@ qemu-kvm -net nic,model=? /dev/null ... &lt;devices&gt; &lt;video&gt; - &lt;model type='vga' heads='1'&gt; + &lt;model type='vga' vgamem='16384' heads='1'&gt; &lt;acceleration accel3d='yes' accel2d='yes'/&gt; &lt;/model&gt; &lt;/video&gt; @@ -4434,16 +4434,17 @@ qemu-kvm -net nic,model=? /dev/null is set but there is a <code>graphics</code> in domain xml, then libvirt will add a default <code>video</code> according to the guest type. For a guest of type "kvm", the default <code>video</code> for it is: - <code>type</code> with value "cirrus", and <code>heads</code> with - value "1". By default, the first video device in domain xml is the - primary one, but the optional attribute <code>primary</code> - (<span class="since">since 1.0.2</span>) with value 'yes' can be - used to mark the primary in cases of multiple video device. - The non-primary must be type of "qxl". The optional attribute + <code>type</code> with value "cirrus", <code>vgamem</code> + (<span class="since">since 1.2.8</span>) with value "16384" + and <code>heads</code> with value "1". By default, the first video + device in domain xml is the primary one, but the optional attribute + <code>primary</code> (<span class="since">since 1.0.2</span>) with + value 'yes' can be used to mark the primary in cases of multiple video + device. The non-primary must be type of "qxl". 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 <code>vram</code> specifies the secondary bar size. - If "ram" or "vram" are not supplied a default value is used. + If "ram", "vram" or "vgamem" are not supplied a default value is used. </dd> <dt><code>model</code></dt> @@ -4452,10 +4453,20 @@ qemu-kvm -net nic,model=? /dev/null attribute which takes the value "vga", "cirrus", "vmvga", "xen", "vbox", or "qxl" (<span class="since">since 0.8.6</span>) depending on the hypervisor features available. - You can also provide the amount of video memory in kibibytes - (blocks of 1024 bytes) using - <code>vram</code> and the number of screen with <code>heads</code>. - For type of kvm <code>vram</code> attribute is only valid for "qxl". + <p> + <code>vram</code> attribute specifies the amount of video memory + in kibibytes (blocks of 1024 bytes). For type of kvm, it is only + valid for type of "qxl". + </p> + <p> + <code>vgamem</code> attribute <span class="since">since 1.2.8, + QEMU and KVM only</span> specifies the size of the framebuffer + portion of the "ram" region. And it is only valid for type of + "vga", "vmvga" and "qxl". + </p> + <p> + <code>heads</code> attribute specifies the number of screen. + </p> </dd> <dt><code>acceleration</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 033f2f6..b2cc218 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2786,6 +2786,11 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <attribute name="vgamem"> + <ref name="unsignedInt"/> + </attribute> + </optional> <optional> <attribute name="heads"> <ref name="unsignedInt"/> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5c762fa..7097570 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9639,6 +9639,21 @@ virDomainVideoDefaultType(const virDomainDef *def) } } +int +virDomainVideoDefaultVgamem(int type) +{ + switch (type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + case VIR_DOMAIN_VIDEO_TYPE_QXL: + /* QEMU use 16M as default value for vga/vmvga/qxl device*/ + return 16 * 1024; + + default: + return 0; + } +} + static virDomainVideoAccelDefPtr virDomainVideoAccelDefParseXML(xmlNodePtr node) { @@ -9694,6 +9709,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, char *type = NULL; char *heads = NULL; char *vram = NULL; + char *vgamem = NULL; char *ram = NULL; char *primary = NULL; @@ -9703,11 +9719,12 @@ virDomainVideoDefParseXML(xmlNodePtr node, cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE) { - if (!type && !vram && !ram && !heads && + if (!type && !vram && !ram && !heads && !vgamem && xmlStrEqual(cur->name, BAD_CAST "model")) { type = virXMLPropString(cur, "type"); ram = virXMLPropString(cur, "ram"); vram = virXMLPropString(cur, "vram"); + vgamem = virXMLPropString(cur, "vgamem"); heads = virXMLPropString(cur, "heads"); if ((primary = virXMLPropString(cur, "primary")) != NULL) { @@ -9754,13 +9771,31 @@ virDomainVideoDefParseXML(xmlNodePtr node, if (vram) { if (virStrToLong_ui(vram, NULL, 10, &def->vram) < 0) { virReportError(VIR_ERR_XML_ERROR, - _("cannot parse video ram '%s'"), vram); + _("cannot parse video vram '%s'"), vram); goto error; } } else { def->vram = virDomainVideoDefaultRAM(dom, def->type); } + if (vgamem) { + if (def->type != VIR_DOMAIN_VIDEO_TYPE_VGA && + def->type != VIR_DOMAIN_VIDEO_TYPE_VMVGA && + def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { + virReportError(VIR_ERR_XML_ERROR, + _("Unsupported vgamem attribute for %s device"), + virDomainVideoTypeToString(def->type)); + goto error; + } + if (virStrToLong_ui(vgamem, NULL, 10, &def->vgamem) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("cannot parse video vgamem '%s'"), vgamem); + goto error; + } + } else { + def->vgamem = virDomainVideoDefaultVgamem(def->type); + } + if (heads) { if (virStrToLong_ui(heads, NULL, 10, &def->heads) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -9777,6 +9812,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); + VIR_FREE(vgamem); VIR_FREE(heads); return def; @@ -9786,6 +9822,7 @@ virDomainVideoDefParseXML(xmlNodePtr node, VIR_FREE(type); VIR_FREE(ram); VIR_FREE(vram); + VIR_FREE(vgamem); VIR_FREE(heads); return NULL; } @@ -12996,6 +13033,7 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(video); goto error; } + video->vgamem = virDomainVideoDefaultVgamem(video->type); video->vram = virDomainVideoDefaultRAM(def, video->type); video->heads = 1; if (VIR_ALLOC_N(def->videos, 1) < 0) { @@ -13891,6 +13929,14 @@ virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src, return false; } + if (src->vgamem != dst->vgamem) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target video card vgamem %u does not match source %u"), + dst->vgamem, src->vgamem); + return false; + } + + if (src->heads != dst->heads) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target video card heads %u does not match source %u"), @@ -16937,6 +16983,8 @@ virDomainVideoDefFormat(virBufferPtr buf, virBufferAsprintf(buf, " ram='%u'", def->ram); if (def->vram) virBufferAsprintf(buf, " vram='%u'", def->vram); + if (def->vgamem) + virBufferAsprintf(buf, " vgamem='%u'", def->vgamem); if (def->heads) virBufferAsprintf(buf, " heads='%u'", def->heads); if (def->primary) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ff7d640..d1ef6ec 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1244,6 +1244,7 @@ struct _virDomainVideoDef { int type; unsigned int ram; /* kibibytes (multiples of 1024) */ unsigned int vram; /* kibibytes (multiples of 1024) */ + unsigned int vgamem; /* kibibytes (multiples of 1024) */ unsigned int heads; bool primary; virDomainVideoAccelDefPtr accel; @@ -2492,6 +2493,7 @@ virDomainFSDefPtr virDomainFSRemove(virDomainDefPtr def, size_t i); int virDomainVideoDefaultType(const virDomainDef *def); int virDomainVideoDefaultRAM(const virDomainDef *def, int type); +int virDomainVideoDefaultVgamem(int type); int virDomainObjListNumOfDomains(virDomainObjListPtr doms, bool active, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 08111d4..e481d92 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -416,6 +416,7 @@ virDomainVcpuPinFindByVcpu; virDomainVcpuPinIsDuplicate; virDomainVideoDefaultRAM; virDomainVideoDefaultType; +virDomainVideoDefaultVgamem; virDomainVideoDefFree; virDomainVideoTypeFromString; virDomainVideoTypeToString; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 360cc67..146d67c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -265,6 +265,10 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "numa", "memory-backend-file", "usb-audio", + "vga-vgamem", + + "qxl-vgamem", /* 175 */ + "vmware-vgamem", ); @@ -1073,6 +1077,14 @@ virQEMUCapsComputeCmdFlags(const char *help, virQEMUCapsSet(qemuCaps, QEMU_CAPS_VGA_QXL); if ((p = strstr(p, "|none")) && p < nl) virQEMUCapsSet(qemuCaps, QEMU_CAPS_VGA_NONE); + /* It seems that QEMU supports to be communicated with + * qmp command since 1.2.0. When qemuCaps->usedQMP is + * true, these logical code will be invalid. Does it need here? */ + if (version >= 1002000) { + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VGA_VGAMEM_MB); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_QXL_VGAMEM_MB); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VMWARE_VGAMEM_MB);

+ } } if (strstr(help, "-spice")) virQEMUCapsSet(qemuCaps, QEMU_CAPS_SPICE); @@ -3034,6 +3046,9 @@ virQEMUCapsInitQMPBasic(virQEMUCapsPtr qemuCaps) virQEMUCapsSet(qemuCaps, QEMU_CAPS_DUMP_GUEST_CORE); virQEMUCapsSet(qemuCaps, QEMU_CAPS_VNC_SHARE_POLICY); virQEMUCapsSet(qemuCaps, QEMU_CAPS_HOST_PCI_MULTIDOMAIN); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VGA_VGAMEM_MB); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_QXL_VGAMEM_MB); + virQEMUCapsSet(qemuCaps, QEMU_CAPS_VMWARE_VGAMEM_MB);

} /* Capabilities that are architecture depending diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 2911759..cdf6920 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -213,6 +213,9 @@ typedef enum { QEMU_CAPS_NUMA = 171, /* newer -numa handling with disjoint cpu ranges */ QEMU_CAPS_OBJECT_MEMORY_FILE = 172, /* -object memory-backend-file */ QEMU_CAPS_OBJECT_USB_AUDIO = 173, /* usb-audio device support */ + QEMU_CAPS_VGA_VGAMEM_MB = 174, /* -global VGA.vgamem_mb */ + QEMU_CAPS_QXL_VGAMEM_MB = 175, /* -global qxl-vga.vgamem_mb */ + QEMU_CAPS_VMWARE_VGAMEM_MB = 176, /* -global vmware-svga.vgamem_mb */ QEMU_CAPS_LAST, /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c3f860e..c15099a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4827,6 +4827,14 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def, virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); } + /* 1. Ignore cirrus-vga as guests would not use it anyway. + * 2. QEMU accepts MByte for vgamem_mb and ensure its value + * a power of two and range: 1 MB -> 256 MB */ + if (video->type != VIR_DOMAIN_VIDEO_TYPE_CIRRUS && + video->vgamem > 1024) { + virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024); + } + if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0) goto error; @@ -8766,36 +8774,86 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArgList(cmd, "-vga", vgastr, NULL); - if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL && - (def->videos[0]->vram || def->videos[0]->ram) && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { - const char *dev = (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL_VGA) - ? "qxl-vga" : "qxl"); + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + const char *dev = NULL; + bool vgamemSupport = true; int ram = def->videos[0]->ram; int vram = def->videos[0]->vram; + int vgamem = def->videos[0]->vgamem; + switch (primaryVideoType) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) + dev = "VGA"; + if (dev && vgamem && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM_MB)) { + vgamemSupport = false; + VIR_WARN("This QEMU does not support vgamem " + "attribute, ignore it");