[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH V3 1/3] qemu: Hide vram attribute for some useless cases.



On 14.08.2014 14:43, Wang Rui wrote:
From: Zeng Junliang <zengjunliang 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 huawei com>
Signed-off-by: Wang Rui <moon wangrui 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
    ...
    &lt;devices&gt;
      &lt;video&gt;
-      &lt;model type='vga' vram='8192' heads='1'&gt;
+      &lt;model type='vga' heads='1'&gt;
          &lt;acceleration accel3d='yes' accel2d='yes'/&gt;
        &lt;/model&gt;
      &lt;/video&gt;
@@ -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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]