Re: [Xen-devel] [PATCH v4 23/32] libxl_qmp: Respond to QMP greeting

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jul 27, 2018 at 03:06:05PM +0100, Anthony PERARD wrote:
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_qmp.c | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> index e649b8054d..83afce3192 100644
> --- a/tools/libxl/libxl_qmp.c
> +++ b/tools/libxl/libxl_qmp.c
> @@ -1309,6 +1309,9 @@ int libxl__qmp_initializations(libxl__gc *gc, uint32_t 
> domid,
>  
>  /* ------------ Implementation of libxl__ev_qmp ---------------- */
>  
> +/* hard coded message ID used for capability negociation */
> +static int qmp_capability_negociation_msgid = 1;

Could this be a define instead? Or at least constify it because it
doesn't seem like it's modified (at least in this patch).

>  static int qmp_ev_prepare_cmd(libxl__gc *gc,
>                                libxl__ev_qmp *ev,
>                                const char *cmd,
> @@ -1379,7 +1382,7 @@ static int qmp_ev_handle_response(libxl__egc *egc,
>      }
>  
>      id = libxl__json_object_get_integer(o);
> -    if (id != ev->id)
> +    if (id != ev->id && id != qmp_capability_negociation_msgid)
>          return 0;
>  
>      switch (type) {
> @@ -1413,9 +1416,21 @@ static int qmp_ev_handle_response(libxl__egc *egc,
>          abort();
>      }
>  
> -    ev->id = -1;
> -    ev->callback(egc, ev, response, rc); /* must be last */
> -    return 1;
> +    /*
> +     * Even if the current state is capability_negociation and the correct ID
> +     * as been received, call the callback on error.
> +     */
> +    if (ev->qmp_state == qmp_state_capability_negociation &&
> +        id == qmp_capability_negociation_msgid &&
> +        rc == 0) {
> +        ev->qmp_state = qmp_state_connected;
> +        libxl__ev_fd_modify(gc, &ev->qmp_efd, ev->qmp_efd.events | POLLOUT);
> +        return 0;
> +    } else {
> +        ev->id = -1;
> +        ev->callback(egc, ev, response, rc); /* must be last */
> +        return 1;
> +    }
>  }
>  
>  /* return 1 when a user callback as been called */
> @@ -1429,6 +1444,11 @@ static int qmp_ev_handle_message(libxl__egc *egc,
>      switch (type) {
>      case LIBXL__QMP_MESSAGE_TYPE_QMP:
>          /* greeting message */
> +        assert(ev->qmp_state == qmp_state_connecting);
> +        ev->qmp_state = qmp_state_greeting;
> +        /* Allow qmp_ev_callback_writable to be called in order to send
> +         * qmp_capabilities */
> +        libxl__ev_fd_modify(gc, &ev->qmp_efd, ev->qmp_efd.events | POLLOUT);
>          return 0;
>      case LIBXL__QMP_MESSAGE_TYPE_RETURN:
>      case LIBXL__QMP_MESSAGE_TYPE_ERROR:
> @@ -1563,7 +1583,11 @@ static int qmp_ev_callback_writable(libxl__gc *gc, 
> libxl__ev_qmp *ev, int fd)
>       * be send for now will be in this call. */
>      libxl__ev_fd_modify(gc, &ev->qmp_efd, ev->qmp_efd.events & ~POLLOUT);
>  
> -    if (ev->qmp_state == qmp_state_connected) {
> +    if (ev->qmp_state == qmp_state_greeting) {
> +        buf = qmp_prepare_qmp_cmd(gc, "qmp_capabilities", NULL,
> +                                  qmp_capability_negociation_msgid, &len);
> +        ev->qmp_state = qmp_state_capability_negociation;
> +    } else if (ev->qmp_state == qmp_state_connected) {

A switch (ev->qmp_state) might be better.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel