Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML
On 02/21/2013 04:32 PM, Christophe Fergeau wrote: > Hey Martin, > > Sorry, took me a while to get back to this patch, > No problem, I had the same issue :) > On Thu, Feb 14, 2013 at 05:54:02PM +0100, Martin Kletzander wrote: >> 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 ram (since >>> 1.0.2) is allowed for "qxl" type only and specifies >>> the size of the primary bar, while vram 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. > > Yes, split, I'll push this doc change under the trivial rule (unless > there's a freeze going on). > >> >>> +value is used. The optional attribute revision (>> +class="since">since 1.0.3) specifies the revision of >>> +the QXL device, newer revisions provides more functionality. >> >> s/provides/provide/ if I'm not mistaken > > Yes, I agree, changed. > >> >>> >>> >>>model >>> 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 @@ >>> >>> >>> >>> + >>> + >>> + >> >> This should be > 0 according to my experiments with qemu, but I believe >> we don't deal with such nuances in the RNG scheme. > > I indeed don't know how to do that, and I couldn't find attributes doing it > so it's going to stay as is for now ;) > >> >>> + >>> + >>> >>> >>> >>> 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. > > Oops, thanks! Bad news is that there are already other leaks there in error > paths, I'll send patches. > >>> 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. > > I prefer the more explicit version (with != 0), I've kept it this way as > there are other similar if () in the same file. > I haven't noticed the other if()s and we have no
Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML
Hey Martin, Sorry, took me a while to get back to this patch, On Thu, Feb 14, 2013 at 05:54:02PM +0100, Martin Kletzander wrote: > 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 ram (since > > 1.0.2) is allowed for "qxl" type only and specifies > > the size of the primary bar, while vram 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. Yes, split, I'll push this doc change under the trivial rule (unless there's a freeze going on). > > > +value is used. The optional attribute revision ( > +class="since">since 1.0.3) specifies the revision of > > +the QXL device, newer revisions provides more functionality. > > s/provides/provide/ if I'm not mistaken Yes, I agree, changed. > > > > > > >model > > 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 @@ > > > > > > > > + > > + > > + > > This should be > 0 according to my experiments with qemu, but I believe > we don't deal with such nuances in the RNG scheme. I indeed don't know how to do that, and I couldn't find attributes doing it so it's going to stay as is for now ;) > > > + > > + > > > > > > > > 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. Oops, thanks! Bad news is that there are already other leaks there in error paths, I'll send patches. > > 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. I prefer the more explicit version (with != 0), I've kept it this way as there are other similar if () in the same file. I'm a bit lost by your conversation with Alon, do you expect more work to be done to get the supported revisions out of QEMU, or will a v2 fixing what you pointed out in this email be enough
Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML
On 02/17/2013 11:50 AM, Alon Levy wrote: > Hi Martin, > >> On 02/04/2013 04:16 PM, Christophe Fergeau wrote: >>> QXL devices have an associated 'revision' which is raised when >>> new features have been introduced which would break migration >>> to older versions. This commit makes it possible to set this >>> revision as QEMU sometimes support newer QXL revisions than what >>> it defaults to. >> >> This could help a lot, but few questions, if I may: >> - This only helps when the revision is specified, otherwise not, >> right? > > If by "helps" you mean "changes the revision", then it's a bit more complex - > the revision is determined by order of precedence by the following: > default > machine type (i.e. via -M) > global (which this patch sets) > Even though this is not the root cause neither the place we want to fix it; so the theoretical migration problem occurs because the machine type doesn't specify the revision, right? Just asking to make sure I understand. >> - Isn't there a way how to get the current supported (running) >> revision >> of QEMU's qxl video? >> >> If we don't know the current (and supported) revision, we are >> exposing a >> parameter we know completely nothing about and thus can only try to >> start qemu with it and wait if it initializes (I know that's how we >> do >> it with devices, but there's no other option). Transferring current >> revision during migration (even when unspecified) would help >> determining >> such errors before starting QEMU on destination. >> >> There's not much info about the revisions if you don't look in QEMU >> sources, so I'm not sure what our possibilities are. It's a good >> thing >> that the 'revision' parameter is supported since the same commit as >> qxl >> driver, though. > > There is no "introspection" from the help, sorry. So like you said the only > way to know what supported revisions are is to: > 1) start a vm with -vga qxl as stopped, query the revision of the qxl device, > you get R, then [1..R] are supported (i.e. inclusive) This could be pretty doable. We already query some information from qemu, although not device-relevant. I have to look in the code, but good to know it is gettable from the monitor. Maybe we can run the initial '-M none' with '-vga qxl' or '-device qxl-[vga]' to get the revision. > 2) to find out which higher revisions are supported, you'll have to start > with each successive higher revision until you get an error code (you can > wait for the machine start event, that is sent after all devices are > initialized) > > Of course we can fix qemu to report it now, but it won't help for any of the > older versions.. > Thanks for the info, Martin -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML
Hi Martin, > On 02/04/2013 04:16 PM, Christophe Fergeau wrote: > > QXL devices have an associated 'revision' which is raised when > > new features have been introduced which would break migration > > to older versions. This commit makes it possible to set this > > revision as QEMU sometimes support newer QXL revisions than what > > it defaults to. > > This could help a lot, but few questions, if I may: > - This only helps when the revision is specified, otherwise not, > right? If by "helps" you mean "changes the revision", then it's a bit more complex - the revision is determined by order of precedence by the following: default machine type (i.e. via -M) global (which this patch sets) > - Isn't there a way how to get the current supported (running) > revision > of QEMU's qxl video? > > If we don't know the current (and supported) revision, we are > exposing a > parameter we know completely nothing about and thus can only try to > start qemu with it and wait if it initializes (I know that's how we > do > it with devices, but there's no other option). Transferring current > revision during migration (even when unspecified) would help > determining > such errors before starting QEMU on destination. > > There's not much info about the revisions if you don't look in QEMU > sources, so I'm not sure what our possibilities are. It's a good > thing > that the 'revision' parameter is supported since the same commit as > qxl > driver, though. There is no "introspection" from the help, sorry. So like you said the only way to know what supported revisions are is to: 1) start a vm with -vga qxl as stopped, query the revision of the qxl device, you get R, then [1..R] are supported (i.e. inclusive) 2) to find out which higher revisions are supported, you'll have to start with each successive higher revision until you get an error code (you can wait for the machine start event, that is sent after all devices are initialized) Of course we can fix qemu to report it now, but it won't help for any of the older versions.. > > Few pointers inline... > > > --- > > 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 ram (since > > 1.0.2) is allowed for "qxl" type only and specifies > > the size of the primary bar, while vram > > 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 > > revision ( > +class="since">since 1.0.3) specifies the revision > > of > > +the QXL device, newer revisions provides more > > functionality. > > s/provides/provide/ if I'm not mistaken > > > > > > >model > > 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 @@ > > > > > > > > + > > + > > + > > This should be > 0 according to my experiments with qemu, but I > believe > we don't deal with such nuances in the RNG scheme. > > > + > > + > > > > > > > > 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 > > n
Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML
On 02/04/2013 04:16 PM, Christophe Fergeau wrote: > QXL devices have an associated 'revision' which is raised when > new features have been introduced which would break migration > to older versions. This commit makes it possible to set this > revision as QEMU sometimes support newer QXL revisions than what > it defaults to. This could help a lot, but few questions, if I may: - This only helps when the revision is specified, otherwise not, right? - Isn't there a way how to get the current supported (running) revision of QEMU's qxl video? If we don't know the current (and supported) revision, we are exposing a parameter we know completely nothing about and thus can only try to start qemu with it and wait if it initializes (I know that's how we do it with devices, but there's no other option). Transferring current revision during migration (even when unspecified) would help determining such errors before starting QEMU on destination. There's not much info about the revisions if you don't look in QEMU sources, so I'm not sure what our possibilities are. It's a good thing that the 'revision' parameter is supported since the same commit as qxl driver, though. Few pointers inline... > --- > 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 ram (since > 1.0.2) is allowed for "qxl" type only and specifies > the size of the primary bar, while vram 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 revision ( +class="since">since 1.0.3) specifies the revision of > +the QXL device, newer revisions provides more functionality. s/provides/provide/ if I'm not mistaken > > >model > 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 @@ > > > > + > + > + This should be > 0 according to my experiments with qemu, but I believe we don't deal with such nuances in the RNG scheme. > + > + > > > > 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. > > if ((primary = virXMLPropString(cur, "primary")) != NULL) > if (STREQ(primary, "yes")) > @@ -7456,6 +7458,19 @@ virDomainVideoDefParseXML(const xmlNodePtr node, > def->vram = virDomainVideoDefaultRAM(dom, def->type); > } > > +if (revision) { > +if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { > +virReportError(VIR_ERR_XML_ERROR, "%s", > + _("revision attribute only supported for type of > qxl")); > +goto error; > +} > +if (virStrToLong_ui(revision, NULL, 10, &def->revision) < 0) { > +virReportError(VIR_ERR_XML_ERROR, > + _("cannot parse video revision '%s'"), revision); > +goto error; > +
Re: [libvirt] [PATCH] Add support for qxl.revision in domain XML
Ping? On Mon, Feb 04, 2013 at 04:16:36PM +0100, Christophe Fergeau wrote: > QXL devices have an associated 'revision' which is raised when > new features have been introduced which would break migration > to older versions. This commit makes it possible to set this > revision as QEMU sometimes support newer QXL revisions than what > it defaults to. > --- > 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 ram (since > 1.0.2) is allowed for "qxl" type only and specifies > the size of the primary bar, while vram 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 > +value is used. The optional attribute revision ( +class="since">since 1.0.3) specifies the revision of > +the QXL device, newer revisions provides more functionality. > > >model > 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 @@ > > > > + > + > + > + > + > > > > 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"); > > if ((primary = virXMLPropString(cur, "primary")) != NULL) > if (STREQ(primary, "yes")) > @@ -7456,6 +7458,19 @@ virDomainVideoDefParseXML(const xmlNodePtr node, > def->vram = virDomainVideoDefaultRAM(dom, def->type); > } > > +if (revision) { > +if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { > +virReportError(VIR_ERR_XML_ERROR, "%s", > + _("revision attribute only supported for type of > qxl")); > +goto error; > +} > +if (virStrToLong_ui(revision, NULL, 10, &def->revision) < 0) { > +virReportError(VIR_ERR_XML_ERROR, > + _("cannot parse video revision '%s'"), revision); > +goto error; > +} > +} > + > if (heads) { > if (virStrToLong_ui(heads, NULL, 10, &def->heads) < 0) { > virReportError(VIR_ERR_INTERNAL_ERROR, > @@ -13406,6 +13421,8 @@ virDomainVideoDefFormat(virBufferPtr buf, > virBufferAsprintf(buf, " heads='%u'", def->heads); > if (def->primary) > virBufferAddLit(buf, " primary='yes'"); > +if (def->revision) > +virBufferAsprintf(buf, " revision='%u'", def->revision); > if (def->accel) { > virBufferAddLit(buf, ">\n"); > virDomainVideoAccelDefFormat(buf, def->accel); > 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