* [PATCH V6 0/3] Introduce a QMP client
@ 2011-06-30 17:30 Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl Anthony PERARD
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Anthony PERARD @ 2011-06-30 17:30 UTC (permalink / raw)
To: Xen Devel; +Cc: Ian Campbell, Anthony PERARD, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl.
2011-06-30 17:30 [PATCH V6 0/3] Introduce a QMP client Anthony PERARD
@ 2011-06-30 17:30 ` Anthony PERARD
2011-07-01 8:03 ` Ian Campbell
2011-06-30 17:30 ` [PATCH V6 2/3] libxl: Introduce JSON parsing stuff Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 3/3] libxl, Introduce a QMP client Anthony PERARD
2 siblings, 1 reply; 14+ messages in thread
From: Anthony PERARD @ 2011-06-30 17:30 UTC (permalink / raw)
To: Xen Devel; +Cc: Ian Campbell, Anthony PERARD, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
2011-06-30 17:30 [PATCH V6 0/3] Introduce a QMP client Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl Anthony PERARD
@ 2011-06-30 17:30 ` Anthony PERARD
2011-07-01 8:14 ` Ian Campbell
2011-07-05 17:21 ` Ian Jackson
2011-06-30 17:30 ` [PATCH V6 3/3] libxl, Introduce a QMP client Anthony PERARD
2 siblings, 2 replies; 14+ messages in thread
From: Anthony PERARD @ 2011-06-30 17:30 UTC (permalink / raw)
To: Xen Devel; +Cc: Ian Campbell, Anthony PERARD, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH V6 3/3] libxl, Introduce a QMP client
2011-06-30 17:30 [PATCH V6 0/3] Introduce a QMP client Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 2/3] libxl: Introduce JSON parsing stuff Anthony PERARD
@ 2011-06-30 17:30 ` Anthony PERARD
2011-07-01 8:39 ` Ian Campbell
2011-07-05 17:32 ` Ian Jackson
2 siblings, 2 replies; 14+ messages in thread
From: Anthony PERARD @ 2011-06-30 17:30 UTC (permalink / raw)
To: Xen Devel; +Cc: Ian Campbell, Anthony PERARD, Stefano Stabellini
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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl.
2011-06-30 17:30 ` [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl Anthony PERARD
@ 2011-07-01 8:03 ` Ian Campbell
2011-07-01 10:22 ` Anthony PERARD
0 siblings, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2011-07-01 8:03 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Xen Devel, Stefano Stabellini
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.
[...]
> 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
2011-06-30 17:30 ` [PATCH V6 2/3] libxl: Introduce JSON parsing stuff Anthony PERARD
@ 2011-07-01 8:14 ` Ian Campbell
2011-07-01 10:16 ` Anthony PERARD
2011-07-05 17:21 ` Ian Jackson
1 sibling, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2011-07-01 8:14 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Xen Devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 3/3] libxl, Introduce a QMP client
2011-06-30 17:30 ` [PATCH V6 3/3] libxl, Introduce a QMP client Anthony PERARD
@ 2011-07-01 8:39 ` Ian Campbell
2011-07-01 15:04 ` Anthony PERARD
2011-07-05 17:32 ` Ian Jackson
1 sibling, 1 reply; 14+ messages in thread
From: Ian Campbell @ 2011-07-01 8:39 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Xen Devel, Stefano Stabellini
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
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
2011-07-01 8:14 ` Ian Campbell
@ 2011-07-01 10:16 ` Anthony PERARD
0 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2011-07-01 10:16 UTC (permalink / raw)
To: Ian Campbell; +Cc: Anthony PERARD, Xen Devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl.
2011-07-01 8:03 ` Ian Campbell
@ 2011-07-01 10:22 ` Anthony PERARD
0 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2011-07-01 10:22 UTC (permalink / raw)
To: Ian Campbell; +Cc: Anthony PERARD, Xen Devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH V6 3/3] libxl, Introduce a QMP client
2011-07-01 8:39 ` Ian Campbell
@ 2011-07-01 15:04 ` Anthony PERARD
2011-07-01 15:16 ` Ian Campbell
0 siblings, 1 reply; 14+ messages in thread
From: Anthony PERARD @ 2011-07-01 15:04 UTC (permalink / raw)
To: Ian Campbell; +Cc: Anthony PERARD, Xen Devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Re: [PATCH V6 3/3] libxl, Introduce a QMP client
2011-07-01 15:04 ` Anthony PERARD
@ 2011-07-01 15:16 ` Ian Campbell
0 siblings, 0 replies; 14+ messages in thread
From: Ian Campbell @ 2011-07-01 15:16 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Xen Devel, Stefano Stabellini
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;
> >> +}
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 2/3] libxl: Introduce JSON parsing stuff.
2011-06-30 17:30 ` [PATCH V6 2/3] libxl: Introduce JSON parsing stuff Anthony PERARD
2011-07-01 8:14 ` Ian Campbell
@ 2011-07-05 17:21 ` Ian Jackson
1 sibling, 0 replies; 14+ messages in thread
From: Ian Jackson @ 2011-07-05 17:21 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Ian Campbell, Xen Devel, Stefano Stabellini
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 3/3] libxl, Introduce a QMP client
2011-06-30 17:30 ` [PATCH V6 3/3] libxl, Introduce a QMP client Anthony PERARD
2011-07-01 8:39 ` Ian Campbell
@ 2011-07-05 17:32 ` Ian Jackson
2011-07-08 17:19 ` Anthony PERARD
1 sibling, 1 reply; 14+ messages in thread
From: Ian Jackson @ 2011-07-05 17:32 UTC (permalink / raw)
To: Anthony PERARD; +Cc: Ian Campbell, Xen Devel, Stefano Stabellini
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V6 3/3] libxl, Introduce a QMP client
2011-07-05 17:32 ` Ian Jackson
@ 2011-07-08 17:19 ` Anthony PERARD
0 siblings, 0 replies; 14+ messages in thread
From: Anthony PERARD @ 2011-07-08 17:19 UTC (permalink / raw)
To: Ian Jackson; +Cc: Ian Campbell, Xen Devel, Stefano Stabellini
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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2011-07-08 17:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-30 17:30 [PATCH V6 0/3] Introduce a QMP client Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 1/3] libxl: Introduce libxl_internal_types.idl Anthony PERARD
2011-07-01 8:03 ` Ian Campbell
2011-07-01 10:22 ` Anthony PERARD
2011-06-30 17:30 ` [PATCH V6 2/3] libxl: Introduce JSON parsing stuff Anthony PERARD
2011-07-01 8:14 ` Ian Campbell
2011-07-01 10:16 ` Anthony PERARD
2011-07-05 17:21 ` Ian Jackson
2011-06-30 17:30 ` [PATCH V6 3/3] libxl, Introduce a QMP client Anthony PERARD
2011-07-01 8:39 ` Ian Campbell
2011-07-01 15:04 ` Anthony PERARD
2011-07-01 15:16 ` Ian Campbell
2011-07-05 17:32 ` Ian Jackson
2011-07-08 17:19 ` Anthony PERARD
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.