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

Re: [libvirt] [PATCH] Support for 3d Acceleration in video tag



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/


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