Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices.

[Eric Blake <eblake@redhat.com>]

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

On 05/06/2013 07:43 AM, John Baboval wrote:
> From: "John V. Baboval" <john.baboval@virtualcomputer.com>
> 
> This parameter will cause writes to tty backed chardevs to return
> -EAGAIN if the backing tty has buffered more than the specified
> number of characters. When data is sent, the TIOCOUTQ ioctl is invoked
> to determine the current TTY output buffer depth.
> 

Reviewing just the interface portion of the patch:

> +++ b/qapi-schema.json
> @@ -3182,11 +3182,14 @@
>  #
>  # @device: The name of the special file for the device,
>  #          i.e. /dev/ttyS0 on Unix or COM1: on Windows
> +# @maxqdepth: The maximum depth of the underlying tty
> +              output queue (Unix) 

Trailing whitespace.  Run your patch through scripts/checkpatch.pl.

Since you are adding a new member, you should use a "(since 1.6)"
comment on this line.  Also, most interfaces tend to use a blank line
between member documentation.

>  # @type: What kind of device this is.

Hmm - we have a pre-existing documentation bug - this line probably
should have been deleted during commit d36b2b90.

>  #
>  # Since: 1.4
>  ##
> -{ 'type': 'ChardevHostdev', 'data': { 'device' : 'str' } }
> +{ 'type': 'ChardevHostdev', 'data': { 'device'    : 'str',
> +                                      'maxqdepth' : 'int' } }

Ouch - this says that maxqdepth is mandatory.  But that is a
backwards-incompatible change with apps that target the 'chardev-add'
QMP command of qemu 1.4.  You MUST make it optional, since older apps
will not be providing it.

-- 
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 --]