| Message ID | 20171106094643.14881-19-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC,v3,01/27] char-io: fix possible race on IOWatchPoll | expand |
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.
On Wed, Nov 15, 2017 at 10:50:15AM +0000, Stefan Hajnoczi wrote: > 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". Sure. > > > +# > > +# @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! Added. > > > +# > > +# @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. Ok. > > 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. Will do. Thanks!
On Thu, Nov 16, 2017 at 01:56:54PM +0800, Peter Xu wrote: > On Wed, Nov 15, 2017 at 10:50:15AM +0000, Stefan Hajnoczi wrote: > > 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". > > Sure. > > > > > > +# > > > +# @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! > > Added. > > > > > > +# > > > +# @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. > > Ok. I just found this one tricky: currently we allow all kinds of "id" fields in QMP commands, which means that here I should allow all kinds of "id" fields as well rather than restrict it as "string" typed. But I don't really know how to do that in QMP, say, what I want is something like: { 'event': 'REQUEST_DROPPED' , 'data': { 'id': 'object', 'reason': 'RequestDropReason' } } ^^^^^^^^ Any thoughts on how to do that the simple way?
On Thu, Nov 16, 2017 at 02:29:05PM +0800, Peter Xu wrote: [...] > > > > +# > > > > +# @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. > > > > Ok. > > I just found this one tricky: currently we allow all kinds of "id" > fields in QMP commands, which means that here I should allow all kinds > of "id" fields as well rather than restrict it as "string" typed. > > But I don't really know how to do that in QMP, say, what I want is > something like: > > { 'event': 'REQUEST_DROPPED' , > 'data': { 'id': 'object', 'reason': 'RequestDropReason' } } > ^^^^^^^^ > > Any thoughts on how to do that the simple way? Please ignore it. I just noticed that we have type 'any' which I can use. Sorry for the noise.
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. +# +# @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. +# +# @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. +# +# @reason: The reason why the request is dropped. +# +# Since: 2.12 +# +# Example: +# +# { "event": "REQUEST_DROPPED", +# "data": {"result": {"id": "libvirt-102", +# "reason": "queue-full" } } } +# +## +{ 'event': 'REQUEST_DROPPED' , + 'data': { 'id': 'str', 'reason': 'RequestDropReason' } }
This event will be emitted if one QMP request is dropped. Along, declare an enum for the reasons. Signed-off-by: Peter Xu <peterx@redhat.com> --- qapi-schema.json | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)