[libvirt] [PATCH V3 1/3] qemu: Hide vram attribute for some useless cases.
Michal Privoznik
mprivozn at redhat.com
Fri Sep 19 12:03:03 UTC 2014
On 14.08.2014 14:43, Wang Rui wrote:
> From: Zeng Junliang <zengjunliang at huawei.com>
>
> The vram attribute is never used for VGA, CIRRUS and VMVGA as
> QEMU has no vram attribute for these models. Hide it for qemu
> in qemuDomainDevicePostParse function. And update the
> corresponding test cases and descriptions.
>
> Signed-off-by: Zeng Junliang <zengjunliang at huawei.com>
> Signed-off-by: Wang Rui <moon.wangrui at huawei.com>
> ---
> docs/formatdomain.html.in | 24 +++++++++++-----------
> src/qemu/qemu_command.c | 3 ++-
> src/qemu/qemu_domain.c | 12 +++++++++++
> ...qemuhotplug-console-compat-2+console-virtio.xml | 2 +-
> .../qemuxml2argv-console-compat-2.xml | 2 +-
> .../qemuxml2argv-controller-order.xml | 2 +-
> .../qemuxml2argv-graphics-listen-network.xml | 2 +-
> .../qemuxml2argv-graphics-listen-network2.xml | 2 +-
> .../qemuxml2argv-graphics-sdl-fullscreen.xml | 2 +-
> .../qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml | 2 +-
> .../qemuxml2argv-graphics-spice-agentmouse.xml | 2 +-
> .../qemuxml2argv-graphics-spice-timeout.xml | 2 +-
> .../qemuxml2argv-graphics-vnc-policy.xml | 2 +-
> .../qemuxml2argv-graphics-vnc-sasl.xml | 2 +-
> .../qemuxml2argv-graphics-vnc-socket.xml | 2 +-
> .../qemuxml2argv-graphics-vnc-tls.xml | 2 +-
> .../qemuxml2argv-graphics-vnc-websocket.xml | 2 +-
> .../qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml | 2 +-
> .../qemuxml2argv-net-bandwidth.xml | 2 +-
> .../qemuxml2argv-pci-autoadd-addr.xml | 2 +-
> .../qemuxml2argv-pci-autoadd-idx.xml | 2 +-
> tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 2 +-
> .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml | 2 +-
> .../qemuxml2xmlout-graphics-listen-network2.xml | 2 +-
> .../qemuxml2xmlout-graphics-spice-timeout.xml | 2 +-
> .../qemuxml2xmlout-pci-autoadd-addr.xml | 2 +-
> .../qemuxml2xmlout-pci-autoadd-idx.xml | 2 +-
> 27 files changed, 50 insertions(+), 37 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index bd99ae0..3012e3c 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' vram='8192' heads='1'>
> + <model type='vga' heads='1'>
> <acceleration accel3d='yes' accel2d='yes'/>
> </model>
> </video>
> @@ -4434,17 +4434,16 @@ 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", <code>vram</code> with value
> - "9216", 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.
> + <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>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.
> </dd>
NACK to these two chunks! The fact that qemu doesn't support setting
vram (or maybe it does meanwhile, but we're missing implementation)
doesn't mean it should be removed from the XML - moreover, some other
drivers support this setting (e.g. vmx and virtualbox). We must keep it
there otherwise vmx users won't like us anymore.
>
> <dt><code>model</code></dt>
> @@ -4456,6 +4455,7 @@ qemu-kvm -net nic,model=? /dev/null
> 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".
True. This is a good note to have in docs. Okay.
> </dd>
>
> <dt><code>acceleration</code></dt>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8a69976..c3f860e 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -11766,7 +11766,8 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
> vid->type = VIR_DOMAIN_VIDEO_TYPE_XEN;
> else
> vid->type = video;
> - vid->vram = virDomainVideoDefaultRAM(def, vid->type);
> + vid->vram = vid->type == VIR_DOMAIN_VIDEO_TYPE_QXL ?
> + virDomainVideoDefaultRAM(def, vid->type) : 0;
> vid->ram = vid->type == VIR_DOMAIN_VIDEO_TYPE_QXL ?
> virDomainVideoDefaultRAM(def, vid->type) : 0;
> vid->heads = 1;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e9506e0..9b02f39 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -950,6 +950,18 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
> goto cleanup;
> }
>
> + /* hide the confusing vram attribute which is never used
> + * for VGA, CIRRUS and VMVGA devices */
> + if (dev->type == VIR_DOMAIN_DEVICE_VIDEO &&
> + (dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_VGA ||
> + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS ||
> + dev->data.video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA)) {
> + VIR_DEBUG("Hide the confusing vram attribute here, "
> + "which is never used for VGA, CIRRUS and "
> + "VMVGA devices");
> + dev->data.video->vram = 0;
> + }
> +
Well, the field can be looked as reporting field back to user. But I can
live with this too.
Michal
More information about the libvir-list
mailing list