Re: [libvirt] [PATCH] add fullscreen support to qemu sdl xml (via fullscreen='true' attribute for the graphics element)

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

 



On Wed, Dec 10, 2008 at 06:06:51AM -0500, Itamar Heim wrote:
> Sure - probably not a good idea to send patches at such a late hour anyway
> :)
> Attached diff.
> 
> Some more details:
> For those using SDL ("for some crazy reason" as Berrange put it in the
> original patch), there is a use for the fullscreen option, causing the
> qemu window to open in full screen, change resolution, etc.
> This patch accepts an additional attribute in the graphics element:
> <graphics type='sdl' display=':0.1' xauth='/root/.Xauthority'
> fullscreen='true'/>
> 
> If fullscreen is true, "-full-screen" is added to the qemu command line
> (unlike the display and auth which are added as environment variables)

  Looks overall fine to me, the syntactic construct sounds generic
enough.
  I would still suggest 2 more changes:
    - in virDomainGraphicsDefParseXML check the values coming from
      the XML, I guess we should accept only "true" and "false"
      and raise an error, otherwise one may pass --fullscreen=foo
      on the command line !
    - drop stdbool.h and use int instead of bool, this doesn't really
      gain anything and causes some troubles (e.g. with CIL)

Also if you could set your mail agent to set a mime-type of text/plain
on the diff or patch attachemnts, that would allow to comment them
in-line, which allows for faster feedback loop in the future.

  thanks :-) !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]