Re: [PATCH] qmp: Reset mon->commands on CHR_EVENT_CLOSED

[Marc-André Lureau]

Hi

On Wed, Nov 6, 2019 at 5:04 PM Jason Andryuk <address@hidden> wrote:
>
> Currently, mon->commands is uninitialized until CHR_EVENT_OPENED where
> it is set to &qmp_cap_negotiation_commands.  After capability
> negotiation, it is set to &qmp_commands.  If the chardev is closed,
> CHR_EVENT_CLOSED, mon->commands remains as &qmp_commands.  Only once the
> chardev is re-opened with CHR_EVENT_OPENED, is it reset to
> &qmp_cap_negotiation_commands.
>
> monitor_qapi_event_emit compares mon->commands to
> &qmp_cap_negotiation_commands, and skips sending events when they are
> equal.  In the case of a closed chardev, QMP events are still sent down
> to the closed chardev which needs to drop them.

This is a minor improvement, not really a bug fix or do I read that incorrectly?

>
> Set mon->commands to &qmp_cap_negotiation_commands for CHR_EVENT_CLOSED
> to stop sending events.  Setting for the CHR_EVENT_OPENED case remains
> since that is how mon->commands is set for a newly opened connections.
>
> Signed-off-by: Jason Andryuk <address@hidden>
> ---
>  monitor/qmp.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 9d9e5d8b27..5e2073c5eb 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -333,6 +333,7 @@ static void monitor_qmp_event(void *opaque, int event)
>           * is closed.
>           */
>          monitor_qmp_cleanup_queues(mon);
> +        mon->commands = &qmp_cap_negotiation_commands;
>          json_message_parser_destroy(&mon->parser);
>          json_message_parser_init(&mon->parser, handle_qmp_command,
>                                   mon, NULL);
> --
> 2.21.0
>
>

Looks good to me,
Reviewed-by: Marc-André Lureau <address@hidden>


-- 
Marc-André Lureau