[libvirt] [PATCH] Support for 3d Acceleration in video tag
Hi All, I have added support for 3d Acceleration in the video tag, the patch for the same is attached here. Regards, Pritesh
On Mon, Aug 10, 2009 at 01:51:44PM +0200, Pritesh Kothari wrote:
Hi All,
I have added support for 3d Acceleration in the video tag, the patch for the same is attached here.
I recall you said there might be other options /settings related to 3d acceleration in virtualbox in the future. If so I think it might be worth making that element all of its own <video> <model type='vbox'> <3daccel /> </model> </video> So we can easily add more attributes on the '3daccel' element keeping it all grouped together nicely Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
I recall you said there might be other options /settings related to 3d acceleration in virtualbox in the future. If so I think it might be worth making that element all of its own
<video> <model type='vbox'> <3daccel /> </model> </video>
true, will add a element called <acceleration/> cause there are some features for 2d acceleration as well, so that will take care of 3d and 2d acceleration both. will post a patch soon with the above changes in it. Regards, Pritesh
true, will add a element called <acceleration/> cause there are some features for 2d acceleration as well, so that will take care of 3d and 2d acceleration both.
will post a patch soon with the above changes in it.
Reposting the patch with changes mentioned above. Regards, Pritesh
On Wed, Aug 19, 2009 at 10:45:25AM +0200, Pritesh Kothari wrote:
true, will add a element called <acceleration/> cause there are some features for 2d acceleration as well, so that will take care of 3d and 2d acceleration both.
will post a patch soon with the above changes in it.
Reposting the patch with changes mentioned above. [...] diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index f857301..8f82e01 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -814,6 +814,26 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <element name="acceleration"> + <optional> + <attribute name="3d"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + <optional> + <attribute name="2d"> + <choice> + <value>yes</value> + <value>no</value> + </choice> + </attribute> + </optional> + </element> + </optional> </element> </optional> </element>
I'm afraid that in the long run we may have to deal with far more video emulation options, a bit like cpu emulation flags, but that's a reasonable approach for now. [...]
diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..2f3a02d 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -307,12 +307,21 @@ enum virDomainVideoType { };
+typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef; +typedef virDomainVideoAccelDef *virDomainVideoAccelDefPtr; +struct _virDomainVideoAccelDef { + int support3d : 1; + int support2d : 1; +}; + + typedef struct _virDomainVideoDef virDomainVideoDef; typedef virDomainVideoDef *virDomainVideoDefPtr; struct _virDomainVideoDef { int type; unsigned int vram; unsigned int heads; + virDomainVideoAccelDef accel; };
/* 3 possible graphics console modes */
I'm not that fond of adding a substructure like this by value, I don't really see what this brings and make the code below less clever
@@ -3824,7 +3876,12 @@ virDomainVideoDefFormat(virConnectPtr conn, virBufferVSprintf(buf, " vram='%u'", def->vram); if (def->heads) virBufferVSprintf(buf, " heads='%u'", def->heads); - virBufferAddLit(buf, "/>\n"); + virBufferAddLit(buf, ">\n"); + + virDomainVideoAccelDefFormat(buf, def->accel); + + virBufferAddLit(buf, " </model>\n"); + virBufferAddLit(buf, " </video>\n");
return 0;
as this forces us to always keep the model closed with an end tag. As a result a lot of the regression tests fail because the format changed. I will revamp that part a bit before pushing this out. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Wed, Sep 02, 2009 at 06:23:01PM +0200, Daniel Veillard wrote:
On Wed, Aug 19, 2009 at 10:45:25AM +0200, Pritesh Kothari wrote:
true, will add a element called <acceleration/> cause there are some features for 2d acceleration as well, so that will take care of 3d and 2d acceleration both.
will post a patch soon with the above changes in it.
Reposting the patch with changes mentioned above. [...] diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index f857301..8f82e01 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -814,6 +814,26 @@ <ref name="unsignedInt"/> </attribute> </optional> + <optional> + <element name="acceleration"> + <optional> + <attribute name="3d">
3d and 2d are not XML Names, so changed to accel2d and accel3d, which is actually what was used in the code, so the rng was just behind. Note: please make check before sending a patch, thanks ;-)
+ <attribute name="2d"> [...]
I'm afraid that in the long run we may have to deal with far more video emulation options, a bit like cpu emulation flags, but that's a reasonable approach for now.
[...]
diff --git a/src/domain_conf.h b/src/domain_conf.h index 44302be..2f3a02d 100644 --- a/src/domain_conf.h +++ b/src/domain_conf.h @@ -307,12 +307,21 @@ enum virDomainVideoType { };
+typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef; +typedef virDomainVideoAccelDef *virDomainVideoAccelDefPtr; +struct _virDomainVideoAccelDef { + int support3d : 1; + int support2d : 1; +}; + + typedef struct _virDomainVideoDef virDomainVideoDef; typedef virDomainVideoDef *virDomainVideoDefPtr; struct _virDomainVideoDef { int type; unsigned int vram; unsigned int heads; + virDomainVideoAccelDef accel; };
/* 3 possible graphics console modes */
I'm not that fond of adding a substructure like this by value, I don't really see what this brings and make the code below less clever
It was actually in my opinion severley broken, the parsing routine was allocating the struct on teh stack and passing it as a return value, to avoid this I used virDomainVideoAccelDefPtr accel in the referencing structure, and makes virDomainVideoAccelDefParseXML allocate the structure, and return is by pointer as it should.
@@ -3824,7 +3876,12 @@ virDomainVideoDefFormat(virConnectPtr conn, virBufferVSprintf(buf, " vram='%u'", def->vram); if (def->heads) virBufferVSprintf(buf, " heads='%u'", def->heads); - virBufferAddLit(buf, "/>\n"); + virBufferAddLit(buf, ">\n"); + + virDomainVideoAccelDefFormat(buf, def->accel); + + virBufferAddLit(buf, " </model>\n"); + virBufferAddLit(buf, " </video>\n");
I also fixed this to call virDomainVideoAccelDefFormat only if def->accel is non null i.e. if an acceleration element with attribute was found. thanks, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel P. Berrange -
Daniel Veillard -
Pritesh Kothari