Re: [Qemu-devel] [PATCH 2/2] fbdev: add monitor commands to enable/disable/query

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1247 bytes --]

On 06/26/2013 09:56 AM, Luiz Capitulino wrote:
> On Wed, 26 Jun 2013 13:38:04 +0200
> Gerd Hoffmann <kraxel@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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]