qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC v3 18/27] qmp: add new event "request-dropped"


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [RFC v3 18/27] qmp: add new event "request-dropped"
Date: Wed, 15 Nov 2017 10:50:15 +0000
User-agent: Mutt/1.9.1 (2017-09-22)

On Mon, Nov 06, 2017 at 05:46:34PM +0800, Peter Xu wrote:
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 531fd4c0db..650714da06 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3222,3 +3222,38 @@
>  # Since: 2.11
>  ##
>  { 'command': 'watchdog-set-action', 'data' : {'action': 'WatchdogAction'} }
> +
> +##
> +# @RequestDropReason:
> +#
> +# Reasons that caused one request to be dropped.

Please use "command" consistently.  QMP does not call it not "request".

> +#
> +# @queue-full: the queue of request is full.
> +#
> +# Since: 2.12
> +##
> +{ 'enum': 'RequestDropReason',
> +  'data': ['queue-full' ] }
> +
> +##
> +# @REQUEST_DROPPED:
> +#
> +# Emitted when one QMP request is dropped due to some reason.

Please add:

  REQUEST_DROPPED is only emitted when the oob capability is enabled.

Rationale: old clients don't know about this event so they cannot be
expected to handle it!

> +#
> +# @id:    If the original request contains an string-typed "id" field,
> +#         it'll be put into this field.  Otherwise it'll be an empty
> +#         string.

Please change:

  @id: The dropped command's string-typed "id" field.

Sending commands without the id field is likely to cause confusion since
there are cases where the client is unable to determine which command
was meant.  Since client code needs to be updated to enable the oob
capability anyway, we might as well require that clients always include
the id field with every command when the oob capability is enabled.
Please mention this requirement where the oob capability is documented.

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]