Mailing List Archive

[PATCH V2 08/11] libxl_qmp, Introduce qmp_request_handle.
This structure helps keep the return code of the callback, so a caller can read
it.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_qmp.c | 71 +++++++++++++++++++++++++++++-----------------
1 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 5ea9429..547cd53 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -46,10 +46,16 @@ typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
const libxl__json_object *tree,
void *opaque);

-typedef struct callback_id_pair {
- int id;
+typedef struct qmp_request_handle {
qmp_callback_t callback;
void *opaque;
+ /* return code of the callback */
+ int rc;
+} qmp_request_handle;
+typedef struct callback_id_pair {
+ qmp_request_handle *handle;
+ bool free_it; /* tell if handle need to be freed */
+ int id;
SIMPLEQ_ENTRY(callback_id_pair) next;
} callback_id_pair;

@@ -73,7 +79,7 @@ struct libxl__qmp_handler {

static int qmp_send(libxl__qmp_handler *qmp,
const char *cmd, libxl_key_value_list *args,
- qmp_callback_t callback, void *opaque);
+ qmp_request_handle *handle, bool free_it);

static const int QMP_SOCKET_CONNECT_TIMEOUT = 5;

@@ -161,8 +167,9 @@ static int qmp_capabilities_callback(libxl__qmp_handler *qmp,

static int enable_qmp_capabilities(libxl__qmp_handler *qmp)
{
- return qmp_send(qmp, "qmp_capabilities", NULL,
- qmp_capabilities_callback, NULL);
+ qmp_request_handle *h = calloc(1, sizeof (qmp_request_handle));
+ h->callback = qmp_capabilities_callback;
+ return qmp_send(qmp, "qmp_capabilities", NULL, h, 1);
}

/*
@@ -213,14 +220,17 @@ static void qmp_handle_error_response(libxl__qmp_handler *qmp,
resp = libxl__json_map_get("desc", resp, JSON_STRING);

if (pp) {
- if (pp->callback) {
- pp->callback(qmp, NULL, pp->opaque);
+ qmp_request_handle *h = pp->handle;
+ if (h) {
+ h->rc = h->callback(qmp, NULL, h->opaque);
}
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);
+ if (pp->free_it)
+ free(pp->handle);
free(pp);
}

@@ -241,31 +251,33 @@ static int qmp_handle_response(libxl__qmp_handler *qmp,
switch (type) {
case LIBXL__QMP_MESSAGE_TYPE_QMP:
/* On the greeting message from the server, enable QMP capabilities */
- enable_qmp_capabilities(qmp);
- break;
+ return enable_qmp_capabilities(qmp);
case LIBXL__QMP_MESSAGE_TYPE_RETURN: {
callback_id_pair *pp = qmp_get_callback_from_id(qmp, resp);

if (pp) {
- if (pp->callback) {
- pp->callback(qmp,
+ qmp_request_handle *h = pp->handle;
+ if (h) {
+ h->rc = h->callback(qmp,
libxl__json_map_get("return", resp, JSON_ANY),
- pp->opaque);
+ h->opaque);
}
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);
+ if (pp->free_it)
+ free(pp->handle);
free(pp);
}
- break;
+ return 0;
}
case LIBXL__QMP_MESSAGE_TYPE_ERROR:
qmp_handle_error_response(qmp, resp);
- break;
+ return -1;
case LIBXL__QMP_MESSAGE_TYPE_EVENT:
- break;
+ return 0;
case LIBXL__QMP_MESSAGE_TYPE_INVALID:
return -1;
}
@@ -358,6 +370,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)

char *incomplete = NULL;
size_t incomplete_size = 0;
+ int rc = 0;

do {
fd_set rfds;
@@ -415,7 +428,7 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
o = libxl__json_parse(gc, s);

if (o) {
- qmp_handle_response(qmp, o);
+ rc = qmp_handle_response(qmp, o);
libxl__json_object_free(gc, o);
} else {
LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
@@ -430,12 +443,12 @@ static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp)
} while (s < s_end);
} while (s < s_end);

- return 1;
+ return rc;
}

static int qmp_send(libxl__qmp_handler *qmp,
const char *cmd, libxl_key_value_list *args,
- qmp_callback_t callback, void *opaque)
+ qmp_request_handle *handle, bool free_it)
{
yajl_gen_config conf = { 0, NULL };
const unsigned char *buf;
@@ -475,8 +488,8 @@ static int qmp_send(libxl__qmp_handler *qmp,
goto error;
}
elm->id = qmp->last_id_used;
- elm->callback = callback;
- elm->opaque = opaque;
+ elm->handle = handle;
+ elm->free_it = free_it;
SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);

LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "next qmp command: '%s'", buf);
@@ -499,14 +512,14 @@ error:

static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
libxl_key_value_list *args,
- qmp_callback_t callback, void *opaque,
+ qmp_request_handle *handle,
int ask_timeout)
{
int id = 0;
int ret = 0;
libxl__gc gc = LIBXL_INIT_GC(qmp->ctx);

- id = qmp_send(qmp, cmd, args, callback, opaque);
+ id = qmp_send(qmp, cmd, args, handle, 0);
if (id <= 0) {
return -1;
}
@@ -594,10 +607,16 @@ void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)

int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
{
- return qmp_synchronous_send(qmp, "query-chardev", NULL,
- register_serials_chardev_callback,
- NULL,
- qmp->timeout);
+ int rc = 0;
+ qmp_request_handle request = {
+ .callback = register_serials_chardev_callback,
+ };
+ rc = qmp_synchronous_send(qmp, "query-chardev", NULL,
+ &request, qmp->timeout);
+ if (rc == 0) {
+ rc = request.rc;
+ }
+ return rc;
}

int libxl__qmp_initializations(libxl_ctx *ctx, uint32_t domid)
--
Anthony PERARD


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH V2 08/11] libxl_qmp, Introduce qmp_request_handle. [ In reply to ]
Anthony PERARD writes ("[Xen-devel] [PATCH V2 08/11] libxl_qmp, Introduce qmp_request_handle."):
> This structure helps keep the return code of the callback, so a
> caller can read it.

This is heading in a plausible direction but:

* You need to state which of the members of the struct are for use by
which parts of libxl, and when any shared members are written etc.

* "handle" is the wrong word for this. "handle" would refer to a
pointer or perhaps descriptor. This is a context/info structure.

* The "free_it" parameter to qmp_send seems odd. Surely it should
just be the case that the same person that allocated the context
should free it ?

Ian.

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