[libvirt] [PATCH V3 2/3] qemu: Introduce vgamem attribute for video model
Michal Privoznik
mprivozn at redhat.com
Fri Sep 19 12:02:59 UTC 2014
On 14.08.2014 14:43, Wang Rui wrote:
> From: Zeng Junliang <zengjunliang at huawei.com>
>
> 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 at huawei.com>
> Signed-off-by: Wang Rui <moon.wangrui at huawei.com>
> ---
> 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
> ...
> <devices>
> <video>
> - <model type='vga' heads='1'>
> + <model type='vga' vgamem='16384' heads='1'>
> <acceleration accel3d='yes' accel2d='yes'/>
> </model>
> </video>
> @@ -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);
At least for cirrus vga this is not true. The vgamem_mb property of
cirrus vga was introduced in 19403a68 (v1.3.0). However, I'd expect
these to be set by something else than version based test. For instance,
aren't the properties listen in device prop query command?
> + }
> }
> 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);
No.
> }
>
> /* 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");
Why? If I deliberately set vgamem the domain startup process must fail
if qemu doesn't support it.
Michal
More information about the libvir-list
mailing list