On 06/27/2013 07:29 AM, Luiz Capitulino wrote: > On Thu, 27 Jun 2013 12:05:33 +0200 > Gerd Hoffmann <kra...@redhat.com> wrote: >
>>>> +# @device: #optional specifies framebuffer device, default: /dev/fb0 >>> >>> Actually, it will try to get the device name from an env variable first, >>> which sounds too automatic for a building block API like QMP. >> >> The env variable is icky indeed. But a fixed default (which will be the >> one you need in 99% of the cases) is fine IMO. > > I prefer having no defaults, but I'm not strong about this. I also think that letting HMP leave the parameter optional, with first-level fallback to the environment variable and secondary fallback to the hardcoded /dev/fv0, while making the parameter mandatory in QMP, makes more sense. > >> >>>> +# Since 1.6 >>>> +## >>>> +{ 'type': 'FramebufferInfo', >>>> + 'data': { 'enabled': 'bool', >>>> + '*scale' : 'bool', >>>> + '*device': 'str', >>> >>> Why is device optional? >> >> When the framebuffer is'nt active you'll get just >> { "enabled" : "false" }, thats why the other ones are optional. They >> all are filled in case the framebuffer is active. Would it be any simpler to just have the query command fail when the framebuffer is disabled, leave out the 'enabled' member from FramebufferInfo, and have all the other members mandatory? Also, would it ever make sense for a future enhancement to allow having more than one framebuffer in use by a single domain? Even if your initial implementation supports at most one /dev/fb* device per guest, it might be worth having the query-framebuffer command return an array of all enabled framebuffers (a 0-length array when none is enabled), rather than hard-coding it to a single FramebufferInfo return. Likewise, make sure that framebuffer-display can deal with extension if it makes sense (the suggestion to split into -enable and -disable commands might be better stated as splitting into framebuffer-display-add and framebuffer-display-delete, more like hotplugging of other devices). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature