On 06/26/2013 09:56 AM, Luiz Capitulino wrote: > On Wed, 26 Jun 2013 13:38:04 +0200 > Gerd Hoffmann <kra...@redhat.com> wrote: > >> This patch adds a fbdev monitor command to enable/disable >> the fbdev display at runtime to both qmp and hmp. >>
>> +##
>> +# @framebuffer-display:
>
> Let me bike-shed: we're trying to make command's names verbs. So, we
> could call this framebuffer-display-set or maybe have two commands,
> framebuffer-display-enable and framebuffer-display-disable. I prefer
> the latter.
Having 2 commands also avoids the semantic quandary of what to do for
"enable":false,"device":"/path/to/non-default" - the device parameter
only makes sense when enabling the framebuffer display.
>> +
>> +##
>> +# @FramebufferInfo:
>> +#
>
> Missing docs.
>
>> +# Since 1.6
>> +##
>> +{ 'type': 'FramebufferInfo',
>> + 'data': { 'enabled': 'bool',
>> + '*scale' : 'bool',
>> + '*device': 'str',
>
> Why is device optional?
>
>> + '*vtno' : 'int' } }
Also, 'vtno' isn't a word; is it worth spelling it out a bit more by
naming it 'vt-number'?
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
