New:
- Only one makefile rules for all user of gentypes.py.
- No more #ifdef DEBUG_ANSWER in the code flow, there is now macros
DEBUG_GEN_*
- Introduce libxl_json.c for the JSON parsing stuff.
Change v4->v5:
- Add a separate patch for XEN_RUN_DIR path.
- Add a new patch to generate an some internal type with gentypes.py.
It''s
used only to have a ENUM <-> String convertion with the QMP client.
- libxl_qmp.h content have been moved to libxl_internal.h
- return value of every single alloc are now check
- introduce some DEBUG_GEN_* macro to avoid some #ifdefs
- remove unused exported function: libxL__qmp_next libxl__qmp_send_command
- introduce libxl__qmp_query_serial and store all serial port chardev in
xenstore.
- introduce json_object_{get,is}_* inline functions.
- coding style fixed
Anthony PERARD (3):
libxl: Introduce libxl_internal_types.idl.
libxl: Introduce JSON parsing stuff.
libxl, Introduce a QMP client
tools/libxl/Makefile | 21 +-
tools/libxl/gentypes.py | 15 +-
tools/libxl/libxl.c | 2 +
tools/libxl/libxl_create.c | 3 +
tools/libxl/libxl_dm.c | 9 +
tools/libxl/libxl_internal.h | 117 ++++++
tools/libxl/libxl_json.c | 521 +++++++++++++++++++++++++
tools/libxl/libxl_qmp.c | 570 ++++++++++++++++++++++++++++
tools/libxl/{libxl.idl => libxl_types.idl} | 0
tools/libxl/libxl_types_internal.idl | 10 +
10 files changed, 1253 insertions(+), 15 deletions(-)
create mode 100644 tools/libxl/libxl_json.c
create mode 100644 tools/libxl/libxl_qmp.c
rename tools/libxl/{libxl.idl => libxl_types.idl} (100%)
create mode 100644 tools/libxl/libxl_types_internal.idl
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jun-30 17:30 UTC
[Xen-devel] [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/Makefile | 16 +++++++++-------
tools/libxl/gentypes.py | 15 ++++++++-------
tools/libxl/libxl_internal.h | 1 +
tools/libxl/{libxl.idl => libxl_types.idl} | 0
tools/libxl/libxl_types_internal.idl | 10 ++++++++++
5 files changed, 28 insertions(+), 14 deletions(-)
rename tools/libxl/{libxl.idl => libxl_types.idl} (100%)
create mode 100644 tools/libxl/libxl_types_internal.idl
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index bfe9c58..a95cd5d 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -35,7 +35,7 @@ LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
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_OBJS-y)
-LIBXL_OBJS += _libxl_types.o libxl_flask.o
+LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
$(LIBXL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenguest)
$(CFLAGS_libxenstore) $(CFLAGS_libblktapctl)
@@ -51,8 +51,8 @@ $(XL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
$(XL_OBJS): CFLAGS += $(CFLAGS_libxenlight)
testenum.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight)
-testenum.c: libxl.idl gentest.py
- $(PYTHON) gentest.py libxl.idl testenum.c.new
+testenum.c: libxl_types.idl gentest.py
+ $(PYTHON) gentest.py libxl_types.idl testenum.c.new
mv testenum.c.new testenum.c
.PHONY: all
@@ -79,13 +79,15 @@ _libxl_paths.h: genpath
libxl_paths.c: _libxl_paths.h
libxl.h: _libxl_types.h
+libxl_internal.h: _libxl_types_internal.h
$(LIBXL_OBJS) $(LIBXLU_OBJS) $(XL_OBJS): libxl.h
+$(LIBXL_OBJS): libxl_internal.h
-_libxl_%.h _libxl_%.c: libxl.idl gen%.py libxl%.py
- $(PYTHON) gen$*.py libxl.idl __libxl_$*.h __libxl_$*.c
- mv __libxl_$*.h _libxl_$*.h
- mv __libxl_$*.c _libxl_$*.c
+_libxl_type%.h _libxl_type%.c: libxl_type%.idl gentypes.py libxltypes.py
+ $(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*.c
+ mv __libxl_type$*.h _libxl_type$*.h
+ mv __libxl_type$*.c _libxl_type$*.c
libxenlight.so: libxenlight.so.$(MAJOR)
ln -sf $< $@
diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py
index c9a3d9c..5188b66 100644
--- a/tools/libxl/gentypes.py
+++ b/tools/libxl/gentypes.py
@@ -162,9 +162,10 @@ if __name__ == ''__main__'':
print "outputting libxl type definitions to %s" % header
f = open(header, "w")
-
- f.write("""#ifndef __LIBXL_TYPES_H
-#define __LIBXL_TYPES_H
+
+ header_define =
header.upper().replace(''.'',''_'')
+ f.write("""#ifndef %s
+#define %s
/*
* DO NOT EDIT.
@@ -172,9 +173,9 @@ if __name__ == ''__main__'':
* This file is autogenerated by
* "%s"
*/
-
-""" % " ".join(sys.argv))
-
+
+""" % (header_define, header_define, "
".join(sys.argv)))
+
for ty in types:
f.write(libxl_C_type_define(ty) + ";\n")
if ty.destructor_fn is not None:
@@ -185,7 +186,7 @@ if __name__ == ''__main__'':
f.write("extern libxl_enum_string_table
%s_string_table[];\n" % (ty.typename))
f.write("\n")
- f.write("""#endif /* __LIBXL_TYPES_H */\n""")
+ f.write("""#endif /* %s */\n""" %
(header_define))
f.close()
impl = sys.argv[3]
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 47752e5..71eb189 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -35,6 +35,7 @@
#include "flexarray.h"
#include "libxl_utils.h"
+#include "_libxl_types_internal.h"
#define LIBXL_DESTROY_TIMEOUT 10
#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
diff --git a/tools/libxl/libxl.idl b/tools/libxl/libxl_types.idl
similarity index 100%
rename from tools/libxl/libxl.idl
rename to tools/libxl/libxl_types.idl
diff --git a/tools/libxl/libxl_types_internal.idl
b/tools/libxl/libxl_types_internal.idl
new file mode 100644
index 0000000..d993298
--- /dev/null
+++ b/tools/libxl/libxl_types_internal.idl
@@ -0,0 +1,10 @@
+
+libxl__qmp_message_type = Enumeration("qmp_message_type", [
+ (1, "QMP"),
+ (2, "return"),
+ (3, "error"),
+ (4, "event"),
+ (5, "invalid"),
+ ],
+ namespace = "libxl__")
+
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jun-30 17:30 UTC
[Xen-devel] [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
We use the yajl parser, but we need to make a tree from the parse result
to use it outside the parser.
So this patch include json_object struct that is used to hold the JSON
data.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/Makefile | 5 +-
tools/libxl/libxl_internal.h | 97 ++++++++
tools/libxl/libxl_json.c | 521 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 622 insertions(+), 1 deletions(-)
create mode 100644 tools/libxl/libxl_json.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index a95cd5d..0306cb0 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -32,9 +32,12 @@ endif
LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o
LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o
+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_OBJS-y)
+ libxl_internal.o libxl_utils.o libxl_uuid.o libxl_json.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_internal.h b/tools/libxl/libxl_internal.h
index 71eb189..555d9f3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -381,4 +381,101 @@ _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_json */
+typedef enum {
+ JSON_ERROR,
+ JSON_NULL,
+ JSON_TRUE,
+ JSON_FALSE,
+ JSON_INTEGER,
+ JSON_DOUBLE,
+ JSON_STRING,
+ JSON_MAP,
+ JSON_ARRAY,
+ JSON_ANY
+} libxl__json_node_type;
+
+typedef struct libxl__json_object {
+ libxl__json_node_type type;
+ union {
+ long i;
+ double d;
+ const char *string;
+ /* List of libxl__json_object */
+ flexarray_t *array;
+ /* List of libxl__json_map_node */
+ flexarray_t *map;
+ } u;
+ struct libxl__json_object *parent;
+} libxl__json_object;
+
+typedef struct {
+ const char *map_key;
+ libxl__json_object *obj;
+} libxl__json_map_node;
+
+typedef struct libxl__yajl_ctx libxl__yajl_ctx;
+
+static inline bool json_object_is_string(const libxl__json_object *o)
+{
+ return o != NULL && o->type == JSON_STRING;
+}
+static inline bool json_object_is_integer(const libxl__json_object *o)
+{
+ return o != NULL && o->type == JSON_INTEGER;
+}
+static inline bool json_object_is_map(const libxl__json_object *o)
+{
+ return o != NULL && o->type == JSON_MAP;
+}
+static inline bool json_object_is_array(const libxl__json_object *o)
+{
+ return o != NULL && o->type == JSON_ARRAY;
+}
+
+static inline const char *json_object_get_string(const libxl__json_object *o)
+{
+ if (json_object_is_string(o))
+ return o->u.string;
+ else
+ return NULL;
+}
+static inline flexarray_t *json_object_get_map(const libxl__json_object *o)
+{
+ if (json_object_is_map(o))
+ return o->u.map;
+ else
+ return NULL;
+}
+static inline flexarray_t *json_object_get_array(const libxl__json_object *o)
+{
+ if (json_object_is_array(o))
+ return o->u.array;
+ else
+ return NULL;
+}
+static inline long json_object_get_integer(const libxl__json_object *o)
+{
+ if (json_object_is_integer(o))
+ return o->u.i;
+ else
+ return -1;
+}
+
+_hidden const libxl__json_object *json_object_get(const char *key,
+ const libxl__json_object *o,
+ libxl__json_node_type expected_type);
+_hidden void json_object_free(libxl_ctx *ctx, libxl__json_object *obj);
+
+/* s: is the buffer to parse, libxl__json_parse will advance the pointer the
+ * part that has not been parsed
+ * *yajl_ctx: is set if the buffer have been whole consume, but the JSON
+ * structure is not complete.
+ * return NULL in case of error or when the JSON structure is not complete.
+ */
+_hidden libxl__json_object *libxl__json_parse(libxl_ctx *ctx,
+ libxl__yajl_ctx **yajl_ctx,
+ const unsigned char **s,
+ ssize_t len);
+
#endif
diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
new file mode 100644
index 0000000..ff9a176
--- /dev/null
+++ b/tools/libxl/libxl_json.c
@@ -0,0 +1,521 @@
+/*
+ * Copyright (C) 2011 Citrix Ltd.
+ *
+ * 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.
+ */
+
+#include <string.h>
+
+#include <yajl/yajl_parse.h>
+#include <yajl/yajl_gen.h>
+
+#include "libxl_internal.h"
+
+#define DEBUG_ANSWER
+
+struct libxl__yajl_ctx {
+ libxl_ctx *ctx;
+ yajl_handle hand;
+ libxl__json_object *head;
+ libxl__json_object *current;
+#ifdef DEBUG_ANSWER
+ yajl_gen g;
+#endif
+};
+
+#ifdef DEBUG_ANSWER
+# define DEBUG_GEN_ALLOC(h) \
+ if (h == NULL) { \
+ yajl_gen_config conf = { 1, " " }; \
+ h = yajl_gen_alloc(&conf, NULL); \
+ }
+# define DEBUG_GEN_FREE(h) if (h) yajl_gen_free(h)
+# define DEBUG_GEN(h, type) yajl_gen_##type(h)
+# define DEBUG_GEN_VALUE(h, type, value) yajl_gen_##type(h, value)
+# define DEBUG_GEN_STRING(h, str, n) yajl_gen_string(h, str, n)
+# define DEBUG_GEN_REPORT(h, ctx) \
+ do { \
+ const unsigned char *buf = NULL; \
+ unsigned int len = 0; \
+ yajl_gen_get_buf(h, &buf, &len); \
+ LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "response:\n%s", buf); \
+ yajl_gen_free(h); \
+ h = NULL; \
+ } while (0)
+#else
+# define DEBUG_GEN_ALLOC(h) ((void)0)
+# define DEBUG_GEN_FREE(h) ((void)0)
+# define DEBUG_GEN(h, type) ((void)0)
+# define DEBUG_GEN_VALUE(h, type, value) ((void)0)
+# define DEBUG_GEN_STRING(h, value, lenght) ((void)0)
+# define DEBUG_GEN_REPORT(h, ctx) ((void)0)
+#endif
+
+/*
+ * libxl__json_object helper functions
+ */
+
+static libxl__json_object *json_object_alloc(libxl_ctx *ctx,
+ libxl__json_node_type type)
+{
+ libxl__json_object *obj;
+
+ obj = calloc(1, sizeof (libxl__json_object));
+ if (obj == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Failed to allocate a libxl__json_object");
+ return NULL;
+ }
+
+ obj->type = type;
+
+ if (type == JSON_MAP || type == JSON_ARRAY) {
+ flexarray_t *array = flexarray_make(1, 1);
+ if (array == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Failed to allocate a flexarray");
+ free(obj);
+ return NULL;
+ }
+ if (type == JSON_MAP)
+ obj->u.map = array;
+ else
+ obj->u.array = array;
+ }
+
+ return obj;
+}
+
+static int json_object_append_to(libxl_ctx *ctx,
+ libxl__json_object *obj,
+ libxl__json_object *dst)
+{
+ if (!dst) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "No parent json object to fill");
+ return -1;
+ }
+
+ switch (dst->type) {
+ case JSON_MAP: {
+ libxl__json_map_node *last;
+
+ if (dst->u.map->count == 0) {
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Try to add a value to an empty map (with no
key)");
+ return -1;
+ }
+ flexarray_get(dst->u.map, dst->u.map->count - 1,
(void**)&last);
+ last->obj = obj;
+ break;
+ }
+ case JSON_ARRAY:
+ if (flexarray_append(dst->u.array, obj) == 2) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Failed to grow a flexarray");
+ return -1;
+ }
+ break;
+ default:
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
+ "Try append an object is not a map/array (%i)\n",
+ dst->type);
+ return -1;
+ }
+
+ obj->parent = dst;
+ return 0;
+}
+
+void json_object_free(libxl_ctx *ctx, libxl__json_object *obj)
+{
+ int index = 0;
+
+ if (obj == NULL)
+ return;
+ switch (obj->type) {
+ case JSON_STRING:
+ free((void*)obj->u.string);
+ break;
+ case JSON_MAP: {
+ libxl__json_map_node *node = NULL;
+
+ for (index = 0; index < obj->u.map->count; index++) {
+ if (flexarray_get(obj->u.map, index, (void**)&node) != 0)
+ break;
+ json_object_free(ctx, node->obj);
+ free((void*)node->map_key);
+ free(node);
+ node = NULL;
+ }
+ flexarray_free(obj->u.map);
+ break;
+ }
+ case JSON_ARRAY: {
+ libxl__json_object *node = NULL;
+ break;
+
+ for (index = 0; index < obj->u.array->count; index++) {
+ if (flexarray_get(obj->u.array, index, (void**)&node) != 0)
+ break;
+ json_object_free(ctx, node);
+ node = NULL;
+ }
+ flexarray_free(obj->u.array);
+ break;
+ }
+ default:
+ break;
+ }
+ free(obj);
+}
+
+const libxl__json_object *json_object_get(const char *key,
+ const libxl__json_object *o,
+ libxl__json_node_type expected_type)
+{
+ flexarray_t *maps = NULL;
+ int index = 0;
+
+ if (json_object_is_map(o)) {
+ libxl__json_map_node *node = NULL;
+
+ maps = o->u.map;
+ for (index = 0; index < maps->count; index++) {
+ if (flexarray_get(maps, index, (void**)&node) != 0)
+ return NULL;
+ if (strcmp(key, node->map_key) == 0) {
+ if (expected_type == JSON_ANY
+ || (node->obj && node->obj->type ==
expected_type)) {
+ return node->obj;
+ } else {
+ return NULL;
+ }
+ }
+ }
+ }
+ return NULL;
+}
+
+
+/*
+ * JSON callbacks
+ */
+
+static int json_callback_null(void *opaque)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ libxl__json_object *obj;
+
+ DEBUG_GEN(ctx->g, null);
+
+ if ((obj = json_object_alloc(ctx->ctx, JSON_NULL)) == NULL)
+ return 0;
+
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_boolean(void *opaque, int boolean)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ libxl__json_object *obj;
+
+ DEBUG_GEN_VALUE(ctx->g, bool, boolean);
+
+ if ((obj = json_object_alloc(ctx->ctx,
+ boolean ? JSON_TRUE : JSON_FALSE)) == NULL)
+ return 0;
+
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_integer(void *opaque, long value)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ libxl__json_object *obj;
+
+ DEBUG_GEN_VALUE(ctx->g, integer, value);
+
+ if ((obj = json_object_alloc(ctx->ctx, JSON_INTEGER)) == NULL)
+ return 0;
+ obj->u.i = value;
+
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_double(void *opaque, double value)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ libxl__json_object *obj;
+
+ DEBUG_GEN_VALUE(ctx->g, double, value);
+
+ if ((obj = json_object_alloc(ctx->ctx, JSON_DOUBLE)) == NULL)
+ return 0;
+ obj->u.d = value;
+
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_string(void *opaque, const unsigned char *str,
+ unsigned int len)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ char *t = malloc(len + 1);
+ libxl__json_object *obj = NULL;
+
+ if (t == NULL) {
+ LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR, "Failed to
allocate");
+ return 0;
+ }
+
+ DEBUG_GEN_STRING(ctx->g, str, len);
+
+ strncpy(t, (const char *) str, len);
+ t[len] = 0;
+
+ if ((obj = json_object_alloc(ctx->ctx, JSON_STRING)) == NULL) {
+ free(t);
+ return 0;
+ }
+ obj->u.string = t;
+
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_map_key(void *opaque, const unsigned char *str,
+ unsigned int len)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ char *t = malloc(len + 1);
+ libxl__json_object *obj = ctx->current;
+
+ if (t == NULL) {
+ LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR, "Failed to
allocate");
+ return 0;
+ }
+
+ DEBUG_GEN_STRING(ctx->g, str, len);
+
+ strncpy(t, (const char *) str, len);
+ t[len] = 0;
+
+ if (json_object_is_map(obj)) {
+ libxl__json_map_node *node = malloc(sizeof (libxl__json_map_node));
+ if (node == NULL) {
+ LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR,
+ "Failed to allocate");
+ return 0;
+ }
+
+ node->map_key = t;
+ node->obj = NULL;
+
+ if (flexarray_append(obj->u.map, node) == 2) {
+ LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR,
+ "Failed to grow a flexarray");
+ return 0;
+ }
+ } else {
+ LIBXL__LOG(ctx->ctx, LIBXL__LOG_ERROR,
+ "Current json object is not a map");
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_start_map(void *opaque)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ libxl__json_object *obj = NULL;
+
+ DEBUG_GEN(ctx->g, map_open);
+
+ if ((obj = json_object_alloc(ctx->ctx, JSON_MAP)) == NULL)
+ return 0;
+
+ if (ctx->current) {
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+ }
+
+ ctx->current = obj;
+ if (ctx->head == NULL) {
+ ctx->head = obj;
+ }
+
+ return 1;
+}
+
+static int json_callback_end_map(void *opaque)
+{
+ libxl__yajl_ctx *ctx = opaque;
+
+ DEBUG_GEN(ctx->g, map_close);
+
+ if (ctx->current) {
+ ctx->current = ctx->current->parent;
+ } else {
+ LIBXL__LOG(ctx->ctx, LIBXL__LOG_ERROR,
+ "No current libxl__json_object, cannot use his
parent.");
+ return 0;
+ }
+
+ return 1;
+}
+
+static int json_callback_start_array(void *opaque)
+{
+ libxl__yajl_ctx *ctx = opaque;
+ libxl__json_object *obj = NULL;
+
+ DEBUG_GEN(ctx->g, array_open);
+
+ if ((obj = json_object_alloc(ctx->ctx, JSON_ARRAY)) == NULL)
+ return 0;
+
+ if (ctx->current) {
+ if (json_object_append_to(ctx->ctx, obj, ctx->current) == -1) {
+ json_object_free(ctx->ctx, obj);
+ return 0;
+ }
+ }
+
+ ctx->current = obj;
+ if (ctx->head == NULL) {
+ ctx->head = obj;
+ }
+
+ return 1;
+}
+
+static int json_callback_end_array(void *opaque)
+{
+ libxl__yajl_ctx *ctx = opaque;
+
+ DEBUG_GEN(ctx->g, array_close);
+
+ if (ctx->current) {
+ ctx->current = ctx->current->parent;
+ } else {
+ LIBXL__LOG(ctx->ctx, LIBXL__LOG_ERROR,
+ "No current libxl__json_object, cannot use his
parent.");
+ return 0;
+ }
+
+ return 1;
+}
+
+static yajl_callbacks callbacks = {
+ json_callback_null,
+ json_callback_boolean,
+ json_callback_integer,
+ json_callback_double,
+ NULL,
+ json_callback_string,
+ json_callback_start_map,
+ json_callback_map_key,
+ json_callback_end_map,
+ json_callback_start_array,
+ json_callback_end_array
+};
+
+static void yajl_ctx_free(libxl__yajl_ctx *yajl_ctx)
+{
+ if (yajl_ctx->hand)
+ yajl_free(yajl_ctx->hand);
+ DEBUG_GEN_FREE(yajl_ctx->g);
+}
+
+libxl__json_object *libxl__json_parse(libxl_ctx *ctx,
+ libxl__yajl_ctx **yajl_ctx_p,
+ const unsigned char **s,
+ ssize_t len)
+{
+ yajl_status status;
+ const unsigned char *bak_s = *s;
+ libxl__yajl_ctx *yajl_ctx = *yajl_ctx_p;
+
+ if (yajl_ctx == NULL) {
+ yajl_ctx = calloc(1, sizeof (libxl__yajl_ctx));
+ if (yajl_ctx == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Failed to allocate the parser
context");
+ return NULL;
+ }
+ yajl_ctx->ctx = ctx;
+ }
+
+ DEBUG_GEN_ALLOC(yajl_ctx->g);
+ /* parse the input */
+ if (yajl_ctx->hand == NULL) {
+ /* allow comments */
+ yajl_parser_config cfg = { 1, 1 };
+ yajl_ctx->hand = yajl_alloc(&callbacks, &cfg, NULL,
yajl_ctx);
+ }
+ status = yajl_parse(yajl_ctx->hand, *s, len);
+ *s += yajl_get_bytes_consumed(yajl_ctx->hand);
+
+ if (status != yajl_status_ok
+ && status != yajl_status_insufficient_data) {
+ unsigned char *str = yajl_get_error(yajl_ctx->hand, 1, bak_s, len);
+
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "yajl error: %s", str);
+ yajl_free_error(yajl_ctx->hand, str);
+
+ json_object_free(ctx, yajl_ctx->head);
+ yajl_ctx_free(yajl_ctx);
+ *yajl_ctx_p = NULL;
+ return NULL;
+ }
+
+ if (status == yajl_status_ok) {
+ libxl__json_object *o = yajl_ctx->head;
+
+ DEBUG_GEN_REPORT(yajl_ctx->g, ctx);
+
+ yajl_ctx->head = NULL;
+
+ yajl_ctx_free(yajl_ctx);
+ *yajl_ctx_p = NULL;
+ return o;
+ }
+ *yajl_ctx_p = yajl_ctx;
+ return NULL;
+}
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jun-30 17:30 UTC
[Xen-devel] [PATCH V6 3/3] libxl, Introduce a QMP client
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).
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
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@citrix.com>
+ *
+ * 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_handler *qmp = NULL;
+
+ qmp = calloc(1, sizeof (libxl__qmp_handler));
+ if (qmp == NULL) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Failed to allocate qmp_handler");
+ return NULL;
+ }
+ qmp->ctx = ctx;
+ qmp->domid = domid;
+ qmp->timeout = 5;
+
+ SIMPLEQ_INIT(&qmp->callback_list);
+
+ return qmp;
+}
+
+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));
+
+ 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;
+ }
+ if (tmp)
+ free(tmp);
+}
+
+static int qmp_next(libxl__qmp_handler *qmp)
+{
+ ssize_t rd;
+ const unsigned char *s = NULL;
+ const unsigned char *s_end = NULL;
+
+ /* read the socket */
+ while (1) {
+ fd_set rfds;
+ int ret = 0;
+ struct timeval timeout = {
+ .tv_sec = qmp->timeout,
+ .tv_usec = 0,
+ };
+
+ FD_ZERO(&rfds);
+ FD_SET(qmp->qmp_fd, &rfds);
+
+ ret = select(qmp->qmp_fd + 1, &rfds, NULL, NULL, &timeout);
+ if (ret > 0) {
+ rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE);
+ if (rd > 0) {
+ break;
+ } else if (rd < 0) {
+ LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
+ "Socket read error");
+ return rd;
+ }
+ } else if (ret == 0) {
+ LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR, "timeout");
+ return -1;
+ } else if (ret < 0) {
+ LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR, "Select
error");
+ return -1;
+ }
+ }
+ DEBUG_REPORT_RECEIVED(rd, qmp->buffer);
+
+ s = qmp->buffer;
+ s_end = qmp->buffer + rd;
+ while (s < s_end) {
+ libxl__json_object *o = libxl__json_parse(qmp->ctx,
&qmp->yajl_ctx,
+ &s, s_end - s);
+ if (o) {
+ qmp_handle_response(qmp, o);
+ json_object_free(qmp->ctx, o);
+ }
+
+ /* skip the CRLF of the end of a command, this avoid doing an extra
+ * turn just for them */
+ while (s < s_end && (*s == ''\r'' || *s ==
''\n'')) {
+ s++;
+ }
+ }
+ return 1;
+}
+
+static int qmp_send(libxl__qmp_handler *qmp,
+ const char *cmd, qmp_callback_t callback)
+{
+ yajl_gen_config conf = { 0, NULL };
+ const unsigned char *buf;
+ unsigned int len = 0;
+ yajl_gen_status s;
+ yajl_gen hand;
+
+ hand = yajl_gen_alloc(&conf, NULL);
+ if (!hand) {
+ return -1;
+ }
+
+ yajl_gen_map_open(hand);
+ libxl__yajl_gen_asciiz(hand, "execute");
+ libxl__yajl_gen_asciiz(hand, cmd);
+ libxl__yajl_gen_asciiz(hand, "id");
+ yajl_gen_integer(hand, ++qmp->last_id_used);
+ yajl_gen_map_close(hand);
+
+ s = yajl_gen_get_buf(hand, &buf, &len);
+
+ if (s) {
+ LIBXL__LOG(qmp->ctx, LIBXL__LOG_ERROR,
+ "Failed to generate a qmp command");
+ return -1;
+ }
+
+ if (callback) {
+ callback_id_pair *elm = malloc(sizeof (callback_id_pair));
+ if (elm == NULL) {
+ LIBXL__LOG_ERRNO(qmp->ctx, LIBXL__LOG_ERROR,
+ "Failed to allocate a QMP callback");
+ yajl_gen_free(hand);
+ return -1;
+ }
+ elm->id = qmp->last_id_used;
+ elm->callback = callback;
+ SIMPLEQ_INSERT_TAIL(&qmp->callback_list, elm, next);
+ }
+
+ LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "next qmp command:
''%s''", buf);
+
+ if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, buf, len,
+ "QMP command", "QMP socket"))
+ goto error;
+ if (libxl_write_exactly(qmp->ctx, qmp->qmp_fd, "\r\n", 2,
+ "CRLF", "QMP socket"))
+ goto error;
+
+ yajl_gen_free(hand);
+
+ return qmp->last_id_used;
+
+error:
+ yajl_gen_free(hand);
+ return -1;
+}
+
+static int qmp_synchronous_send(libxl__qmp_handler *qmp, const char *cmd,
+ qmp_callback_t callback, int ask_timeout)
+{
+ int id = 0;
+ int ret = 0;
+
+ id = qmp_send(qmp, cmd, callback);
+ if (id <= 0) {
+ return -1;
+ }
+ qmp->wait_for_id = id;
+
+ while (qmp->wait_for_id == id) {
+ if ((ret = qmp_next(qmp)) < 0) {
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static void qmp_free_handler(libxl__qmp_handler *qmp)
+{
+ free(qmp);
+}
+
+/*
+ * API
+ */
+
+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;
+ }
+ }
+
+ libxl__free_all(&gc);
+ return qmp;
+}
+
+void libxl__qmp_close(libxl__qmp_handler *qmp)
+{
+ if (!qmp)
+ return;
+ qmp_close(qmp);
+ qmp_free_handler(qmp);
+}
+
+void libxl__qmp_cleanup(libxl__gc *gc, uint32_t domid)
+{
+ libxl_ctx *ctx = libxl__gc_owner(gc);
+ char *qmp_socket;
+
+ qmp_socket = libxl__sprintf(gc,
+ "%s/qmp-%d", libxl_run_dir_path(),
domid);
+ if (unlink(qmp_socket) == -1) {
+ if (errno != ENOENT) {
+ LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR,
+ "Failed to remove QMP socket file %s",
+ qmp_socket);
+ }
+ }
+}
+
+int libxl__qmp_query_serial(libxl__qmp_handler *qmp)
+{
+ return qmp_synchronous_send(qmp, "query-chardev",
+ register_serials_chardev_callback,
+ qmp->timeout);
+}
+
+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) {
+ ret = libxl__qmp_query_serial(qmp);
+ }
+ libxl__qmp_close(qmp);
+ return ret;
+}
--
1.7.2.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-01 08:03 UTC
[Xen-devel] Re: [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl.
On Thu, 2011-06-30 at 18:30 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > tools/libxl/Makefile | 16 +++++++++------- > tools/libxl/gentypes.py | 15 ++++++++------- > tools/libxl/libxl_internal.h | 1 + > tools/libxl/{libxl.idl => libxl_types.idl} | 0This is going to need changes to at least tools/python/Makefile and tools/ocaml/libs/xl/Makefile as well. Probably best to split the rename into another patch. [...]> diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl > new file mode 100644 > index 0000000..d993298 > --- /dev/null > +++ b/tools/libxl/libxl_types_internal.idl > @@ -0,0 +1,10 @@ > + > +libxl__qmp_message_type = Enumeration("qmp_message_type", [ > + (1, "QMP"), > + (2, "return"), > + (3, "error"), > + (4, "event"), > + (5, "invalid"), > + ], > + namespace = "libxl__")I wonder if the IDL infrastructure should export a way to set the default namespace? So we''d end up with: default_namespace = "libxl__" libxl__qmp_message_type = Enumeration("...", [....]) libxl__another_internal_type = Struct(...) We could use the same in libxl_types.idl and change the default to be no namespace, making the whole thing a tiny bit more generic. I expect that scoping will require the actual syntax to use the module e.g. libxltypes.default_namespace = "libxl__" or maybe a function call is cleaner libxltypes.default_namespace("libxl__") Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-01 08:14 UTC
[Xen-devel] Re: [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
On Thu, 2011-06-30 at 18:30 +0100, Anthony PERARD wrote:> We use the yajl parser, but we need to make a tree from the parse result > to use it outside the parser. > > So this patch include json_object struct that is used to hold the JSON > data. > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > tools/libxl/Makefile | 5 +- > tools/libxl/libxl_internal.h | 97 ++++++++ > tools/libxl/libxl_json.c | 521 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 622 insertions(+), 1 deletions(-) > create mode 100644 tools/libxl/libxl_json.c > > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile > index a95cd5d..0306cb0 100644 > --- a/tools/libxl/Makefile > +++ b/tools/libxl/Makefile > @@ -32,9 +32,12 @@ endif > LIBXL_OBJS-$(CONFIG_X86) += libxl_cpuid.o > LIBXL_OBJS-$(CONFIG_IA64) += libxl_nocpuid.o > > +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_OBJS-y) > + libxl_internal.o libxl_utils.o libxl_uuid.o libxl_json.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_internal.h b/tools/libxl/libxl_internal.h > index 71eb189..555d9f3 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -381,4 +381,101 @@ _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_json */ > +typedef enum { > + JSON_ERROR, > + JSON_NULL, > + JSON_TRUE, > + JSON_FALSE, > + JSON_INTEGER, > + JSON_DOUBLE, > + JSON_STRING, > + JSON_MAP, > + JSON_ARRAY, > + JSON_ANY > +} libxl__json_node_type;Could be internal IDL? Maybe no much point if you aren''t using the to_string functions etc?> + > +typedef struct libxl__json_object { > + libxl__json_node_type type; > + union { > + long i; > + double d; > + const char *string;Is it really const? Seems to be malloc/freed?> [...]+#ifdef DEBUG_ANSWER > +# define DEBUG_GEN_ALLOC(h) \ > + if (h == NULL) { \ > + yajl_gen_config conf = { 1, " " }; \ > + h = yajl_gen_alloc(&conf, NULL); \ > + }All the callers of these macros use ctx->g. I think you could push the ->g down into the macros. [...] Ian _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-01 08:39 UTC
[Xen-devel] Re: [PATCH V6 3/3] libxl, Introduce a QMP client
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@citrix.com> > --- > 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@citrix.com> > + * > + * 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@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-01 10:16 UTC
Re: [Xen-devel] Re: [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
On Fri, Jul 1, 2011 at 09:14, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:>> +/* from libxl_json */ >> +typedef enum { >> + JSON_ERROR, >> + JSON_NULL, >> + JSON_TRUE, >> + JSON_FALSE, >> + JSON_INTEGER, >> + JSON_DOUBLE, >> + JSON_STRING, >> + JSON_MAP, >> + JSON_ARRAY, >> + JSON_ANY >> +} libxl__json_node_type; > > Could be internal IDL? Maybe no much point if you aren''t using the > to_string functions etc?The to_string function could be use for debug messages. But that all.>> + >> +typedef struct libxl__json_object { >> + libxl__json_node_type type; >> + union { >> + long i; >> + double d; >> + const char *string; > > Is it really const? Seems to be malloc/freed?Yes, the string belong only to this structure. In that case, there is no const ?>> [...]+#ifdef DEBUG_ANSWER >> +# define DEBUG_GEN_ALLOC(h) \ >> + if (h == NULL) { \ >> + yajl_gen_config conf = { 1, " " }; \ >> + h = yajl_gen_alloc(&conf, NULL); \ >> + } > > All the callers of these macros use ctx->g. I think you could push the > ->g down into the macros.Yes, maybe better, specially for the case of the _alloc where the parameter is modified. Thanks, -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-01 10:22 UTC
Re: [Xen-devel] Re: [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl.
On Fri, Jul 1, 2011 at 09:03, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:> On Thu, 2011-06-30 at 18:30 +0100, Anthony PERARD wrote: >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> tools/libxl/Makefile | 16 +++++++++------- >> tools/libxl/gentypes.py | 15 ++++++++------- >> tools/libxl/libxl_internal.h | 1 + >> tools/libxl/{libxl.idl => libxl_types.idl} | 0 > > This is going to need changes to at least tools/python/Makefile and > tools/ocaml/libs/xl/Makefile as well. Probably best to split the rename > into another patch.:(, this file is use everywhere. So yes, I will split the patch.> [...] >> diff --git a/tools/libxl/libxl_types_internal.idl b/tools/libxl/libxl_types_internal.idl >> new file mode 100644 >> index 0000000..d993298 >> --- /dev/null >> +++ b/tools/libxl/libxl_types_internal.idl >> @@ -0,0 +1,10 @@ >> + >> +libxl__qmp_message_type = Enumeration("qmp_message_type", [ >> + (1, "QMP"), >> + (2, "return"), >> + (3, "error"), >> + (4, "event"), >> + (5, "invalid"), >> + ], >> + namespace = "libxl__") > > I wonder if the IDL infrastructure should export a way to set the > default namespace? So we''d end up with: > default_namespace = "libxl__" > > libxl__qmp_message_type = Enumeration("...", [....]) > libxl__another_internal_type = Struct(...) > > We could use the same in libxl_types.idl and change the default to be no > namespace, making the whole thing a tiny bit more generic. > > I expect that scoping will require the actual syntax to use the module > e.g. > libxltypes.default_namespace = "libxl__" > or maybe a function call is cleaner > libxltypes.default_namespace("libxl__")I will try to do that. The .idl files will be much more cleanner after that. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-01 15:04 UTC
Re: [Xen-devel] Re: [PATCH V6 3/3] libxl, Introduce a QMP client
On Fri, Jul 1, 2011 at 09:39, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:> 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?Well, this socket belong to QEMU (the creator). So I''m not sure that "libxl" in the name is appropriate. But did you prefer to see "qmp-libxl-$(domid)" as a socket file name ? [...]>> +/* >> + * Handler functions >> + */ >> + >> +static libxl__qmp_handler *qmp_init_handler(libxl_ctx *ctx, uint32_t domid) > > libxl__qmp_init_handler?"libxl__" event if it''s intern to this file?> 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?So, I should use the garbage collector for the handler, but also for the callback ? For the callback, I know when I can free them because they will not be used anymore. But if I use the gc, the callbacks will not be freed before the dustman is called by the caller.> 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).Well, a json_string should be strdup because, with or without gc, everything malloc for a json_object will be free with this object. I will try to use the gc with the json parsing stuff.>> +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?I think it''s should be a ETIMEDOUT. ENOENT or ECONNREFUSED explain just why we timeout.>> + >> + 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.Yes.>> +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...That meen that QEMU did not respond to the "qmp_capabilities" command. And it''s needed in order send other commands. So yes, it''s an error. I will return NULL.> [...] >> +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; >> +}-- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-01 15:16 UTC
Re: [Xen-devel] Re: [PATCH V6 3/3] libxl, Introduce a QMP client
On Fri, 2011-07-01 at 16:04 +0100, Anthony PERARD wrote:> On Fri, Jul 1, 2011 at 09:39, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote: > > 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? > > Well, this socket belong to QEMU (the creator). So I''m not sure that > "libxl" in the name is appropriate.It''s created due to libxl asking it too was how I was thinking about it.> But did you prefer to see "qmp-libxl-$(domid)" as a socket file name ?Something like that, yeah.> [...] > > >> +/* > >> + * Handler functions > >> + */ > >> + > >> +static libxl__qmp_handler *qmp_init_handler(libxl_ctx *ctx, uint32_t domid) > > > > libxl__qmp_init_handler? > > "libxl__" event if it''s intern to this file?I guess it''s not required, I''m just used to seeing it. The use of libxl__gc for internal functions is more important.> > 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? > > So, I should use the garbage collector for the handler, but also for > the callback ? For the callback, I know when I can free them because > they will not be used anymore. But if I use the gc, the callbacks will > not be freed before the dustman is called by the caller.I don''t think using the gc infrastructure should be mandatory, just do what leads to the cleanest code IMHO.> > 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). > > Well, a json_string should be strdup because, with or without gc, > everything malloc for a json_object will be free with this object. > > I will try to use the gc with the json parsing stuff. > > >> +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? > > I think it''s should be a ETIMEDOUT. ENOENT or ECONNREFUSED explain > just why we timeout. > > >> + > >> + 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. > > Yes. > > >> +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... > > That meen that QEMU did not respond to the "qmp_capabilities" command. > And it''s needed in order send other commands. So yes, it''s an error. I > will return NULL. > > > [...] > >> +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; > >> +} >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jul-05 17:21 UTC
Re: [Xen-devel] [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
Anthony PERARD writes ("[Xen-devel] [PATCH V6 2/3] libxl: Introduce JSON
parsing stuff."):> +static int json_object_append_to(libxl_ctx *ctx,
> + libxl__json_object *obj,
> + libxl__json_object *dst)
> +{
> + if (!dst) {
> + LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
> + "No parent json object to fill");
> + return -1;
> + }
Surely it is fair enough for this function to dereference NULL (and
segfault) if dst==0 ? Returning -1 (ie, operational error similar to
memory allocation failure) for a programming mistake like that is not
a good idea IMO.
> +static int json_callback_string(void *opaque, const unsigned char *str,
> + unsigned int len)
> +{
> + libxl__yajl_ctx *ctx = opaque;
> + char *t = malloc(len + 1);
> + libxl__json_object *obj = NULL;
> +
> + if (t == NULL) {
> + LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR, "Failed to
allocate");
> + return 0;
> + }
This separation of the allocation from the test is pretty nasty.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Ian Jackson
2011-Jul-05 17:32 UTC
Re: [Xen-devel] [PATCH V6 3/3] libxl, Introduce a QMP client
Anthony PERARD writes ("[Xen-devel] [PATCH V6 3/3] libxl, Introduce a QMP
client"):> QMP stands for QEMU Monitor Protocol and it is used to query information
> from QEMU or to control QEMU.
> if (dm_starting) {
> + if (dm_info->device_model_version ==
LIBXL_DEVICE_MODEL_VERSION_QEMU
_XEN) {> + libxl__qmp_initializations(ctx, domid);
> + }
...> + flexarray_append(dm_args,
> + libxl__sprintf(gc,
"socket,id=libxl-cmd,path=%s/qmp-%d,
server,nowait",> + libxl_run_dir_path(),
Lines too long, please wrap
> +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);
...> + 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);
I think this needs to call the callback, or the code which set up the
callback will surely just continue to wait forever.
...
Later: I see that you have a single wait_for_id. What if multiple
callers want to use a single qmp for multiple things ? Or is
"wait_for_id" simply the one that you''re waiting on
synchronously ?
> +static int qmp_next(libxl__qmp_handler *qmp)
> +{
...> + ret = select(qmp->qmp_fd + 1, &rfds, NULL, NULL,
&timeout);
> + if (ret > 0) {
> + rd = read(qmp->qmp_fd, qmp->buffer,
QMP_RECEIVE_BUFFER_SIZE);
> + if (rd > 0) {
> + break;
...> + s = qmp->buffer;
> + s_end = qmp->buffer + rd;
> + while (s < s_end) {
> + libxl__json_object *o = libxl__json_parse(qmp->ctx,
&qmp->yajl_ctx,
> + &s, s_end - s);
This assumes that the response will be received in a single read().
This is not correct. read() may return partial results; it may also
aggregate multiple writes from the sending qemu into a single read()
result.
You need to pull data into the buffer and then test the buffer for
completeness (eg by looking for the cr-lf), and split the buffer up
into packets yourself, and if there are partial packets left over
go round and read again.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-08 17:19 UTC
Re: [Xen-devel] [PATCH V6 3/3] libxl, Introduce a QMP client
On Tue, Jul 5, 2011 at 18:32, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:> >> +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); > ... >> + 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); > > I think this needs to call the callback, or the code which set up the > callback will surely just continue to wait forever.Ok, I will just call the callback with NULL, so, the callback will fail explicitly. That will not change anything for the only callback I have.> Later: I see that you have a single wait_for_id. What if multiple > callers want to use a single qmp for multiple things ? Or is > "wait_for_id" simply the one that you''re waiting on synchronously ?The second. Multiple callers can call qmp_send_synchronous multiple time.>> +static int qmp_next(libxl__qmp_handler *qmp) >> +{ > ... >> + ret = select(qmp->qmp_fd + 1, &rfds, NULL, NULL, &timeout); >> + if (ret > 0) { >> + rd = read(qmp->qmp_fd, qmp->buffer, QMP_RECEIVE_BUFFER_SIZE); >> + if (rd > 0) { >> + break; > ... >> + s = qmp->buffer; >> + s_end = qmp->buffer + rd; >> + while (s < s_end) { >> + libxl__json_object *o = libxl__json_parse(qmp->ctx, &qmp->yajl_ctx, >> + &s, s_end - s); > > This assumes that the response will be received in a single read(). > This is not correct. read() may return partial results; it may also > aggregate multiple writes from the sending qemu into a single read() > result.Actually, json_parse takes care of these. It tells if it''s a partiel result and how much of the buffer have been read. So the function assume that the caller will call qmp_next again if it''s needed (like in case a callback have not been called). I tried the function with a small buffer (not enough for an entire response) and it works fine.> You need to pull data into the buffer and then test the buffer for > completeness (eg by looking for the cr-lf), and split the buffer up > into packets yourself, and if there are partial packets left over > go round and read again.At least, I will check if the protocole is respected, and I will not relie anymore on json_parse to tell me the end of a packet. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel