Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML

On 02/04/2013 04:16 PM, Christophe Fergeau wrote: > --- > docs/formatdomain.html.in | 5 ++++- > docs/schemas/domaincommon.rng | 5 +++++ > src/conf/domain_conf.c | 17 +++++++++++++++++ > src/conf/domain_conf.h | 1 + > src/qemu/qemu_command.c | 8 ++++++++ > .../qemuxml2argv-graphics-spice-compression.args | 3 ++- > .../qemuxml2argv-graphics-spice-compression.xml | 4 ++-- > .../qemuxml2argv-graphics-spice-qxl-vga.args | 3 ++- > .../qemuxml2argv-graphics-spice-qxl-vga.xml | 4 ++-- > tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.args | 3 ++- > tests/qemuxml2argvdata/qemuxml2argv-graphics-spice.xml | 4 ++-- > 11 files changed, 47 insertions(+), 10 deletions(-) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 7ad8aea..4b269c8 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -3569,7 +3569,10 @@ qemu-kvm -net nic,model=? /dev/null > 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. > + secondary bar size. If "ram" or "vram" are not supplied a default Good fix, but it should in a be separate patch.

> + value is used. The optional attribute <code>revision</code> (<span > + class="since">since 1.0.3</span>) specifies the revision of > + the QXL device, newer revisions provides more functionality. s/provides/provide/ if I'm not mistaken

> </dd> > > <dt><code>model</code></dt> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 049f232..fc78e2d 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -2280,6 +2280,11 @@ > <ref name="unsignedInt"/> > </attribute> > </optional> > + <optional> > + <attribute name="revision"> > + <ref name="unsignedInt"/> This should be > 0 according to my experiments with qemu, but I believe we don't deal with such nuances in the RNG scheme.

> + </attribute> > + </optional> > </group> > </choice> > <optional> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 5782abb..83be711 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -7391,6 +7391,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, > char *vram = NULL; > char *ram = NULL; > char *primary = NULL; > + char *revision = NULL; > > if (VIR_ALLOC(def) < 0) { > virReportOOMError(); > @@ -7406,6 +7407,7 @@ virDomainVideoDefParseXML(const xmlNodePtr node, > ram = virXMLPropString(cur, "ram"); > vram = virXMLPropString(cur, "vram"); > heads = virXMLPropString(cur, "heads"); > + revision = virXMLPropString(cur, "revision"); You're leaking the revision string in here.

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 9a9e0b1..81925b1 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -1160,6 +1160,7 @@ struct _virDomainVideoDef { > unsigned int ram; /* kibibytes (multiples of 1024) */ > unsigned int vram; /* kibibytes (multiples of 1024) */ > unsigned int heads; > + unsigned int revision; > bool primary; > virDomainVideoAccelDefPtr accel; > virDomainDeviceInfo info; > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index f6273c1..e45c808 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -3598,6 +3598,9 @@ qemuBuildDeviceVideoStr(virDomainVideoDefPtr video, > > /* QEMU accepts bytes for vram_size. */ > virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024); > + > + if (video->revision != 0) you can drop the '!= 0' in here, but that's unimportant.