WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH V6 3/3] libxl, Introduce a QMP client

To: Anthony PERARD <anthony.perard@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH V6 3/3] libxl, Introduce a QMP client
From: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>
Date: Fri, 1 Jul 2011 09:39:53 +0100
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
Delivery-date: Fri, 01 Jul 2011 01:40:32 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1309455045-3062-4-git-send-email-anthony.perard@xxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Citrix Systems, Inc.
References: <1309455045-3062-1-git-send-email-anthony.perard@xxxxxxxxxx> <1309455045-3062-4-git-send-email-anthony.perard@xxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Thu, 2011-06-30 at 18:30 +0100, Anthony PERARD wrote:
> QMP stands for QEMU Monitor Protocol and it is used to query information
> from QEMU or to control QEMU.
> 
> This implementation will ask QEMU the list of chardevice and store the
> path to serial0 in xenstored. So we will be able to use xl console with
> QEMU upstream.
> 
> In order to connect to the QMP server, a socket file is created in
> /var/run/xen/qmp-$(domid).

Should include "libxl" in the name somewhere to differentiate from other
potential connections in the future?

> 
> Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  tools/libxl/Makefile         |    2 +-
>  tools/libxl/libxl.c          |    2 +
>  tools/libxl/libxl_create.c   |    3 +
>  tools/libxl/libxl_dm.c       |    9 +
>  tools/libxl/libxl_internal.h |   19 ++
>  tools/libxl/libxl_qmp.c      |  570 
> ++++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 604 insertions(+), 1 deletions(-)
>  create mode 100644 tools/libxl/libxl_qmp.c
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index 0306cb0..aea0e93 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -37,7 +37,7 @@ LIBXL_LIBS += -lyajl
>  LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
>                         libxl_dom.o libxl_exec.o libxl_xshelp.o 
> libxl_device.o \
>                         libxl_internal.o libxl_utils.o libxl_uuid.o 
> libxl_json.o \
> -                       $(LIBXL_OBJS-y)
> +                       libxl_qmp.o $(LIBXL_OBJS-y)
>  LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
> 
>  $(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest) 
> $(CFLAGS_libxenstore) $(CFLAGS_libblktapctl)
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index abba45e..9d5e547 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -757,6 +757,8 @@ int libxl_domain_destroy(libxl_ctx *ctx, uint32_t domid, 
> int force)
>      if (dm_present) {
>          if (libxl__destroy_device_model(&gc, domid) < 0)
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl__destroy_device_model 
> failed for %d", domid);
> +
> +        libxl__qmp_cleanup(&gc, domid);
>      }
>      if (libxl__devices_destroy(&gc, domid, force) < 0)
>          LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "libxl_destroy_devices failed for 
> %d", domid);
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 5612aeb..28e38fa 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -522,6 +522,9 @@ static int do_domain_create(libxl__gc *gc, 
> libxl_domain_config *d_config,
>      }
> 
>      if (dm_starting) {
> +        if (dm_info->device_model_version == 
> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) {
> +            libxl__qmp_initializations(ctx, domid);
> +        }
>          ret = libxl__confirm_device_model_startup(gc, dm_starting);
>          if (ret < 0) {
>              LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 76d72a5..fa11370 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -249,6 +249,15 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>      flexarray_vappend(dm_args, dm,
>                        "-xen-domid", libxl__sprintf(gc, "%d", info->domid), 
> NULL);
> 
> +    flexarray_append(dm_args, "-chardev");
> +    flexarray_append(dm_args,
> +                     libxl__sprintf(gc, 
> "socket,id=libxl-cmd,path=%s/qmp-%d,server,nowait",
> +                                    libxl_run_dir_path(),
> +                                    info->domid));
> +
> +    flexarray_append(dm_args, "-mon");
> +    flexarray_append(dm_args, "chardev=libxl-cmd,mode=control");
> +
>      if (info->type == LIBXL_DOMAIN_TYPE_PV) {
>          flexarray_append(dm_args, "-xen-attach");
>      }
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 555d9f3..338f8a6 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -381,6 +381,25 @@ _hidden int libxl__e820_alloc(libxl_ctx *ctx, uint32_t 
> domid, libxl_domain_confi
>  #define STRINGIFY(x) #x
>  #define TOSTRING(x) STRINGIFY(x)
> 
> +/* from libxl_qmp */
> +typedef struct libxl__qmp_handler libxl__qmp_handler;
> +
> +/* Initialise and connect to the QMP socket.
> + *   Return an handler or NULL if there is an error
> + */
> +_hidden libxl__qmp_handler *libxl__qmp_initialize(libxl_ctx *ctx,
> +                                                  uint32_t domid);
> +/* ask to QEMU the serial port information and store it in xenstore. */
> +_hidden int libxl__qmp_query_serial(libxl__qmp_handler *qmp);
> +/* close and free the QMP handler */
> +_hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
> +/* remove the socket file, if the file has already been removed,
> + * nothing happen */
> +_hidden void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid);
> +
> +/* this helper calls qmp_initialize, query_serial and qmp_close */
> +_hidden int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid);
> +
>  /* from libxl_json */
>  typedef enum {
>      JSON_ERROR,
> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
> new file mode 100644
> index 0000000..6b03c08
> --- /dev/null
> +++ b/tools/libxl/libxl_qmp.c
> @@ -0,0 +1,570 @@
> +/*
> + * Copyright (C) 2011      Citrix Ltd.
> + * Author Anthony PERARD <anthony.perard@xxxxxxxxxx>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU Lesser General Public License as published
> + * by the Free Software Foundation; version 2.1 only. with the special
> + * exception on linking described in file LICENSE.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU Lesser General Public License for more details.
> + */
> +
> +/*
> + * This file implement a client for QMP (QEMU Monitor Protocol). For the
> + * Specification, see in the QEMU repository.
> + */
> +
> +#include <unistd.h>
> +#include <sys/un.h>
> +#include <sys/queue.h>
> +
> +#include <yajl/yajl_gen.h>
> +
> +#include "libxl_internal.h"
> +
> +/* #define DEBUG_RECEIVED */
> +
> +#ifdef DEBUG_RECEIVED
> +#  define DEBUG_REPORT_RECEIVED(len, buf) \
> +    LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "received: '%.*s'", len, buf)
> +#else
> +#  define DEBUG_REPORT_RECEIVED(len, buf) ((void)0)
> +#endif
> +
> +/*
> + * QMP types & constant
> + */
> +
> +#define QMP_RECEIVE_BUFFER_SIZE 4096
> +
> +typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
> +                              const libxl__json_object *tree);
> +
> +typedef struct callback_id_pair {
> +    int id;
> +    qmp_callback_t callback;
> +    SIMPLEQ_ENTRY(callback_id_pair) next;
> +} callback_id_pair;
> +
> +struct libxl__qmp_handler {
> +    struct sockaddr_un addr;
> +    int qmp_fd;
> +    bool connected;
> +    time_t timeout;
> +    /* wait_for_id will be used by the synchronous send function */
> +    int wait_for_id;
> +
> +    unsigned char buffer[QMP_RECEIVE_BUFFER_SIZE];
> +    libxl__yajl_ctx *yajl_ctx;
> +
> +    libxl_ctx *ctx;
> +    uint32_t domid;
> +
> +    int last_id_used;
> +    SIMPLEQ_HEAD(callback_list, callback_id_pair) callback_list;
> +};
> +
> +static int qmp_send(libxl__qmp_handler *qmp,
> +                    const char *cmd, qmp_callback_t callback);
> +
> +static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;
> +
> +/*
> + * QMP callbacks functions
> + */
> +
> +static int store_serial_port_info(libxl__qmp_handler *qmp,
> +                                  const char *chardev,
> +                                  int port)
> +{
> +    libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);
> +    char *path = NULL;
> +    int ret = 0;
> +
> +    if (!(chardev && strncmp("pty:", chardev, 4) == 0)) {
> +        return -1;
> +    }
> +
> +    path = libxl__xs_get_dompath(&gc, qmp->domid);
> +    path = libxl__sprintf(&gc, "%s/serial/%d/tty", path, port);
> +
> +    ret = libxl__xs_write(&gc, XBT_NULL, path, "%s", chardev + 4);
> +
> +    libxl__free_all(&gc);
> +    return ret;
> +}
> +
> +static int register_serials_chardev_callback(libxl__qmp_handler *qmp,
> +                                             const libxl__json_object *o)
> +{
> +    const libxl__json_object *obj = NULL;
> +    const libxl__json_object *label = NULL;
> +    const char *s = NULL;
> +    flexarray_t *array = NULL;
> +    int index = 0;
> +    const char *chardev = NULL;
> +    int ret = 0;
> +
> +    if ((array = json_object_get_array(o)) == NULL) {
> +        return -1;
> +    }
> +
> +    for (index = 0; index < array->count; index++) {
> +        if (flexarray_get(array, index, (void**)&obj) != 0)
> +            break;
> +        label = json_object_get("label", obj, JSON_STRING);
> +        s = json_object_get_string(label);
> +
> +        if (s && strncmp("serial", s, strlen("serial")) == 0) {
> +            const libxl__json_object *filename = NULL;
> +            char *endptr = NULL;
> +            int port_number;
> +
> +            filename = json_object_get("filename", obj, JSON_STRING);
> +            chardev = json_object_get_string(filename);
> +
> +            s += strlen("serial");
> +            port_number = strtol(s, &endptr, 10);
> +            if (*s == 0 || *endptr != 0) {
> +                LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
> +                           "Invalid serial port number: %s", s);
> +                return -1;
> +            }
> +            ret = store_serial_port_info(qmp, chardev, port_number);
> +            if (ret) {
> +                LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
> +                                 "Failed to store serial port information"
> +                                 " in xenstore");
> +                return ret;
> +            }
> +        }
> +    };
> +
> +    return ret;
> +}
> +
> +static int qmp_capabilities_callback(libxl__qmp_handler *qmp,
> +                                     const libxl__json_object *o)
> +{
> +    qmp->connected = true;
> +
> +    return 0;
> +}
> +
> +/*
> + * QMP commands
> + */
> +
> +static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
> +{
> +    return qmp_send(qmp, "qmp_capabilities", qmp_capabilities_callback);
> +}
> +
> +/*
> + * Helpers
> + */
> +
> +static libxl__qmp_message_type qmp_response_type(libxl__qmp_handler *qmp,
> +                                                 const libxl__json_object *o)
> +{
> +    flexarray_t *maps = json_object_get_map(o);
> +    libxl__qmp_message_type type;
> +
> +    if (maps) {
> +        libxl__json_map_node *node = NULL;
> +        int index = 0;
> +
> +        for (index = 0; index < maps->count; index++) {
> +            if (flexarray_get(maps, index, (void**)&node) != 0)
> +                break;
> +            if (libxl__qmp_message_type_from_string(node->map_key, &type) == 
> 0)
> +                return type;
> +        }
> +    }
> +
> +    return LIBXL__QMP_MESSAGE_TYPE_INVALID;
> +}
> +
> +static callback_id_pair *qmp_get_callback_from_id(libxl__qmp_handler *qmp,
> +                                                  const libxl__json_object 
> *o)
> +{
> +    const libxl__json_object *id_object = json_object_get("id", o,
> +                                                          JSON_INTEGER);
> +    int id = -1;
> +    callback_id_pair *pp = NULL;
> +
> +    if (id_object) {
> +        id = json_object_get_integer(id_object);
> +
> +        SIMPLEQ_FOREACH(pp, &qmp->callback_list, next) {
> +            if (pp->id == id) {
> +                return pp;
> +            }
> +        }
> +    }
> +    return NULL;
> +}
> +
> +static void qmp_handle_error_response(libxl__qmp_handler *qmp,
> +                                      const libxl__json_object *resp)
> +{
> +    callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
> +
> +    resp = json_object_get("error", resp, JSON_MAP);
> +    resp = json_object_get("desc", resp, JSON_STRING);
> +
> +    if (pp) {
> +        if (pp->id == qmp->wait_for_id) {
> +            /* tell that the id have been processed */
> +            qmp->wait_for_id = 0;
> +        }
> +        SIMPLEQ_REMOVE(&qmp->callback_list, pp, callback_id_pair, next);
> +        free(pp);
> +    }
> +
> +    LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
> +               "received an error message from QMP server: %s",
> +               json_object_get_string(resp));
> +}
> +
> +static int qmp_handle_response(libxl__qmp_handler *qmp,
> +                               const libxl__json_object *resp)
> +{
> +    libxl__qmp_message_type type = LIBXL__QMP_MESSAGE_TYPE_INVALID;
> +
> +    type = qmp_response_type(qmp, resp);
> +    LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG,
> +               "message type: %s", libxl__qmp_message_type_to_string(type));
> +
> +    switch (type) {
> +    case LIBXL__QMP_MESSAGE_TYPE_QMP:
> +        /* On the greeting message from the server, enable QMP capabilities 
> */
> +        enable_qmp_capabilities(qmp);
> +        break;
> +    case LIBXL__QMP_MESSAGE_TYPE_RETURN: {
> +        callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);
> +
> +        if (pp) {
> +            pp->callback(qmp, json_object_get("return", resp, JSON_ANY));
> +            if (pp->id == qmp->wait_for_id) {
> +                /* tell that the id have been processed */
> +                qmp->wait_for_id = 0;
> +            }
> +            SIMPLEQ_REMOVE(&qmp->callback_list, pp, callback_id_pair, next);
> +            free(pp);
> +        }
> +        break;
> +    }
> +    case LIBXL__QMP_MESSAGE_TYPE_ERROR:
> +        qmp_handle_error_response(qmp, resp);
> +        break;
> +    case LIBXL__QMP_MESSAGE_TYPE_EVENT:
> +        break;
> +    case LIBXL__QMP_MESSAGE_TYPE_INVALID:
> +        return -1;
> +    }
> +    return 0;
> +}
> +
> +static inline yajl_gen_status libxl__yajl_gen_asciiz(yajl_gen hand,
> +                                                     const char *str)
> +{
> +    return yajl_gen_string(hand, (const unsigned char *)str, strlen(str));
> +}
> +
> +/*
> + * Handler functions
> + */
> +
> +static libxl__qmp_handler *qmp_init_handler(libxl_ctx *ctx, uint32_t domid)

libxl__qmp_init_handler?

Generally internal functions should take a libxl__gc *gc rather than a
ctx (applies to a bunch of functions).

> +{
> +    libxl__qmp_handler *qmp = NULL;
> +
> +    qmp = calloc(1, sizeof (libxl__qmp_handler));

Could be attached to the libxl__gc infrastructure instead of using an
explicit free?

BTW, I was wondering the same thing as I read the parser stuff in the
previous patch but that would involve plumbing a *gc through and careful
thought about whether or not a given data field could ever reach the
libxl user (e.g. JSON_STRING's could ultimately get returned to the
caller).

> +static int qmp_open(libxl__qmp_handler *qmp, const char *qmp_socket_path,
> +                    int timeout)
> +{
> +    int ret;
> +    int i = 0;
> +
> +    qmp->qmp_fd = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0);
> +    if (qmp->qmp_fd < 0) {
> +        return -1;
> +    }
> +
> +    memset(&qmp->addr, 0, sizeof (&qmp->addr));
> +    qmp->addr.sun_family = AF_UNIX;
> +    strncpy(qmp->addr.sun_path, qmp_socket_path,
> +            sizeof (qmp->addr.sun_path));
> +
> +    do {
> +        ret = connect(qmp->qmp_fd, (struct sockaddr *) &qmp->addr,
> +                      sizeof (qmp->addr));
> +        if (ret == 0)
> +            break;
> +        if (errno == ENOENT || errno == ECONNREFUSED) {
> +            /* ENOENT       : Socket may not have shown up yet
> +             * ECONNREFUSED : Leftover socket hasn't been removed yet */
> +            continue;
> +        }
> +        return -1;
> +    } while ((++i / 5 <= timeout) && (usleep(200 * 1000) <= 0));

If we exit this loop because we timed out do we need to set errno to
ETIMEDOUT or something similar to indicate this? Or is the existing err
errno (one of ENOENT or ECONNREFUSED?) a sufficient indicator?

> +
> +    return ret;
> +}
> +
> +static void qmp_close(libxl__qmp_handler *qmp)
> +{
> +    callback_id_pair *pp = NULL;
> +    callback_id_pair *tmp = NULL;
> +
> +    close(qmp->qmp_fd);
> +    SIMPLEQ_FOREACH(pp, &qmp->callback_list, next) {
> +        if (tmp)
> +            free(tmp);
> +        tmp = pp;

That seems like an odd construct, but I suppose it is necessary to work
around the lack of a SIMPLEQ_FOREACH variant which is safe against
removing the iterator from the list.

> +libxl__qmp_handler *libxl__qmp_initialize(libxl_ctx *ctx, uint32_t domid)
> +{
> +    int ret = 0;
> +    libxl__qmp_handler *qmp = NULL;
> +    char *qmp_socket;
> +    libxl__gc gc = LIBXL_INIT_GC(ctx);
> +
> +    qmp = qmp_init_handler(ctx, domid);
> +
> +    qmp_socket = libxl__sprintf(&gc, "%s/qmp-%d",
> +                                libxl_run_dir_path(), domid);
> +    if ((ret = qmp_open(qmp, qmp_socket, QMP_SOCKET_CONNECT_TIMEOUT)) < 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Connection error");
> +        qmp_free_handler(qmp);
> +        return NULL;
> +    }
> +
> +    LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "connected to %s", qmp_socket);
> +
> +    /* Wait for the response to qmp_capabilities */
> +    while (!qmp->connected) {
> +        if ((ret = qmp_next(qmp)) < 0) {
> +            break;

Is it an error condition not to get in to the connected state? Should we
therefore qmp_free_handler and return NULL? That would save callers
checking qmp->connected since they can just assume it is true...

[...]
> +int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
> +{
> +    libxl__qmp_handler *qmp = NULL;
> +    int ret = 0;
> +
> +    qmp = libxl__qmp_initialize(ctx, domid);
> +    if (!qmp)
> +        return -1;
> +    if (qmp->connected) {

... like here.

> +        ret = libxl__qmp_query_serial(qmp);
> +    }
> +    libxl__qmp_close(qmp);
> +    return ret;
> +}
> --
> 1.7.2.5
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>