[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character d
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] Add 'maxqdepth' as an option to tty character devices. |
Date: |
Mon, 06 May 2013 08:43:25 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130402 Thunderbird/17.0.5 |
On 05/06/2013 07:43 AM, John Baboval wrote:
> From: "John V. Baboval" <address@hidden>
>
> 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
signature.asc
Description: OpenPGP digital signature