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

Re: [libvirt] [PATCH v2] qemu: Support ram bar size for qxl devices



On 01/18/2013 11:36 AM, Alon Levy wrote:
> Adds a "ram" attribute globally to the video.model element, that changes
> the resulting qemu command line only if video.type == "qxl".
> 
> <video>
>   <model type='qxl' ram='65536' vram='65536' heads='1'/>
> </video>
> 
> That attribute gets a default value of 64*1024. The schema is unchanged
> for other video element types.
> 
> The resulting qemu command line change is the addition of
> 
> -global qxl-vga.ram_size=<ram>*1024
> 
> or
> 
> -global qxl.ram_size=<ram>*1024
> 
> For the main and secondary qxl devices respectively.
> 
> The default for the qxl ram bar is 64*1024 kilobytes (the same as the
> default qxl vram bar size).
> ---
> changes from v1:
>  renamed attribute to ram (Eric Blake)
>  fixes as given by Eric Blake
> 
> Did not add any of the suggestions that Gerd made, I think it can be done in a
> separate patch and there was no discussion on it yet.

Fair enough.

> +++ b/docs/formatdomain.html.in
> @@ -3565,7 +3565,10 @@ qemu-kvm -net nic,model=? /dev/null
>          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 mutiple
> -        video device. The non-primary must be type of "qxl".
> +        video device. The non-primary must be type of "qxl". The optional
> +        attribute <code>ram</code> is allowed for "qxl" type only and specify

s/specify/specifies/ (I'm also adding a since 1.0.2 notation).

> +        the size of the primary bar, while <code>vram</code> specifies the
> +        secondary bar size.  If "ram" is not supplied a default value is used.
>        </dd>
>  

>      <element name="video">
>        <optional>
> -        <element name="model">
> -          <attribute name="type">
> -            <choice>
> -              <value>vga</value>
> -              <value>cirrus</value>
> -              <value>vmvga</value>
> -              <value>xen</value>
> -              <value>vbox</value>
> -              <value>qxl</value>
> -            </choice>
> -          </attribute>
> +          <element name="model">

Spurious indentation change here.

> @@ -7431,6 +7433,18 @@ virDomainVideoDefParseXML(const xmlNodePtr node,
>          }
>      }
>  
> +    if (ram) {
> +        if (virStrToLong_ui(ram, NULL, 10, &def->ram) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,

This shouldn't be internal error (I know, you were just copying and
pasting).

> +                           _("cannot parse video ram '%s'"), ram);
> +            goto error;
> +        }

Missing a check that def->type is qxl (the check is necessary to match
the RNG schema).

> +++ b/src/conf/domain_conf.h
> @@ -1157,7 +1157,8 @@ struct _virDomainVideoAccelDef {
>  
>  struct _virDomainVideoDef {
>      int type;
> -    unsigned int vram;
> +    unsigned int ram;  /* kilobytes */
> +    unsigned int vram; /* kilobytes */

Technically, kilobytes is 1000, kibibytes is 1024; and this is in units
of 1024.

> +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args
> @@ -5,5 +5,5 @@ unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
>  x509-dir=/etc/pki/libvirt-spice,\
>  image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\
>  playback-compression=on,streaming-video=filter -vga \
> -qxl -global qxl.vram_size=18874368 -device qxl,id=video1,vram_size=33554432,bus=pci.0,addr=0x4 \
> +qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 -device qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \

Still a long line.

ACK with the following squashed in, so I pushed:

diff --git i/docs/formatdomain.html.in w/docs/formatdomain.html.in
index 370de99..600065d 100644
--- i/docs/formatdomain.html.in
+++ w/docs/formatdomain.html.in
@@ -3566,7 +3566,8 @@ qemu-kvm -net nic,model=? /dev/null
         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
mutiple
         video device. The non-primary must be type of "qxl". The optional
-        attribute <code>ram</code> is allowed for "qxl" type only and
specify
+        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" is not supplied a default value
is used.
       </dd>
diff --git i/docs/schemas/domaincommon.rng w/docs/schemas/domaincommon.rng
index 6aca97a..7f3320e 100644
--- i/docs/schemas/domaincommon.rng
+++ w/docs/schemas/domaincommon.rng
@@ -2257,7 +2257,7 @@
   <define name="video">
     <element name="video">
       <optional>
-          <element name="model">
+        <element name="model">
           <choice>
             <attribute name="type">
               <choice>
diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 1925351..cc68bc5 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -7434,20 +7434,23 @@ virDomainVideoDefParseXML(const xmlNodePtr node,
     }

     if (ram) {
+        if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("ram attribute only supported for type of
qxl"));
+            goto error;
+        }
         if (virStrToLong_ui(ram, NULL, 10, &def->ram) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
+            virReportError(VIR_ERR_XML_ERROR,
                            _("cannot parse video ram '%s'"), ram);
             goto error;
         }
-    } else {
-        if (def->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
-            def->ram = virDomainVideoDefaultRAM(dom, def->type);
-        }
+    } else if (def->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
+        def->ram = virDomainVideoDefaultRAM(dom, def->type);
     }

     if (vram) {
         if (virStrToLong_ui(vram, NULL, 10, &def->vram) < 0) {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
+            virReportError(VIR_ERR_XML_ERROR,
                            _("cannot parse video ram '%s'"), vram);
             goto error;
         }
diff --git i/src/conf/domain_conf.h w/src/conf/domain_conf.h
index d05617e..abbfe86 100644
--- i/src/conf/domain_conf.h
+++ w/src/conf/domain_conf.h
@@ -1,7 +1,7 @@
 /*
  * domain_conf.h: domain XML processing
  *
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2013 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -1157,8 +1157,8 @@ struct _virDomainVideoAccelDef {

 struct _virDomainVideoDef {
     int type;
-    unsigned int ram;  /* kilobytes */
-    unsigned int vram; /* kilobytes */
+    unsigned int ram;  /* kibibytes (multiples of 1024) */
+    unsigned int vram; /* kibibytes (multiples of 1024) */
     unsigned int heads;
     bool primary;
     virDomainVideoAccelDefPtr accel;
diff --git
i/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args
w/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args
index 2f537df..59f064b 100644
--- i/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args
+++ w/tests/qemuxml2argvdata/qemuxml2argv-graphics-spice-compression.args
@@ -3,7 +3,9 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test
LOGNAME=test QEMU_AUDIO_DRV=spice \
 unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -hda \
 /dev/HostVG/QEMUGuest1 -spice port=5903,tls-port=5904,addr=127.0.0.1,\
 x509-dir=/etc/pki/libvirt-spice,\
-image-compression=auto_glz,jpeg-wan-compression=auto,zlib-glz-wan-compression=auto,\
+image-compression=auto_glz,jpeg-wan-compression=auto,\
+zlib-glz-wan-compression=auto,\
 playback-compression=on,streaming-video=filter -vga \
-qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368
-device
qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \
+qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368 \
+-device
qxl,id=video1,ram_size=67108864,vram_size=33554432,bus=pci.0,addr=0x4 \
 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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