New: - New patch that add set_default_namespaces to libxltypes.py, so we can set a namespace to use for a specific .idl. - New function: libxl__realloc and libxl__strndup. - The data is now pulled until it''s completed (with a CRLF). Now libxl__json_parse expect only a whole json object. - Use of libxl__gc in sevreal place. Anthony PERARD (7): libxl: Rename libxl.idl to libxl_types.idl. libxl: Add get/set_default_namespace in libxltypes.py. libxl: Introduce libxl_internal_types.idl. libxl, Introduce libxl__realloc. libxl, Intruduce libxl__strndup. 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 | 4 + tools/libxl/libxl_dm.c | 10 + tools/libxl/libxl_internal.c | 30 ++ tools/libxl/libxl_internal.h | 117 ++++++ tools/libxl/libxl_json.c | 505 +++++++++++++++++++++++ tools/libxl/libxl_qmp.c | 597 ++++++++++++++++++++++++++++ tools/libxl/{libxl.idl => libxl_types.idl} | 2 + tools/libxl/libxl_types_internal.idl | 9 + tools/libxl/libxltypes.py | 16 +- tools/ocaml/libs/xl/Makefile | 4 +- tools/python/Makefile | 4 +- 14 files changed, 1315 insertions(+), 21 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} (99%) create mode 100644 tools/libxl/libxl_types_internal.idl -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 1/7] libxl: Rename libxl.idl to libxl_types.idl.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/Makefile | 12 ++++++------ tools/libxl/{libxl.idl => libxl_types.idl} | 0 tools/ocaml/libs/xl/Makefile | 4 ++-- tools/python/Makefile | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) rename tools/libxl/{libxl.idl => libxl_types.idl} (100%) diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 3eeb26f..330ee6e 100644 --- a/tools/libxl/Makefile +++ b/tools/libxl/Makefile @@ -52,8 +52,8 @@ $(XL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h $(XL_OBJS): CFLAGS += $(CFLAGS_libxenlight) testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight) -testidl.c: libxl.idl gentest.py libxl.h - $(PYTHON) gentest.py libxl.idl testidl.c.new +testidl.c: libxl_types.idl gentest.py libxl.h + $(PYTHON) gentest.py libxl_types.idl testidl.c.new mv testidl.c.new testidl.c .PHONY: all @@ -84,10 +84,10 @@ libxl.h: _libxl_types.h $(LIBXL_OBJS) $(LIBXLU_OBJS) $(XL_OBJS): libxl.h -_libxl_%.h _libxl_%.c: libxl.idl gen%.py libxl%.py - $(PYTHON) gen$*.py libxl.idl __libxl_$*.h __libxl_$*.c - $(call move-if-changed,__libxl_$*.h,_libxl_$*.h) - $(call move-if-changed,__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 + $(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h) + $(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c) libxenlight.so: libxenlight.so.$(MAJOR) ln -sf $< $@ 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/ocaml/libs/xl/Makefile b/tools/ocaml/libs/xl/Makefile index 342dc35..a1e79a5 100644 --- a/tools/ocaml/libs/xl/Makefile +++ b/tools/ocaml/libs/xl/Makefile @@ -45,10 +45,10 @@ xl.mli: xl.mli.in _libxl_types.mli.in < xl.mli.in > xl.mli.tmp $(Q)mv xl.mli.tmp xl.mli -_libxl_types.ml.in _libxl_types.mli.in _libxl_types.inc: genwrap.py $(XEN_ROOT)/tools/libxl/libxl.idl \ +_libxl_types.ml.in _libxl_types.mli.in _libxl_types.inc: genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \ $(XEN_ROOT)/tools/libxl/libxltypes.py PYTHONPATH=$(XEN_ROOT)/tools/libxl $(PYTHON) genwrap.py \ - $(XEN_ROOT)/tools/libxl/libxl.idl \ + $(XEN_ROOT)/tools/libxl/libxl_types.idl \ _libxl_types.mli.in _libxl_types.ml.in _libxl_types.inc libs: $(LIBS) diff --git a/tools/python/Makefile b/tools/python/Makefile index de44178..b7bc501 100644 --- a/tools/python/Makefile +++ b/tools/python/Makefile @@ -10,10 +10,10 @@ genpath-target = $(call buildmakevars2file,$(XENPATH)) $(eval $(genpath-target)) .PHONY: build -build: genpath genwrap.py $(XEN_ROOT)/tools/libxl/libxl.idl \ +build: genpath genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \ $(XEN_ROOT)/tools/libxl/libxltypes.py PYTHONPATH=$(XEN_ROOT)/tools/libxl $(PYTHON) genwrap.py \ - $(XEN_ROOT)/tools/libxl/libxl.idl \ + $(XEN_ROOT)/tools/libxl/libxl_types.idl \ xen/lowlevel/xl/_pyxl_types.h \ xen/lowlevel/xl/_pyxl_types.c CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py build -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 2/7] libxl: Add get/set_default_namespace in libxltypes.py.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl_types.idl | 2 ++ tools/libxl/libxltypes.py | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index f7249b1..718688d 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -3,6 +3,8 @@ # Builtin libxl types # +set_default_namespace("libxl_") + libxl_domid = Builtin("domid") libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE) libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE) diff --git a/tools/libxl/libxltypes.py b/tools/libxl/libxltypes.py index b7b4669..64d2f50 100644 --- a/tools/libxl/libxltypes.py +++ b/tools/libxl/libxltypes.py @@ -8,10 +8,21 @@ DIR_IN = 1 DIR_OUT = 2 DIR_BOTH = 3 +default_namespace = "" +def set_default_namespace(s): + if type(s) != str: + raise TypeError, "Require a string for the default namespace." + global default_namespace + default_namespace = s + +def get_default_namespace(): + global default_namespace + return default_namespace + class Type(object): def __init__(self, typename, **kwargs): self.comment = kwargs.setdefault(''comment'', None) - self.namespace = kwargs.setdefault(''namespace'', "libxl_") + self.namespace = kwargs.setdefault(''namespace'', get_default_namespace()) self.dir = kwargs.setdefault(''dir'', DIR_BOTH) if self.dir not in [DIR_NONE, DIR_IN, DIR_OUT, DIR_BOTH]: raise ValueError @@ -248,7 +259,8 @@ def parse(f): elif isinstance(t,type(object)) and issubclass(t, Type): globs[n] = t elif n in [''PASS_BY_REFERENCE'', ''PASS_BY_VALUE'', - ''DIR_NONE'', ''DIR_IN'', ''DIR_OUT'', ''DIR_BOTH'']: + ''DIR_NONE'', ''DIR_IN'', ''DIR_OUT'', ''DIR_BOTH'', + ''set_default_namespace'']: globs[n] = t try: -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 3/7] libxl: Introduce libxl_internal_types.idl.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/Makefile | 4 +++- tools/libxl/gentypes.py | 15 ++++++++------- tools/libxl/libxl_internal.h | 1 + tools/libxl/libxl_types_internal.idl | 9 +++++++++ 4 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 tools/libxl/libxl_types_internal.idl diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 330ee6e..1f6b418 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) @@ -81,8 +81,10 @@ _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_type%.h _libxl_type%.c: libxl_type%.idl gentypes.py libxltypes.py $(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*.c diff --git a/tools/libxl/gentypes.py b/tools/libxl/gentypes.py index 120c6d3..e807f94 100644 --- a/tools/libxl/gentypes.py +++ b/tools/libxl/gentypes.py @@ -148,9 +148,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. @@ -158,9 +159,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: @@ -171,7 +172,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() print "outputting libxl type implementations to %s" % impl diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 579188e..4580be9 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_types_internal.idl b/tools/libxl/libxl_types_internal.idl new file mode 100644 index 0000000..0086bf0 --- /dev/null +++ b/tools/libxl/libxl_types_internal.idl @@ -0,0 +1,9 @@ +set_default_namespace("libxl__") + +libxl__qmp_message_type = Enumeration("qmp_message_type", [ + (1, "QMP"), + (2, "return"), + (3, "error"), + (4, "event"), + (5, "invalid"), + ]) -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 4/7] libxl, Introduce libxl__realloc.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl_internal.c | 20 ++++++++++++++++++++ tools/libxl/libxl_internal.h | 1 + 2 files changed, 21 insertions(+), 0 deletions(-) diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index e259278..527ebc5 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -102,6 +102,26 @@ void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size) return ptr; } +void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size) +{ + void *new_ptr = realloc(ptr, new_size); + int i = 0; + + if (ptr == NULL) { + libxl__ptr_add(gc, new_ptr); + } else if (new_ptr != ptr) { + for (i = 0; i < gc->alloc_maxsize; i++) { + if (gc->alloc_ptrs[i] == ptr) { + gc->alloc_ptrs[i] = new_ptr; + break; + } + } + } + + + return new_ptr; +} + char *libxl__sprintf(libxl__gc *gc, const char *fmt, ...) { char *s; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 4580be9..64942e6 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -145,6 +145,7 @@ _hidden int libxl__ptr_add(libxl__gc *gc, void *ptr); _hidden void libxl__free_all(libxl__gc *gc); _hidden void *libxl__zalloc(libxl__gc *gc, int bytes); _hidden void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size); +_hidden void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size); _hidden char *libxl__sprintf(libxl__gc *gc, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3); _hidden char *libxl__strdup(libxl__gc *gc, const char *c); _hidden char *libxl__dirname(libxl__gc *gc, const char *s); -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 5/7] libxl, Intruduce libxl__strndup.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- tools/libxl/libxl_internal.c | 10 ++++++++++ tools/libxl/libxl_internal.h | 1 + 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c index 527ebc5..66f9c16 100644 --- a/tools/libxl/libxl_internal.c +++ b/tools/libxl/libxl_internal.c @@ -155,6 +155,16 @@ char *libxl__strdup(libxl__gc *gc, const char *c) return s; } +char *libxl__strndup(libxl__gc *gc, const char *c, size_t n) +{ + char *s = strndup(c, n); + + if (s) + libxl__ptr_add(gc, s); + + return s; +} + char *libxl__dirname(libxl__gc *gc, const char *s) { char *c; diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 64942e6..839df14 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -148,6 +148,7 @@ _hidden void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size); _hidden void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size); _hidden char *libxl__sprintf(libxl__gc *gc, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3); _hidden char *libxl__strdup(libxl__gc *gc, const char *c); +_hidden char *libxl__strndup(libxl__gc *gc, const char *c, size_t n); _hidden char *libxl__dirname(libxl__gc *gc, const char *s); _hidden char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, flexarray_t *array, int length); -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 6/7] 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 | 95 ++++++++ tools/libxl/libxl_json.c | 505 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 604 insertions(+), 1 deletions(-) create mode 100644 tools/libxl/libxl_json.c diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index 1f6b418..e50874e 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 839df14..a8aa6d5 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -382,4 +382,99 @@ _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; + 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 { + 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__gc *gc, + const char *s); + #endif diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c new file mode 100644 index 0000000..d0fa5be --- /dev/null +++ b/tools/libxl/libxl_json.c @@ -0,0 +1,505 @@ +/* + * 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 <assert.h> +#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(ctx) \ + if ((ctx)->g == NULL) { \ + yajl_gen_config conf = { 1, " " }; \ + (ctx)->g = yajl_gen_alloc(&conf, NULL); \ + } +# define DEBUG_GEN_FREE(ctx) if ((ctx)->g) yajl_gen_free((ctx)->g) +# define DEBUG_GEN(ctx, type) yajl_gen_##type(ctx->g) +# define DEBUG_GEN_VALUE(ctx, type, value) yajl_gen_##type(ctx->g, value) +# define DEBUG_GEN_STRING(ctx, str, n) yajl_gen_string(ctx->g, str, n) +# define DEBUG_GEN_REPORT(yajl_ctx) \ + do { \ + const unsigned char *buf = NULL; \ + unsigned int len = 0; \ + yajl_gen_get_buf((yajl_ctx)->g, &buf, &len); \ + LIBXL__LOG((yajl_ctx)->ctx, LIBXL__LOG_DEBUG, "response:\n%s", buf); \ + yajl_gen_free((yajl_ctx)->g); \ + (yajl_ctx)->g = NULL; \ + } while (0) +#else +# define DEBUG_GEN_ALLOC(ctx) ((void)0) +# define DEBUG_GEN_FREE(ctx) ((void)0) +# define DEBUG_GEN(ctx, type) ((void)0) +# define DEBUG_GEN_VALUE(ctx, type, value) ((void)0) +# define DEBUG_GEN_STRING(ctx, value, lenght) ((void)0) +# define DEBUG_GEN_REPORT(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) +{ + assert(dst != NULL); + + 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(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(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, 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, 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, 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, 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 = NULL; + libxl__json_object *obj = NULL; + + t = malloc(len + 1); + if (t == NULL) { + LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR, "Failed to allocate"); + return 0; + } + + DEBUG_GEN_STRING(ctx, 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 = NULL; + libxl__json_object *obj = ctx->current; + + t = malloc(len + 1); + if (t == NULL) { + LIBXL__LOG_ERRNO(ctx->ctx, LIBXL__LOG_ERROR, "Failed to allocate"); + return 0; + } + + DEBUG_GEN_STRING(ctx, 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, 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, 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, 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, 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); + yajl_ctx->hand = NULL; + } + DEBUG_GEN_FREE(yajl_ctx); +} + +libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s) +{ + yajl_status status; + libxl__yajl_ctx yajl_ctx; + + memset(&yajl_ctx, 0, sizeof (yajl_ctx)); + yajl_ctx.ctx = libxl__gc_owner(gc); + + DEBUG_GEN_ALLOC(&yajl_ctx); + + /* 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, (const unsigned char *)s, strlen(s)); + status = yajl_parse_complete(yajl_ctx.hand); + + if (status == yajl_status_ok) { + libxl__json_object *o = yajl_ctx.head; + + DEBUG_GEN_REPORT(&yajl_ctx); + + yajl_ctx.head = NULL; + + yajl_ctx_free(&yajl_ctx); + return o; + } else { + unsigned char *str = yajl_get_error(yajl_ctx.hand, 1, (const unsigned char *)s, strlen(s)); + + LIBXL__LOG(yajl_ctx.ctx, LIBXL__LOG_ERROR, "yajl error: %s", str); + yajl_free_error(yajl_ctx.hand, str); + + json_object_free(yajl_ctx.ctx, yajl_ctx.head); + yajl_ctx_free(&yajl_ctx); + return NULL; + } +} -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-20 21:24 UTC
[Xen-devel] [PATCH V7 7/7] 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 | 4 + tools/libxl/libxl_dm.c | 10 + tools/libxl/libxl_internal.h | 19 ++ tools/libxl/libxl_qmp.c | 597 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 633 insertions(+), 1 deletions(-) create mode 100644 tools/libxl/libxl_qmp.c diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile index e50874e..f10c7e8 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 83d604c..79f9992 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 b74b66f..dd746ff 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -520,6 +520,10 @@ 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 b9bf4b0..1a80e7e 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -244,6 +244,16 @@ 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-libxl-%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 a8aa6d5..413bb36 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -382,6 +382,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..90dba72 --- /dev/null +++ b/tools/libxl/libxl_qmp.c @@ -0,0 +1,597 @@ +/* + * 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(buf, len) \ + LIBXL__LOG(qmp->ctx, LIBXL__LOG_DEBUG, "received: ''%.*s''", len, buf) +#else +# define DEBUG_REPORT_RECEIVED(buf, len) ((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; + + 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) { + pp->callback(qmp, NULL); + 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__gc *gc, libxl__qmp_handler *qmp) +{ + ssize_t rd; + char *s = NULL; + char *s_end = NULL; + + char *incomplete = NULL; + size_t incomplete_size = 0; + + do { + 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) { + DEBUG_REPORT_RECEIVED(qmp->buffer, rd); + + do { + char *end = NULL; + if (incomplete) { + size_t current_pos = s - incomplete; + incomplete_size += rd; + incomplete = libxl__realloc(gc, incomplete, incomplete_size + 1); + incomplete = strncat(incomplete, qmp->buffer, rd); + s = incomplete + current_pos; + s_end = incomplete + incomplete_size; + } else { + incomplete = libxl__strndup(gc, qmp->buffer, rd); + incomplete_size = rd; + s = incomplete; + s_end = s + rd; + } + + end = strstr(s, "\r\n"); + if (end) { + libxl__json_object *o = NULL; + + *end = ''\0''; + + o = libxl__json_parse(gc, s); + s = end + 2; + + if (o) { + qmp_handle_response(qmp, o); + json_object_free(qmp->ctx, o); + } + } else { + break; + } + } while (s < s_end); + } 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; + } + } while (s < s_end); + + 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; + libxl__gc gc = LIBXL_INIT_GC(qmp->ctx); + + 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(&gc, qmp)) < 0) { + return ret; + } + } + + libxl__free_all(&gc); + + 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-libxl-%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"); + libxl__free_all(&gc); + 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(&gc, qmp)) < 0) { + break; + } + } + + libxl__free_all(&gc); + if (!qmp->connected) { + LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Failed to connect to QMP"); + libxl__qmp_close(qmp); + return NULL; + } + 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-libxl-%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; + 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
New: - New patch that add set_default_namespaces to libxltypes.py, so we can set a namespace to use for a specific .idl. - New function: libxl__realloc and libxl__strndup. Anthony PERARD (7): libxl: Rename libxl.idl to libxl_types.idl. libxl: Add get/set_default_namespace in libxltypes.py. libxl: Introduce libxl_internal_types.idl. libxl, Introduce libxl__realloc. libxl, Intruduce libxl__strndup. 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 | 4 + tools/libxl/libxl_dm.c | 10 + tools/libxl/libxl_internal.c | 30 ++ tools/libxl/libxl_internal.h | 117 ++++++ tools/libxl/libxl_json.c | 505 +++++++++++++++++++++++ tools/libxl/libxl_qmp.c | 597 ++++++++++++++++++++++++++++ tools/libxl/{libxl.idl => libxl_types.idl} | 2 + tools/libxl/libxl_types_internal.idl | 9 + tools/libxl/libxltypes.py | 16 +- tools/ocaml/libs/xl/Makefile | 4 +- tools/python/Makefile | 4 +- 14 files changed, 1315 insertions(+), 21 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} (99%) create mode 100644 tools/libxl/libxl_types_internal.idl -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 07:49 UTC
[Xen-devel] Re: [PATCH V7 4/7] libxl, Introduce libxl__realloc.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > tools/libxl/libxl_internal.c | 20 ++++++++++++++++++++ > tools/libxl/libxl_internal.h | 1 + > 2 files changed, 21 insertions(+), 0 deletions(-) > > diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c > index e259278..527ebc5 100644 > --- a/tools/libxl/libxl_internal.c > +++ b/tools/libxl/libxl_internal.c > @@ -102,6 +102,26 @@ void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size) > return ptr; > } > > +void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size) > +{ > + void *new_ptr = realloc(ptr, new_size);On failure realloc will return NULL but not free the old pointer, so I think in that case you will set alloc_ptrs[i] to NULL but not actually free the old pointer, hence leaking it. I think you can just check for new_ptr == NULL and return NULL up front. Normally that would leak the old ptr but by leaving it in the gc array we avoid that pitfall. If new_size==0 then realloc behaves as free. I reckon you can just return NULL then too and allow the gc to clean up. Or you could outlaw such uses in this interface and abort(), that seems harsh however. BTW libxl__free_all does cope correctly with NULLs mid-way through alloc_ptrs[] which did concern me initially. Ian.> + int i = 0; > + > + if (ptr == NULL) { > + libxl__ptr_add(gc, new_ptr); > + } else if (new_ptr != ptr) { > + for (i = 0; i < gc->alloc_maxsize; i++) { > + if (gc->alloc_ptrs[i] == ptr) { > + gc->alloc_ptrs[i] = new_ptr; > + break; > + } > + } > + }> + > + > + return new_ptr; > +} > + > char *libxl__sprintf(libxl__gc *gc, const char *fmt, ...) > { > char *s; > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 4580be9..64942e6 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -145,6 +145,7 @@ _hidden int libxl__ptr_add(libxl__gc *gc, void *ptr); > _hidden void libxl__free_all(libxl__gc *gc); > _hidden void *libxl__zalloc(libxl__gc *gc, int bytes); > _hidden void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size); > +_hidden void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size); > _hidden char *libxl__sprintf(libxl__gc *gc, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3); > _hidden char *libxl__strdup(libxl__gc *gc, const char *c); > _hidden char *libxl__dirname(libxl__gc *gc, const char *s);_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 07:56 UTC
[Xen-devel] Re: [PATCH V7 2/7] libxl: Add get/set_default_namespace in libxltypes.py.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>I''m not sure if it matters but default_namespace should possibly be private to the module (__ at the start)? Perhaps get_default_namesspace too, probably neither .idl files nor gen*.py should ever need to query this, they should use the info in specific types. If I were prone to bikeshedding I''d suggest that "set_default_namespace" could be just "namespace" since it would read nicer in the .idl files. In any case that''s all nit picking so: Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/libxl_types.idl | 2 ++ > tools/libxl/libxltypes.py | 16 ++++++++++++++-- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index f7249b1..718688d 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -3,6 +3,8 @@ > # Builtin libxl types > # > > +set_default_namespace("libxl_") > + > libxl_domid = Builtin("domid") > libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE) > libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE) > diff --git a/tools/libxl/libxltypes.py b/tools/libxl/libxltypes.py > index b7b4669..64d2f50 100644 > --- a/tools/libxl/libxltypes.py > +++ b/tools/libxl/libxltypes.py > @@ -8,10 +8,21 @@ DIR_IN = 1 > DIR_OUT = 2 > DIR_BOTH = 3 > > +default_namespace = "" > +def set_default_namespace(s): > + if type(s) != str: > + raise TypeError, "Require a string for the default namespace." > + global default_namespace > + default_namespace = s > + > +def get_default_namespace(): > + global default_namespace > + return default_namespace > + > class Type(object): > def __init__(self, typename, **kwargs): > self.comment = kwargs.setdefault(''comment'', None) > - self.namespace = kwargs.setdefault(''namespace'', "libxl_") > + self.namespace = kwargs.setdefault(''namespace'', get_default_namespace()) > self.dir = kwargs.setdefault(''dir'', DIR_BOTH) > if self.dir not in [DIR_NONE, DIR_IN, DIR_OUT, DIR_BOTH]: > raise ValueError > @@ -248,7 +259,8 @@ def parse(f): > elif isinstance(t,type(object)) and issubclass(t, Type): > globs[n] = t > elif n in [''PASS_BY_REFERENCE'', ''PASS_BY_VALUE'', > - ''DIR_NONE'', ''DIR_IN'', ''DIR_OUT'', ''DIR_BOTH'']: > + ''DIR_NONE'', ''DIR_IN'', ''DIR_OUT'', ''DIR_BOTH'', > + ''set_default_namespace'']: > globs[n] = t > > try:_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 07:57 UTC
[Xen-devel] Re: [PATCH V7 1/7] libxl: Rename libxl.idl to libxl_types.idl.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/Makefile | 12 ++++++------ > tools/libxl/{libxl.idl => libxl_types.idl} | 0 > tools/ocaml/libs/xl/Makefile | 4 ++-- > tools/python/Makefile | 4 ++-- > 4 files changed, 10 insertions(+), 10 deletions(-) > rename tools/libxl/{libxl.idl => libxl_types.idl} (100%) > > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile > index 3eeb26f..330ee6e 100644 > --- a/tools/libxl/Makefile > +++ b/tools/libxl/Makefile > @@ -52,8 +52,8 @@ $(XL_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h > $(XL_OBJS): CFLAGS += $(CFLAGS_libxenlight) > > testidl.o: CFLAGS += $(CFLAGS_libxenctrl) $(CFLAGS_libxenlight) > -testidl.c: libxl.idl gentest.py libxl.h > - $(PYTHON) gentest.py libxl.idl testidl.c.new > +testidl.c: libxl_types.idl gentest.py libxl.h > + $(PYTHON) gentest.py libxl_types.idl testidl.c.new > mv testidl.c.new testidl.c > > .PHONY: all > @@ -84,10 +84,10 @@ libxl.h: _libxl_types.h > > $(LIBXL_OBJS) $(LIBXLU_OBJS) $(XL_OBJS): libxl.h > > -_libxl_%.h _libxl_%.c: libxl.idl gen%.py libxl%.py > - $(PYTHON) gen$*.py libxl.idl __libxl_$*.h __libxl_$*.c > - $(call move-if-changed,__libxl_$*.h,_libxl_$*.h) > - $(call move-if-changed,__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 > + $(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h) > + $(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c) > > libxenlight.so: libxenlight.so.$(MAJOR) > ln -sf $< $@ > 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/ocaml/libs/xl/Makefile b/tools/ocaml/libs/xl/Makefile > index 342dc35..a1e79a5 100644 > --- a/tools/ocaml/libs/xl/Makefile > +++ b/tools/ocaml/libs/xl/Makefile > @@ -45,10 +45,10 @@ xl.mli: xl.mli.in _libxl_types.mli.in > < xl.mli.in > xl.mli.tmp > $(Q)mv xl.mli.tmp xl.mli > > -_libxl_types.ml.in _libxl_types.mli.in _libxl_types.inc: genwrap.py $(XEN_ROOT)/tools/libxl/libxl.idl \ > +_libxl_types.ml.in _libxl_types.mli.in _libxl_types.inc: genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \ > $(XEN_ROOT)/tools/libxl/libxltypes.py > PYTHONPATH=$(XEN_ROOT)/tools/libxl $(PYTHON) genwrap.py \ > - $(XEN_ROOT)/tools/libxl/libxl.idl \ > + $(XEN_ROOT)/tools/libxl/libxl_types.idl \ > _libxl_types.mli.in _libxl_types.ml.in _libxl_types.inc > > libs: $(LIBS) > diff --git a/tools/python/Makefile b/tools/python/Makefile > index de44178..b7bc501 100644 > --- a/tools/python/Makefile > +++ b/tools/python/Makefile > @@ -10,10 +10,10 @@ genpath-target = $(call buildmakevars2file,$(XENPATH)) > $(eval $(genpath-target)) > > .PHONY: build > -build: genpath genwrap.py $(XEN_ROOT)/tools/libxl/libxl.idl \ > +build: genpath genwrap.py $(XEN_ROOT)/tools/libxl/libxl_types.idl \ > $(XEN_ROOT)/tools/libxl/libxltypes.py > PYTHONPATH=$(XEN_ROOT)/tools/libxl $(PYTHON) genwrap.py \ > - $(XEN_ROOT)/tools/libxl/libxl.idl \ > + $(XEN_ROOT)/tools/libxl/libxl_types.idl \ > xen/lowlevel/xl/_pyxl_types.h \ > xen/lowlevel/xl/_pyxl_types.c > CC="$(CC)" CFLAGS="$(CFLAGS)" $(PYTHON) setup.py build_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 08:10 UTC
[Xen-devel] Re: [PATCH V7 3/7] libxl: Introduce libxl_internal_types.idl.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>This needs rebasing over 23728:548b2826293e "libxl: clean up trailing whitespaces in code" otherwise: Acked-by: Ian Campbell <ian.campbell@citrix.com>> - > - f.write("""#ifndef __LIBXL_TYPES_H > -#define __LIBXL_TYPES_H > + > + header_define = header.upper().replace(''.'',''_'')It took me ages to figure out that this doesn''t change the define because we generate to a temporary file with a __ prefix... _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 08:12 UTC
[Xen-devel] Re: [PATCH V7 5/7] libxl, Intruduce libxl__strndup.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>Acked-by: Ian Campbell <ian.campbell@citrix.com>> --- > tools/libxl/libxl_internal.c | 10 ++++++++++ > tools/libxl/libxl_internal.h | 1 + > 2 files changed, 11 insertions(+), 0 deletions(-) > > diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c > index 527ebc5..66f9c16 100644 > --- a/tools/libxl/libxl_internal.c > +++ b/tools/libxl/libxl_internal.c > @@ -155,6 +155,16 @@ char *libxl__strdup(libxl__gc *gc, const char *c) > return s; > } > > +char *libxl__strndup(libxl__gc *gc, const char *c, size_t n) > +{ > + char *s = strndup(c, n); > + > + if (s) > + libxl__ptr_add(gc, s); > + > + return s; > +} > + > char *libxl__dirname(libxl__gc *gc, const char *s) > { > char *c; > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 64942e6..839df14 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -148,6 +148,7 @@ _hidden void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size); > _hidden void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size); > _hidden char *libxl__sprintf(libxl__gc *gc, const char *fmt, ...) PRINTF_ATTRIBUTE(2, 3); > _hidden char *libxl__strdup(libxl__gc *gc, const char *c); > +_hidden char *libxl__strndup(libxl__gc *gc, const char *c, size_t n); > _hidden char *libxl__dirname(libxl__gc *gc, const char *s); > > _hidden char **libxl__xs_kvs_of_flexarray(libxl__gc *gc, flexarray_t *array, int length);_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 09:55 UTC
[Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.
On Wed, 2011-07-20 at 22:24 +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 | 95 ++++++++ > tools/libxl/libxl_json.c | 505 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 604 insertions(+), 1 deletions(-) > create mode 100644 tools/libxl/libxl_json.c > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > index 839df14..a8aa6d5 100644 > --- a/tools/libxl/libxl_internal.h > +++ b/tools/libxl/libxl_internal.h > @@ -382,4 +382,99 @@ _hidden int libxl__e820_alloc(libxl_ctx *ctx, uint32_t domid, libxl_domain_confi[...]> +static inline bool json_object_is_string(const libxl__json_object *o) > [...] > +static inline bool json_object_is_integer(const libxl__json_object *o) > [...] > +static inline bool json_object_is_map(const libxl__json_object *o) > [...] > +static inline bool json_object_is_array(const libxl__json_object *o) > [...] > +static inline const char *json_object_get_string(const libxl__json_object *o) > [...] > +static inline flexarray_t *json_object_get_map(const libxl__json_object *o) > [...] > +static inline flexarray_t *json_object_get_array(const libxl__json_object *o) > [...] > +static inline long json_object_get_integer(const libxl__json_object *o) > [...] > +_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);These should all be libxl__json_object_FOO. (just json is ok for static functions in .c files if you prefer but for functions in headers I think correct namespacing makes sense). Internal functions (such as [libxl__]json_object_free) should always take a libxl__gc and not a libxl_ctx.> +/* s: is the buffer to parse, libxl__json_parse will advance the pointer the > + * part that has not been parsedIt can''t advance a "const char *s" (at least not in a way which the caller can see). Is the comment or the implementation wrong?> + * *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__gc *gc, > + const char *s); > + > #endif > diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c > new file mode 100644 > index 0000000..d0fa5be > --- /dev/null > +++ b/tools/libxl/libxl_json.cI think I reviewed this before so I just did a quick skim.> +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)) {This function only operates on map types? Should it therefore be names [libxl__]json_map_get? Is passing a non-map to this function a hard error (e.g. abort())? I suppose I''ll find out in the next patch ;-)> [...]Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 11:17 UTC
[Xen-devel] Re: [PATCH V7 7/7] libxl, Introduce a QMP client
On Wed, 2011-07-20 at 22:24 +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).That path doesn''t match the implementation now. Again I think I''ve reviewed much of this before so I only skimmed it, although I paid attention to the new stuff relating to pulling through to crlf etc.> diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c > new file mode 100644 > index 0000000..90dba72 > --- /dev/null > +++ b/tools/libxl/libxl_qmp.c[...]> +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;I think a helper function to retrieve an index into a json_array would be helpful. I think in general exposing the internal use of flexarrays in this interface to callers can be avoided.> + label = json_object_get("label", obj, JSON_STRING); > + s = json_object_get_string(label);If obj is not a map label will be null but json_object_get_string will DTRT and return NULL so this works. But perhaps an explicit type check would be correct here?> +/* > + * 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;Another helper function to get the a node from a map? Or even a json_map_for_each_node type construct?> + 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);Check that it is an integer? Presumably the -1 error return is never a valid id but it''d save a useless list walk.> + SIMPLEQ_FOREACH(pp, &qmp->callback_list, next) { > + if (pp->id == id) { > + return pp; > + } > + } > + } > + return NULL; > +} > +[...]> +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)); > +}Belongs in libxl_json I think.> +static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) > +{ > + ssize_t rd; > + char *s = NULL; > + char *s_end = NULL; > + > + char *incomplete = NULL; > + size_t incomplete_size = 0; > + > + do { > + 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 you do "if (rd == 0) continue" and the rd < 0 error handling+return here the error handling is nearer the error-site and hence easier to grok. (also you save a level of indentation, although the inner loop isn''t wrapping in this particular case) Actually the error handling for ret = select is much the same, if you pull the ret==0 and ret<0 cases up after the select it easier to follow.> + if (rd > 0) { > + DEBUG_REPORT_RECEIVED(qmp->buffer, rd); > + > + do { > + char *end = NULL; > + if (incomplete) { > + size_t current_pos = s - incomplete; > + incomplete_size += rd; > + incomplete = libxl__realloc(gc, incomplete, incomplete_size + 1); > + incomplete = strncat(incomplete, qmp->buffer, rd); > + s = incomplete + current_pos; > + s_end = incomplete + incomplete_size; > + } else { > + incomplete = libxl__strndup(gc, qmp->buffer, rd); > + incomplete_size = rd; > + s = incomplete; > + s_end = s + rd; > + } > + > + end = strstr(s, "\r\n");Is s definitely NULL terminated here? (yes, according to strncat(3), good!)> + if (end) { > + libxl__json_object *o = NULL; > + > + *end = ''\0''; > + > + o = libxl__json_parse(gc, s); > + s = end + 2; > + > + if (o) { > + qmp_handle_response(qmp, o); > + json_object_free(qmp->ctx, o); > + }"if (!o)" is now an error since you pulled up to a "\r\n"?> + } else { > + break; > + } > + } while (s < s_end); > + } 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; > + } > + } while (s < s_end); > + > + return 1; > +} > +[...] Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-21 11:34 UTC
[Xen-devel] Re: [PATCH V7 4/7] libxl, Introduce libxl__realloc.
On Thu, 21 Jul 2011, Ian Campbell wrote:> On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote: > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > --- > > tools/libxl/libxl_internal.c | 20 ++++++++++++++++++++ > > tools/libxl/libxl_internal.h | 1 + > > 2 files changed, 21 insertions(+), 0 deletions(-) > > > > diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c > > index e259278..527ebc5 100644 > > --- a/tools/libxl/libxl_internal.c > > +++ b/tools/libxl/libxl_internal.c > > @@ -102,6 +102,26 @@ void *libxl__calloc(libxl__gc *gc, size_t nmemb, size_t size) > > return ptr; > > } > > > > +void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size) > > +{ > > + void *new_ptr = realloc(ptr, new_size); > > On failure realloc will return NULL but not free the old pointer, so I > think in that case you will set alloc_ptrs[i] to NULL but not actually > free the old pointer, hence leaking it.Oops, I''ll fix that, and just return new_ptr (without removing the pointer from gc).> I think you can just check for new_ptr == NULL and return NULL up front. > Normally that would leak the old ptr but by leaving it in the gc array > we avoid that pitfall. > > If new_size==0 then realloc behaves as free. I reckon you can just > return NULL then too and allow the gc to clean up. Or you could outlaw > such uses in this interface and abort(), that seems harsh however.I think I will remove the pointer from gc, so libxl__realloc will behave as expected and free stuff if asked.> BTW libxl__free_all does cope correctly with NULLs mid-way through > alloc_ptrs[] which did concern me initially.Thanks, -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-21 12:02 UTC
[Xen-devel] Re: [PATCH V7 2/7] libxl: Add get/set_default_namespace in libxltypes.py.
On Thu, 21 Jul 2011, Ian Campbell wrote:> On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote: > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > > I''m not sure if it matters but default_namespace should possibly be > private to the module (__ at the start)? Perhaps get_default_namesspace > too, probably neither .idl files nor gen*.py should ever need to query > this, they should use the info in specific types.Ok, I will change names but use only one ''_'' for "private" function/variable.> If I were prone to bikeshedding I''d suggest that "set_default_namespace" > could be just "namespace" since it would read nicer in the .idl files.:), I''ll change that too.> In any case that''s all nit picking so: > > Acked-by: Ian Campbell <ian.campbell@citrix.com>Thanks, -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 13:21 UTC
[Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile > index 1f6b418..e50874e 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 += -lyajlI''m not convinced it is up to date and/or canonical but it would be worth adding yajl to the exteral requirements list in the top-level README too. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-21 14:23 UTC
[Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.
On Thu, 21 Jul 2011, Ian Campbell wrote:> On Wed, 2011-07-20 at 22:24 +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 | 95 ++++++++ > > tools/libxl/libxl_json.c | 505 ++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 604 insertions(+), 1 deletions(-) > > create mode 100644 tools/libxl/libxl_json.c > > > > diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h > > index 839df14..a8aa6d5 100644 > > --- a/tools/libxl/libxl_internal.h > > +++ b/tools/libxl/libxl_internal.h > > @@ -382,4 +382,99 @@ _hidden int libxl__e820_alloc(libxl_ctx *ctx, uint32_t domid, libxl_domain_confi > [...] > > +static inline bool json_object_is_string(const libxl__json_object *o) > > [...] > > +static inline bool json_object_is_integer(const libxl__json_object *o) > > [...] > > +static inline bool json_object_is_map(const libxl__json_object *o) > > [...] > > +static inline bool json_object_is_array(const libxl__json_object *o) > > [...] > > +static inline const char *json_object_get_string(const libxl__json_object *o) > > [...] > > +static inline flexarray_t *json_object_get_map(const libxl__json_object *o) > > [...] > > +static inline flexarray_t *json_object_get_array(const libxl__json_object *o) > > [...] > > +static inline long json_object_get_integer(const libxl__json_object *o) > > [...] > > +_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); > > These should all be libxl__json_object_FOO. (just json is ok for static > functions in .c files if you prefer but for functions in headers I think > correct namespacing makes sense). > > Internal functions (such as [libxl__]json_object_free) should always > take a libxl__gc and not a libxl_ctx.Ok, I will change that.> > +/* s: is the buffer to parse, libxl__json_parse will advance the pointer the > > + * part that has not been parsed > > It can''t advance a "const char *s" (at least not in a way which the > caller can see). Is the comment or the implementation wrong?The comment is wrong, not updated. I''ll just remove it as I think the prototype is explicit enough. (Parse *s, return a json_object OR NULL in case of error)> > + * *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__gc *gc, > > + const char *s); > > + > > #endif > > diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c > > new file mode 100644 > > index 0000000..d0fa5be > > --- /dev/null > > +++ b/tools/libxl/libxl_json.c > > I think I reviewed this before so I just did a quick skim. > > > +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)) { > > This function only operates on map types? Should it therefore be names > [libxl__]json_map_get?Yes, the only type to use keys. So, I''ll rename it.> Is passing a non-map to this function a hard error (e.g. abort())? > > I suppose I''ll find out in the next patch ;-)It is the same error as if the map does not contain the key or the type of the object found is not the right one. So not a hard error here. Thanks, -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 14:40 UTC
[Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.
On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:> + /* parse the input */ > + if (yajl_ctx.hand == NULL) { > + /* allow comments */ > + yajl_parser_config cfg = { 1, 1 };Looks like YALJ v2 has enums for these config values but v1 doesn''t. However yajl_parse_config cfg = { .allowComments = 1, .checkUTF8 = 1, }; is a bit more self documenting. (supporting yajl 1 and 2 is going to be a pain. but lets cross that bridge when we come to it...) Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-21 15:02 UTC
Re: [Xen-devel] Re: [PATCH V7 6/7] libxl: Introduce JSON parsing stuff.
On Thu, Jul 21, 2011 at 15:40, Ian Campbell <Ian.Campbell@eu.citrix.com> wrote:> On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote: >> + /* parse the input */ >> + if (yajl_ctx.hand == NULL) { >> + /* allow comments */ >> + yajl_parser_config cfg = { 1, 1 }; > > Looks like YALJ v2 has enums for these config values but v1 doesn''t. > However > yajl_parse_config cfg = { > .allowComments = 1, > .checkUTF8 = 1, > }; > is a bit more self documenting. > > (supporting yajl 1 and 2 is going to be a pain. but lets cross that > bridge when we come to it...)With the V2, there is "yajl_tree_parse" that basicly do the same as libxl__json_parse. I try to be closer to the tree implementation of the V2, but yes, it will sill be painfull to support both. If we can only support the v1, and change to the v2 later (when widly availlable in distribution), it should be OK. -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Anthony PERARD
2011-Jul-21 16:29 UTC
[Xen-devel] Re: [PATCH V7 7/7] libxl, Introduce a QMP client
On Thu, 21 Jul 2011, Ian Campbell wrote:> On Wed, 2011-07-20 at 22:24 +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). > > That path doesn''t match the implementation now. > > Again I think I''ve reviewed much of this before so I only skimmed it, > although I paid attention to the new stuff relating to pulling through > to crlf etc. > > > diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c > > new file mode 100644 > > index 0000000..90dba72 > > --- /dev/null > > +++ b/tools/libxl/libxl_qmp.c > [...] > > +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; > > I think a helper function to retrieve an index into a json_array would > be helpful. I think in general exposing the internal use of flexarrays > in this interface to callers can be avoided.Ok, I will do this helper.> > + label = json_object_get("label", obj, JSON_STRING); > > + s = json_object_get_string(label); > > If obj is not a map label will be null but json_object_get_string will > DTRT and return NULL so this works. But perhaps an explicit type check > would be correct here?Ok, I will add this explicit check.> > +/* > > + * 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; > > Another helper function to get the a node from a map? Or even a > json_map_for_each_node type construct? > > > + 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); > > Check that it is an integer? Presumably the -1 error return is never a > valid id but it''d save a useless list walk.Actually, the parametter JSON_INTEGER given to json_object_get ask to explicitly return a json_object with an integer, otherwise, the function return null. So the get_integer will not return an error.> > + SIMPLEQ_FOREACH(pp, &qmp->callback_list, next) { > > + if (pp->id == id) { > > + return pp; > > + } > > + } > > + } > > + return NULL; > > +} > > + > [...] > > +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)); > > +} > > Belongs in libxl_json I think.Actually, I have put it here to not expose the yajl_gen.h to all user of libxl_internal.h. Otherwise, yes, it does not really belong to libxl_qmp.> > +static int qmp_next(libxl__gc *gc, libxl__qmp_handler *qmp) > > +{ > > + ssize_t rd; > > + char *s = NULL; > > + char *s_end = NULL; > > + > > + char *incomplete = NULL; > > + size_t incomplete_size = 0; > > + > > + do { > > + 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 you do "if (rd == 0) continue" and the rd < 0 error handling+return > here the error handling is nearer the error-site and hence easier to > grok. (also you save a level of indentation, although the inner loop > isn''t wrapping in this particular case) > > Actually the error handling for ret = select is much the same, if you > pull the ret==0 and ret<0 cases up after the select it easier to follow.OK.> > + if (rd > 0) { > > + DEBUG_REPORT_RECEIVED(qmp->buffer, rd); > > + > > + do { > > + char *end = NULL; > > + if (incomplete) { > > + size_t current_pos = s - incomplete; > > + incomplete_size += rd; > > + incomplete = libxl__realloc(gc, incomplete, incomplete_size + 1); > > + incomplete = strncat(incomplete, qmp->buffer, rd); > > + s = incomplete + current_pos; > > + s_end = incomplete + incomplete_size; > > + } else { > > + incomplete = libxl__strndup(gc, qmp->buffer, rd); > > + incomplete_size = rd; > > + s = incomplete; > > + s_end = s + rd; > > + } > > + > > + end = strstr(s, "\r\n"); > > Is s definitely NULL terminated here? (yes, according to strncat(3), > good!)Exactly :), otherwise a strnstr would be helpfull. (I actually write one before to use strncat.)> > + if (end) { > > + libxl__json_object *o = NULL; > > + > > + *end = ''\0''; > > + > > + o = libxl__json_parse(gc, s); > > + s = end + 2; > > + > > + if (o) { > > + qmp_handle_response(qmp, o); > > + json_object_free(qmp->ctx, o); > > + } > > "if (!o)" is now an error since you pulled up to a "\r\n"?Indeed, I should add a log_error, even if libxl__json_parse already print something. But I''m not sure if return -1 immediatly is a good idee, maybe the next json object is good.> > + } else { > > + break; > > + } > > + } while (s < s_end); > > + } 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; > > + } > > + } while (s < s_end); > > + > > + return 1; > > +} > > + > [...] > > Ian. > >-- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-21 16:43 UTC
[Xen-devel] Re: [PATCH V7 7/7] libxl, Introduce a QMP client
On Thu, 2011-07-21 at 17:29 +0100, Anthony PERARD wrote:> On Thu, 21 Jul 2011, Ian Campbell wrote: > > > On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote: > > > +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); > > > > Check that it is an integer? Presumably the -1 error return is never a > > valid id but it''d save a useless list walk. > > Actually, the parametter JSON_INTEGER given to json_object_get ask to > explicitly return a json_object with an integer, otherwise, the function > return null. > > So the get_integer will not return an error.Oh right, makes sense.> > > + SIMPLEQ_FOREACH(pp, &qmp->callback_list, next) { > > > + if (pp->id == id) { > > > + return pp; > > > + } > > > + } > > > + } > > > + return NULL; > > > +} > > > + > > [...] > > > +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)); > > > +} > > > > Belongs in libxl_json I think. > > Actually, I have put it here to not expose the yajl_gen.h to all user of > libxl_internal.h. > > Otherwise, yes, it does not really belong to libxl_qmp.I wanted it there because my json generating patch also needs it ;-) I think exposing yajl_gen.h to libxl_internal is ok, it''s libxl.h where we should avoid it. I can patch this up in my series which uses JSON though so no worries. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell
2011-Jul-25 10:58 UTC
[Xen-devel] Re: [PATCH V7 2/7] libxl: Add get/set_default_namespace in libxltypes.py.
On Wed, 2011-07-20 at 17:24 -0400, Anthony PERARD wrote:> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > tools/libxl/libxl_types.idl | 2 ++ > tools/libxl/libxltypes.py | 16 ++++++++++++++-- > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index f7249b1..718688d 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -3,6 +3,8 @@ > # Builtin libxl types > # > > +set_default_namespace("libxl_") > + > libxl_domid = Builtin("domid") > libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE) > libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE) > diff --git a/tools/libxl/libxltypes.py b/tools/libxl/libxltypes.py > index b7b4669..64d2f50 100644 > --- a/tools/libxl/libxltypes.py > +++ b/tools/libxl/libxltypes.py > @@ -8,10 +8,21 @@ DIR_IN = 1 > DIR_OUT = 2 > DIR_BOTH = 3 > > +default_namespace = "" > +def set_default_namespace(s): > + if type(s) != str: > + raise TypeError, "Require a string for the default namespace." > + global default_namespace > + default_namespace = s > + > +def get_default_namespace(): > + global default_namespace > + return default_namespaceCan you make this 4-space indent please. The rest of this file is indented that way but this comes first so emacs'' python-guess-indent guesses 2 and that confuses the autoindent of the following code. Ian.> + > class Type(object): > def __init__(self, typename, **kwargs): > self.comment = kwargs.setdefault(''comment'', None) > - self.namespace = kwargs.setdefault(''namespace'', "libxl_") > + self.namespace = kwargs.setdefault(''namespace'', get_default_namespace()) > self.dir = kwargs.setdefault(''dir'', DIR_BOTH) > if self.dir not in [DIR_NONE, DIR_IN, DIR_OUT, DIR_BOTH]: > raise ValueError > @@ -248,7 +259,8 @@ def parse(f): > elif isinstance(t,type(object)) and issubclass(t, Type): > globs[n] = t > elif n in [''PASS_BY_REFERENCE'', ''PASS_BY_VALUE'', > - ''DIR_NONE'', ''DIR_IN'', ''DIR_OUT'', ''DIR_BOTH'']: > + ''DIR_NONE'', ''DIR_IN'', ''DIR_OUT'', ''DIR_BOTH'', > + ''set_default_namespace'']: > globs[n] = t > > try:_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel