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

On 01/18/2013 11:36 AM, Alon Levy wrote: Fair enough. s/specify/specifies/ (I'm also adding a since 1.0.2 notation). Spurious indentation change here. This shouldn't be internal error (I know, you were just copying and pasting). Missing a check that def->type is qxl (the check is necessary to match the RNG schema). Technically, kilobytes is 1000, kibibytes is 1024; and this is in units of 1024. 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