libxl: support json for pretty printing objects
PATCH 1/1 libxl: support json for pretty printing objects
Now that Anthony's QMP series is in we can build upon the use of YAJL to add support for pretty printing libxl objects as JSON.
Also includes a user in xl (to print disks on dry run) and an associated fix to the check-xl-disk-parse test script.
I've appended a couple of unrelated fixes to the xl disk code, only because they also touch check-xl-disk-parse.
PATCH 1/1 libxl: IDL: autogenerate functions to produce JSON from libxl data structures
# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1317312134 -3600
# Node ID 030e844fccebcc3984c5792559c0cbd5424d277a
# Parent fb42038b1f5cd4f72c5cfff6e5f8ffa46f2fa0b2
libxl: IDL: autogenerate functions to produce JSON from libxl data structures.
Two functions are provided. TYPE_gen_json exposes an interface which is compatible with the YAGL generator infrastructure. TYPE_to_string uses this to produce a pretty printed string.
The TYPE_gen_json functions are defined in a new header libxl_json.h which is not exposed via libxl.h due to the use of YAGL datatypes to avoid poluting the namespace us libxl users which don't use the library themselves. If a libxl user is interested in integrating at the YAGL level then it should #include this file itself.
Also update testidl to generate a random version of each IDL datastructure and convert it to JSON. Unfortunately this requires a libxl_ctx and therefore the test must be run on a Xen system now.
tools/libxl/Makefile +6 -3
@@ -84,14 +84,17 @@ _libxl_paths.h: genpath libxl_paths.c: _libxl_paths.h libxl.h: _libxl_types.h +libxl_json.h: _libxl_types_json.h libxl_internal.h: _libxl_types_internal.h +libxl_internal_json.h: _libxl_types_internal_json.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 +_libxl_type%.h _libxl_type%_json.h _libxl_type%.c: libxl_type%.idl gentypes.py libxltypes.py + $(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*_json.h __libxl_type$*.c $(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h) + $(call move-if-changed,__libxl_type$*_json.h,_libxl_type$*_json.h) $(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c) libxenlight.so: libxenlight.so.$(MAJOR)
@@ -140,7 +143,7 @@ install: all ln -sf libxlutil.so.$(XLUMAJOR).$(XLUMINOR) $(DESTDIR)$(LIBDIR)/libxlutil.so.$(XLUMAJOR) ln -sf libxlutil.so.$(XLUMAJOR) $(DESTDIR)$(LIBDIR)/libxlutil.so $(INSTALL_DATA) libxlutil.a $(DESTDIR)$(LIBDIR) - $(INSTALL_DATA) libxl.h _libxl_types.h libxl_utils.h libxl_uuid.h $(DESTDIR)$(INCLUDEDIR) + $(INSTALL_DATA) libxl.h libxl_json.h _libxl_types.h _libxl_types_json.h libxl_utils.h libxl_uuid.h $(DESTDIR)$(INCLUDEDIR) $(INSTALL_DATA) bash-completion $(DESTDIR)$(BASH_COMPLETION_DIR)/xl.sh .PHONY: clean
tools/libxl/gentest.py +229 -5
276 lines not on this page · raw message
tools/libxl/gentypes.py +101 -5
149 lines not on this page · raw message
tools/libxl/idl.txt +9 -0
@@ -49,6 +49,15 @@ Type.autogenerate_destructor: (default: Indicates if the above named Type.destructor_fn should be autogenerated. +Type.json_fn: (default: typename + "_gen_json" or None if type == None) + + The name of the C function which will generate a YAJL data structure + representing this type. + +Type.autogenerate_json: (default: True) + + Indicates if the above named Type.json_fn should be autogenerated. + Other simple type-Classes -------------------------
tools/libxl/libxl.h +2 -2
@@ -199,10 +199,10 @@ typedef struct {int v; } libxl_enum_string_table; +typedef struct libxl__ctx libxl_ctx ; +#include "_libxl_types.h" -typedef struct libxl__ctx libxl_ctx ; -const libxl_version_info *libxl_get_version_info (libxl_ctx *ctx); typedef struct {
tools/libxl/libxl_internal.h +10 -0
@@ -35,7 +35,9 @@#include "flexarray.h" #include "libxl_utils.h" +#include "_libxl_types_internal.h" +#include "libxl_json.h" #define LIBXL_DESTROY_TIMEOUT 10#define LIBXL_DEVICE_MODEL_START_TIMEOUT 10
@@ -337,6 +339,14 @@ _hidden char *libxl__cpupoolid_to_name(l _hidden int libxl__enum_from_string(const libxl_enum_string_table *t,const char *s,int *e); +_hidden yajl_gen_status libxl__yajl_gen_asciiz(yajl_gen hand, const char *str); + +_hidden yajl_gen_status libxl__string_gen_json(yajl_gen hand, const char *p); + +typedef yajl_gen_status (*libxl__gen_json_callback )(yajl_gen hand,void *); +_hidden char *libxl__object_to_json(libxl_ctx *ctx, const char *type, +libxl__gen_json_callback gen,void *p); +/* holds the CPUID response for a single CPUID leaf * input contains the value of the EAX and ECX register, * and each policy string contains a filter to apply to
tools/libxl/libxl_json.c +267 -0
282 lines not on this page · raw message
tools/libxl/libxl_json.h new file +42 -0
42 lines not on this page · raw message
tools/libxl/libxl_types.idl +1 -1
@@ -5,7 +5,7 @@ namespace("libxl_") -libxl_domid = Builtin("domid") +libxl_domid = Builtin("domid", json_fn = "yajl_gen_integer", autogenerate_json = False) libxl_uuid = Builtin("uuid", passby=PASS_BY_REFERENCE) libxl_mac = Builtin("mac", passby=PASS_BY_REFERENCE) libxl_cpumap = Builtin("cpumap", destructor_fn="libxl_cpumap_destroy", passby=PASS_BY_REFERENCE)
tools/libxl/libxltypes.py +18 -2
@@ -50,6 +50,13 @@ class Type(object): self.autogenerate_destructor = kwargs.setdefault('autogenerate_destructor', True) + if self.typename is not None: + self.json_fn = kwargs.setdefault('json_fn', self.typename + "_gen_json") + else: + self.json_fn = kwargs.setdefault('json_fn', None) + + self.autogenerate_json = kwargs.setdefault('autogenerate_json', True) + def marshal_in(self): return self.dir in [DIR_IN, DIR_BOTH] def marshal_out(self):
@@ -83,6 +90,7 @@ class Builtin(Type): def __init__(self, typename, **kwargs): kwargs.setdefault('destructor_fn', None) kwargs.setdefault('autogenerate_destructor', False) + kwargs.setdefault('autogenerate_json', False) Type.__init__(self, typename, **kwargs) class Number(Builtin):
@@ -90,6 +98,7 @@ class Number(Builtin): kwargs.setdefault('namespace', None) kwargs.setdefault('destructor_fn', None) kwargs.setdefault('signed', False) + kwargs.setdefault('json_fn', "yajl_gen_integer") self.signed = kwargs['signed'] Builtin.__init__(self, ctype, **kwargs)
@@ -163,6 +172,8 @@ class Aggregate(Type): comment = None else: n,t,const,comment = f + if n is None: + raise ValueError self.fields.append(Field(t,n,const=const,comment=comment)) # Returns a tuple (stem, field-expr)
@@ -220,7 +231,10 @@ class KeyedUnion(Aggregate): # void = Builtin("void *", namespace = None) -bool = Builtin("bool", namespace = None) +bool = Builtin("bool", namespace = None, + json_fn = "yajl_gen_bool", + autogenerate_json = False) + size_t = Number("size_t", namespace = None) integer = Number("int", namespace = None, signed = True)
@@ -230,7 +244,9 @@ uint16 = UInt(16) uint32 = UInt(32) uint64 = UInt(64) -string = Builtin("char *", namespace = None, destructor_fn = "free") +string = Builtin("char *", namespace = None, destructor_fn = "free", + json_fn = "libxl__string_gen_json", + autogenerate_json = False) class OrderedDict(dict): """A dictionary which remembers insertion order.
Re: [PATCH 1 of 6] libxl: IDL: autogenerate functions to produce JSON from libxl data structures
Ian Campbell writes ("[Xen-devel] [PATCH 1 of 6] libxl: IDL: autogenerate functi> libxl: IDL: autogenerate functions to produce JSON from libxl data structures.
...
Quoted 3 lines
Also update testidl to generate a random version of each IDL datastructure and convert it to JSON. Unfortunately this requires a
libxl_ctxand therefore the test must be run on a Xen system now.
Perhaps we should have a new version of libxl_ctx_alloc which doesn't attempt to connect to xenstore etc., for these kind of purposes. This might turn out to be useful for xl's command line handling too.
Not essential for this patch, though.
Quoted 1 line
+ elif
isinstance(ty, libxltypes.Struct)and (parent is None or ty.json_fn is None):
Do we care about these >80-column lines ?
tools/libxl/gentest.py if __name__ == '__main__': · line 81
Quoted 7 lines
f.write("""#include <
stdio.h>tools/libxl/gentest.py +4
-0+#include <stdlib.h> +#include <string.h> + #include \"libxl.h\" +#include \"libxl_utils.h\"
AIU Python's quoting rules, these \'s are unnecessary. And indeed later we have:
tools/libxl/gentest.py if __name__ == '__main__': · line 98
Quoted 9 lines
tools/libxl/gentest.py +8
-0+ /* A random selection from libxl_cpuid_parse_config */ + {"maxleaf", 32}, +static void rand_bytes(uint8_t *p, size_t sz) +{ + int i; + for (i=0; i<sz; i++) + p[i] = rand() % 256; + //p[i] = i;
This line is leftover cruft ?
tools/libxl/gentypes.py def libxl_C_instance_of(ty, instancename
Quoted 4 lines
diff -r
fb42038b1f5c-r030e844fccebtools/libxl/gentypes.pytools/libxl/gentypes.py +0
-0@@ -29,7 +29,6 @@ def libxl_C_instance_of(ty, instancename
I haven't reviewed this in detail but I have instead looked at the output:
tools/libxl/gentypes.py def libxl_C_instance_of(ty, instancename · line 30
Quoted 2 lines
tools/libxl/gentypes.py +0
-0yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
Are we confident that these functions will never need to take a libxl_ctx ?
Quoted 14 lines
tools/libxl/gentypes.py +0
-0yajl_gen_status libxl_disk_format_gen_json(yajl_gen hand, libxl_disk_format *p) { yajl_gen_status s; { const char *se = libxl_disk_format_to_string(*p); if (se) s = yajl_gen_string(hand, (const unsigned char *)se, strlen(se)); else s = yajl_gen_null(hand); if (s != yajl_gen_status_ok) goto out; } out: return s;
Quoted 1 line
}
There are quite a few functions which all look like this. Perhaps the bulk of this function should be a helper function, so you end up with something like this:
yajl_gen_status libxl_disk_format_gen_json(yajl_gen hand, libxl_disk_format *p)
{
return libxl__yajl_gen_enum(hand, libxl_disk_format_to_string(*p));}
That might reduce the size of the compiled code quite a bit. IME autogenerated code can get very big if one isn't careful.
Quoted 3 lines
diff -r
fb42038b1f5c-r030e844fccebtools/libxl/idl.txttools/libxl/idl.txt +0
-0
...
tools/libxl/idl.txt
Quoted 3 lines
tools/libxl/idl.txt +3
-0+yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand, + libxl_cpuid_policy_list *pcpuid) +{
This rather ad-hoc treatment of cpuid policies isn't ideal I think. Did we want to try to recast them as some more general aggregate ?
Quoted 8 lines
tools/libxl/idl.txt +8
-0+ libxl_cpuid_policy_list cpuid = *pcpuid; + yajl_gen_status s; + const char *input_names[2] = { "leaf", "subleaf" }; + const char *policy_names[4] = { "eax", "ebx", "ecx", "edx" }; + int i, j; + + /* + * Aiming for:
tools/libxl/idl.txt
Quoted 10 lines
tools/libxl/idl.txt +10
-0+ * [ + * { 'leaf': 'val-eax', + * 'subleaf': 'val-edx', + * 'ebx': 'filter', + * 'ecx': 'filter', + * 'edx': 'filter' }, ], + * { 'leaf': 'val-eax', ..., 'eax': 'filter', ... }, + * ... etc ... + * } + */
Mismatched brackets, or confused indenting, in the comment.
Quoted 3 lines
diff -r
fb42038b1f5c-r030e844fccebtools/libxl/libxl_json.htools/libxl/libxl_json.h new file +0
-0
...
tools/libxl/libxl_json.h line 24
Quoted 2 lines
tools/libxl/libxl_json.h new file +2
-0+yajl_gen_status libxl_uuid_gen_json (yajl_gen hand, +libxl_uuid *p);
Shouldn't these declarations be produced automatically by the IDL compiler ? After all it's going to generate calls to these functions.
Ian.
Re: [PATCH 1 of 6] libxl: IDL: autogenerate functions to produce JSON from libxl data structures
Quoted 11 lines · Ian Jackson
Ian Campbell writes ("[Xen-devel] [PATCH 1 of 6] libxl: IDL: autogenerate functi> libxl: IDL: autogenerate functions to produce JSON from libxl data structures.
...Quoted 3 lines
Also update testidl to generate a random version of each IDL datastructure and convert it to JSON. Unfortunately this requires a
libxl_ctxand therefore the test must be run on a Xen system now.Perhaps we should have a new version of
libxl_ctx_allocwhich doesn't attempt to connect to xenstore etc., for these kind of purposes. This might turn out to be useful for xl's command line handling too.Not essential for this patch, though.
I considered adding a new flag to enable this behaviour, but then I realised that libxl_ctx_alloc doesn't have a flag parameter... Perhaps we should add one?
Quoted 4 lines
Quoted 2 lines
+ elif
isinstance(ty, libxltypes.Struct)and (parent is None or ty.json_fn is None):Do we care about these >80-column lines ?
I suppose we should. The IDL stuff isn't totally 80-column clean right now but it is close so I may as well clean it up.
Quoted 11 lines
tools/libxl/gentest.py if __name__ == '__main__': · line 81
Quoted 8 lines
f.write("""#include <
stdio.h>tools/libxl/gentest.py +4
-0+#include <stdlib.h> +#include <string.h> + #include \"libxl.h\" +#include \"libxl_utils.h\"AIU Python's quoting rules, these \'s are unnecessary. And indeed later we have:
Will fix.
Quoted 4 lines
Quoted 3 lines
tools/libxl/gentest.py +2
-0+ /* A random selection from libxl_cpuid_parse_config */ + {"maxleaf", 32},
Quoted 8 lines
tools/libxl/gentest.py if __name__ == '__main__': · line 98
Quoted 6 lines
tools/libxl/gentest.py +6
-0+static void rand_bytes(uint8_t *p, size_t sz) +{ + int i; + for (i=0; i<sz; i++) + p[i] = rand() % 256; + //p[i] = i;This line is leftover cruft ?
Yes.
Quoted 9 lines
tools/libxl/gentypes.py def libxl_C_instance_of(ty, instancename
Quoted 5 lines
diff -r
fb42038b1f5c-r030e844fccebtools/libxl/gentypes.pytools/libxl/gentypes.py +0
-0@@ -29,7 +29,6 @@ def libxl_C_instance_of(ty, instancenameI haven't reviewed this in detail but I have instead looked at the output:
Quoted 5 lines
tools/libxl/gentypes.py def libxl_C_instance_of(ty, instancename · line 30
Quoted 2 lines
tools/libxl/gentypes.py +0
-0yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);Are we confident that these functions will never need to take a
libxl_ctx?
My thinking was that these should be consistent with the interface provided by the equivalent yajl functions (yajl_gen_integer etc).
Partly because otherwise I need to track which types need a ctx and which don't.
I can't think of any reason a ctx would be needed, any allocations made would need to be of the type which could be returned to a caller so they won't be using the gc.
Quoted 21 lines
Quoted 15 lines
tools/libxl/gentypes.py +0
-0yajl_gen_status libxl_disk_format_gen_json(yajl_gen hand, libxl_disk_format *p) { yajl_gen_status s; { const char *se = libxl_disk_format_to_string(*p); if (se) s = yajl_gen_string(hand, (const unsigned char *)se, strlen(se)); else s = yajl_gen_null(hand); if (s != yajl_gen_status_ok) goto out; } out: return s;Quoted 1 line
}
There are quite a few functions which all look like this. Perhaps the bulk of this function should be a helper function, so you end up with something like this:
Quoted 7 lines
yajl_gen_statuslibxl_disk_format_gen_json(yajl_gen hand, libxl_disk_format *p){ return libxl__yajl_gen_enum(hand, libxl_disk_format_to_string(*p));}
That might reduce the size of the compiled code quite a bit. IME autogenerated code can get very big if one isn't careful.
Yes, I'll try this.
Quoted 10 lines
Quoted 3 lines
diff -r
fb42038b1f5c-r030e844fccebtools/libxl/idl.txttools/libxl/idl.txt +0
-0...
tools/libxl/idl.txt
Quoted 3 lines
tools/libxl/idl.txt +3
-0+yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand, + libxl_cpuid_policy_list *pcpuid) +{This rather ad-hoc treatment of cpuid policies isn't ideal I think. Did we want to try to recast them as some more general aggregate ?
We have a smallish number of "builtin" types which really ought to be moved into the IDL, but which are generally complex and need more thought (other than cpuid we have cpumaps, topology info, hwcaps etc).
The cpuid case is interesting because the libxl_cpuid_policy(_list) types are actually opaque typedefs of libxl__cpuid_policy and so are not part of the public IDL. We do now have the internal IDL which could be used and libxl_cpuid_policy_list_gen_json would become a wrapper of sorts for libxl__cpuid_policy_gen_json. Even that is more complex than it sounds since the actual type is currently not using the richer semantic names like "leaf" and "subleaf" or "e*x" but rather has input[2] and policy[4].
I'm not especially keen to tackle all that right now though.
Quoted 19 lines
Quoted 8 lines
tools/libxl/idl.txt +8
-0+ libxl_cpuid_policy_list cpuid = *pcpuid; + yajl_gen_status s; + const char *input_names[2] = { "leaf", "subleaf" }; + const char *policy_names[4] = { "eax", "ebx", "ecx", "edx" }; + int i, j; + + /* + * Aiming for:tools/libxl/idl.txt
Quoted 10 lines
tools/libxl/idl.txt +10
-0+ * [ + * { 'leaf': 'val-eax', + * 'subleaf': 'val-edx', + * 'ebx': 'filter', + * 'ecx': 'filter', + * 'edx': 'filter' }, ], + * { 'leaf': 'val-eax', ..., 'eax': 'filter', ... }, + * ... etc ... + * } + */
Quoted 1 line
Mismatched brackets, or confused indenting, in the comment.
Will fix. Once I figure out what I meant...
Quoted 9 lines
Quoted 3 lines
diff -r
fb42038b1f5c-r030e844fccebtools/libxl/libxl_json.htools/libxl/libxl_json.h new file +0
-0...
tools/libxl/libxl_json.h line 24
Quoted 2 lines
tools/libxl/libxl_json.h new file +2
-0+yajl_gen_status libxl_uuid_gen_json (yajl_gen hand, +libxl_uuid *p);Shouldn't these declarations be produced automatically by the IDL compiler ? After all it's going to generate calls to these functions.
It's somewhat consistent with how we deal with these "builtin" types in other places (and simpler in gentypes.py) to declare these by hand. I'll see if I can make it work though.
Thanks for the review.
Ian.
Quoted 2 lines
Ian.
PATCH 1/1 xl: allow check-xl-disk-parse to run against installed xl as well as build dir
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1317312717 -3600
# Node ID 3fb003a5d5367ccdfb969fd786b57ef2e103f289
# Parent 030e844fccebcc3984c5792559c0cbd5424d277a
xl: allow check-xl-disk-parse to run against installed xl as well as build dir
I can't run from the current directory since my build box isn't running Xen so if ./xl doesn't exist use the installed version on the assumption that I've copied the script to a test host.
I think running from the build dir needs the blktap2 libraries, so update LD_LIBRARY_PATH as appropriate.
tools/libxl/check-xl-disk-parse +8 -2
@@ -2,6 +2,13 @@ set -e +if [ -x ./xl ] ; then + export LD_LIBRARY_PATH=.:../libxc:../xenstore:../blktap2/control + XL=./xl +else + XL=/usr/sbin/xl +fi + fprefix=tmp.check-xl-disk-parse expected () {
@@ -14,8 +21,7 @@ one () { expected_rc=$1; shift printf "test case %s...\n" "$*" set +e - LD_LIBRARY_PATH=.:../libxc:../xenstore \ - ./xl -N block-attach 0 "$@" </dev/null >$fprefix.actual 2>/dev/null + ${XL} -N block-attach 0 "$@" </dev/null >$fprefix.actual 2>/dev/null actual_rc=$? diff -u $fprefix.expected $fprefix.actual diff_rc=$?
Re: [PATCH 2 of 6] xl: allow check-xl-disk-parse to run against installed xl as well as build dir
Ian Campbell writes ("[Xen-devel] [PATCH 2 of 6] xl: allow check-xl-disk-parse to run against installed xl as well as build dir"):
Quoted 1 line
xl: allow check-xl-disk-parse to run against installed xl as well as build dir
This looks good except:
Quoted 1 line
+ XL=/usr/sbin/xl
XL=xl surely ?
Ian.
Re: [PATCH 2 of 6] xl: allow check-xl-disk-parse to run against installed xl as well as build dir
Quoted 8 lines · Ian Jackson
Ian Campbell writes ("[Xen-devel] [PATCH 2 of 6] xl: allow check-xl-disk-parse to run against installed xl as well as build dir"):
Quoted 1 line
xl: allow check-xl-disk-parse to run against installed xl as well as build dir
This looks good except:
Quoted 1 line
+ XL=/usr/sbin/xl
XL=xl surely ?
If you prefer, sure.
Ian.
PATCH 1/1 xl: use libxl_device_disk_to_json to pretty print disk configuration
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1317312915 -3600
# Node ID 6c2b62f0452a73811f708f162490cf7c1c247295
# Parent 3fb003a5d5367ccdfb969fd786b57ef2e103f289
xl: use libxl_device_disk_to_json to pretty print disk configuration
tools/libxl/check-xl-disk-parse +24 -18
@@ -51,15 +51,18 @@ expected </dev/null one $e foo expected <<END -disk.backend_domid = 0 -disk.pdev_path = /dev/vg/guest-volume -disk.vdev = hda -disk.backend = 0 -disk.format = 4 -disk.script = (null) -disk.removable = 0 -disk.readwrite = 1 -disk.is_cdrom = 0 +disk: { + "backend_domid": 0, + "pdev_path": "/dev/vg/guest-volume", + "vdev": "hda", + "backend": "unknown", + "format": "raw", + "script": null, + "removable": 0, + "readwrite": 1, + "is_cdrom": 0 +} + END one 0 /dev/vg/guest-volume,,hda one 0 /dev/vg/guest-volume,raw,hda,rw
@@ -68,15 +71,18 @@ one 0 format=raw vdev=hda access=rw one 0 raw:/dev/vg/guest-volume,hda,w expected <<END -disk.backend_domid = 0 -disk.pdev_path = /root/image.iso -disk.vdev = hdc -disk.backend = 0 -disk.format = 4 -disk.script = (null) -disk.removable = 1 -disk.readwrite = 0 -disk.is_cdrom = 1 +disk: { + "backend_domid": 0, + "pdev_path": "/root/image.iso", + "vdev": "hdc", + "backend": "unknown", + "format": "raw", + "script": null, + "removable": 1, + "readwrite": 0, + "is_cdrom": 1 +} + END one 0 /root/image.iso,,hdc,cdrom one 0 /root/image.iso,,hdc,,cdrom
tools/libxl/xl_cmdimpl.c +3 -11
@@ -4109,17 +4109,9 @@ int main_blockattach(int argc, char **ar disk.backend_domid = be_domid;if (dryrun_only) { -/* fixme: this should be generated from the idl */ -/* fixme: enums (backend, format) should be converted to strings */ -printf ("disk.backend_domid = %" PRIx32"\n" , disk.backend_domid); -printf ("disk.pdev_path = %s\n" , disk.pdev_path); -printf ("disk.vdev = %s\n" , disk.vdev); -printf ("disk.backend = %d\n" , disk.backend); -printf ("disk.format = %d\n" , disk.format); -printf ("disk.script = %s\n" , disk.script); -printf ("disk.removable = %d\n" , disk.removable); -printf ("disk.readwrite = %d\n" , disk.readwrite); -printf ("disk.is_cdrom = %d\n" , disk.is_cdrom); +char *json =libxl_device_disk_to_json (ctx, &disk); +printf ("disk: %s\n" , json); +free (json);if (ferror (stdout) ||fflush (stdout)) {perror ("stdout" );exit (-1 ); }return 0 ; }
Re: [PATCH 3 of 6] xl: use libxl_device_disk_to_json to pretty print disk configuration
Ian Campbell writes ("[Xen-devel] [PATCH 3 of 6] xl: use libxl_device_disk_to_js> xl: use libxl_device_disk_to_json to pretty print disk configuration
Quoted 2 lines
Signed-off-by Ian Campbell <[email protected]>
This one looks good to me and I'll apply it as part of the rest of the series when the time comes.
Ian.
PATCH 1/1 libxl: undo 23728:548b2826293e whitespace cleanup to autogenerated file
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]>
# Date 1317312932 -3600
# Node ID 6056b382a44fd94ead9523a098855830c400ee54
# Parent 6c2b62f0452a73811f708f162490cf7c1c247295
libxl: undo 23728:548b2826293e whitespace cleanup to autogenerated file
tools/libxl/libxlu_disk_l.c +26 -26
@@ -34,7 +34,7 @@#if defined (__STDC_VERSION__) && __STDC_VERSION__ >=199901L /* C99 says to define __STDC_LIMIT_MACROS before including stdint.h, -* if you want the limit (max/min) macros for int types. +* if you want the limit (max/min) macros for int types. */ #ifndef __STDC_LIMIT_MACROS#define __STDC_LIMIT_MACROS 1
@@ -51,7 +51,7 @@ typedef uint32_t flex_uint32_t;typedef signed char flex_int8_t ;typedef short int flex_int16_t ;typedef int flex_int32_t ; -typedef unsigned char flex_uint8_t ; +typedef unsigned char flex_uint8_t ;typedef unsigned short int flex_uint16_t ;typedef unsigned int flex_uint32_t ;
@@ -184,7 +184,7 @@ typedef struct yy_buffer_state *YY_BUFFE#define EOB_ACT_LAST_MATCH 2#define YY_LESS_LINENO (n) - +/* Return all but the first "n" matched characters back to the input stream. */ #define yyless (n) \ do \
@@ -246,7 +246,7 @@ struct yy_buffer_stateint yy_bs_lineno;/**< The line count. */ int yy_bs_column;/**< The column count. */ - +/* Whether to try to fill the input buffer when we reach the * end of it. */
@@ -1869,7 +1869,7 @@ static void xlu__disk_yy_load_buffer_staYY_BUFFER_STATE xlu__disk_yy_create_buffer (FILE * file,int size ,yyscan_t yyscanner) {YY_BUFFER_STATE b; - + b = (YY_BUFFER_STATE )xlu__disk_yyalloc (sizeof (struct yy_buffer_state ) ,yyscanner );if ( ! b )YY_FATAL_ERROR ("out of dynamic memory in xlu__disk_yy_create_buffer()" );
@@ -1913,7 +1913,7 @@ static void xlu__disk_yy_load_buffer_sta#ifndef __cplusplusextern int isatty (int );#endif /* __cplusplus */ - +/* Initializes or reinitializes a buffer. * This function is sometimes called more than once on the same buffer, * such as during a xlu__disk_yyrestart() or at EOF.
@@ -1939,7 +1939,7 @@ extern int isatty (int ); } b->yy_is_interactive = file ? (isatty (fileno (file) ) >0 ) :0 ; - + errno = oerrno; }
@@ -2045,9 +2045,9 @@ static void xlu__disk_yyensure_buffer_st , yyscanner);if ( ! yyg->yy_buffer_stack )YY_FATAL_ERROR ("out of dynamic memory in xlu__disk_yyensure_buffer_stack()" ); - +memset (yyg->yy_buffer_stack,0 , num_to_alloc *sizeof (struct yy_buffer_state *)); - + yyg->yy_buffer_stack_max = num_to_alloc; yyg->yy_buffer_stack_top =0 ;return ;
@@ -2076,12 +2076,12 @@ static void xlu__disk_yyensure_buffer_st* @param base the character buffer * @param size the size in bytes of the character buffer * @param yyscanner The scanner object. -* @return the newly allocated buffer state object. +* @return the newly allocated buffer state object. */ YY_BUFFER_STATE xlu__disk_yy_scan_buffer (char * base,yy_size_t size ,yyscan_t yyscanner) {YY_BUFFER_STATE b; - +if ( size <2 || base[size-2 ] !=YY_END_OF_BUFFER_CHAR || base[size-1 ] !=YY_END_OF_BUFFER_CHAR )
@@ -2117,7 +2117,7 @@ YY_BUFFER_STATE xlu__disk_yy_scan_buffer*/ YY_BUFFER_STATE xlu__disk_yy_scan_string (yyconst char * yystr , yyscan_t yyscanner) { - +return xlu__disk_yy_scan_bytes (yystr,strlen (yystr) ,yyscanner); }
@@ -2134,7 +2134,7 @@ YY_BUFFER_STATE xlu__disk_yy_scan_byteschar *buf;yy_size_t n;int i; - +/* Get memory for full buffer, including space for trailing EOB's. */ n = _yybytes_len +2 ; buf = (char *)xlu__disk_yyalloc (n ,yyscanner );
@@ -2202,10 +2202,10 @@ YY_EXTRA_TYPE xlu__disk_yyget_extra (yyint xlu__disk_yyget_lineno (yyscan_t yyscanner) {struct yyguts_t * yyg = (struct yyguts_t *)yyscanner; - +if (!YY_CURRENT_BUFFER )return 0 ; - +return yylineno; }
@@ -2215,10 +2215,10 @@ int xlu__disk_yyget_lineno (yyscan_t yyint xlu__disk_yyget_column (yyscan_t yyscanner) {struct yyguts_t * yyg = (struct yyguts_t *)yyscanner; - +if (!YY_CURRENT_BUFFER )return 0 ; - +return yycolumn; }
@@ -2279,8 +2279,8 @@ void xlu__disk_yyset_lineno (int line_n/* lineno is only valid if an input buffer exists. */ if (!YY_CURRENT_BUFFER ) -yy_fatal_error ("xlu__disk_yyset_lineno called with no buffer" , yyscanner); - +yy_fatal_error ("xlu__disk_yyset_lineno called with no buffer" , yyscanner); + yylineno = line_number; }
@@ -2294,8 +2294,8 @@ void xlu__disk_yyset_column (int column/* column is only valid if an input buffer exists. */ if (!YY_CURRENT_BUFFER ) -yy_fatal_error ("xlu__disk_yyset_column called with no buffer" , yyscanner); - +yy_fatal_error ("xlu__disk_yyset_column called with no buffer" , yyscanner); + yycolumn = column_no; }
@@ -2378,20 +2378,20 @@ int xlu__disk_yylex_init_extra(YY_EXTRA_ errno =EINVAL ;return 1 ; } - + *ptr_yy_globals = (yyscan_t )xlu__disk_yyalloc (sizeof (struct yyguts_t ), &dummy_yyguts ); - +if (*ptr_yy_globals ==NULL ){ errno =ENOMEM ;return 1 ; } - +/* By setting to 0xAA, we expose bugs in yy_init_globals. Leave at 0x00 for releases. */ memset (*ptr_yy_globals,0x00 ,sizeof (struct yyguts_t )); - +xlu__disk_yyset_extra (yy_user_defined, *ptr_yy_globals); - +return yy_init_globals ( *ptr_yy_globals ); }
tools/libxl/libxlu_disk_l.h +3 -3
@@ -38,7 +38,7 @@#if defined (__STDC_VERSION__) && __STDC_VERSION__ >=199901L /* C99 says to define __STDC_LIMIT_MACROS before including stdint.h, -* if you want the limit (max/min) macros for int types. +* if you want the limit (max/min) macros for int types. */ #ifndef __STDC_LIMIT_MACROS#define __STDC_LIMIT_MACROS 1
@@ -55,7 +55,7 @@ typedef uint32_t flex_uint32_t;typedef signed char flex_int8_t ;typedef short int flex_int16_t ;typedef int flex_int32_t ; -typedef unsigned char flex_uint8_t ; +typedef unsigned char flex_uint8_t ;typedef unsigned short int flex_uint16_t ;typedef unsigned int flex_uint32_t ;
@@ -193,7 +193,7 @@ struct yy_buffer_stateint yy_bs_lineno;/**< The line count. */ int yy_bs_column;/**< The column count. */ - +/* Whether to try to fill the input buffer when we reach the * end of it. */
Re: [PATCH 4 of 6] libxl: undo 23728:548b2826293e whitespace cleanup to autogenerated file
Ian Campbell writes ("[Xen-devel] [PATCH 4 of 6] libxl: undo 23728:548b2826293e whitespace cleanup to autogenerated file"):
Quoted 3 lines
libxl: undo 23728:548b2826293e whitespace cleanup to autogenerated file
Signed-off-by Ian Campbell <[email protected]>
There seemed no reason to wait with this one.
Ian.
PATCH 1/1 libxlu: correctly parse disk "backendtype" field
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1317313034 -3600
# Node ID 50cd0fd187b39a263680d8a4b4f1a8511c7c04ee # Parent 6056b382a44fd94ead9523a098855830c400ee54
libxlu: correctly parse disk "backendtype" field
Currently it tries to parse the value from the full "backendtype=FOO" string but really it needs to parse from the equals.
Before:
# xl -N block-attach d32-1 backendtype=phy,vdev=xvdb,access=w,target=/dev/VG/debian-x86_32-1b command line: config parsing error in disk specification: unknown value for backendtype: near `backendtype=phy' in `backendtype=phy,vdev=xvdb,access=w,target=/dev/VG/debian-x86_32-1b'
After:
# xl -N block-attach d32-1 backendtype=phy,vdev=xvdb,access=w,target=/dev/VG/debian-x86_32-1b
disk: {
"backend_domid": 0,
"pdev_path": "/dev/VG/debian-x86_32-1b",
"vdev": "xvdb",
"backend": "phy",
"format": "raw",
"script": null,
"removable": 0,
"readwrite": 1,
"is_cdrom": 0
}tools/libxl/check-xl-disk-parse +16 -0
@@ -91,4 +91,20 @@ one 0 "format=raw, vdev=hdc, access=ro, one 0 format=raw vdev=hdc access=ro devtype=cdrom target=/root/image.iso one 0 raw:/root/image.iso,hdc:cdrom,ro +expected <<EOF +disk: { + "backend_domid": 0, + "pdev_path": "/dev/vg/guest-volume", + "vdev": "xvdb", + "backend": "phy", + "format": "raw", + "script": null, + "removable": 0, + "readwrite": 1, + "is_cdrom": 0 +} + +EOF +one 0 backendtype=phy,vdev=xvdb,access=w,target=/dev/vg/guest-volume + complete
tools/libxl/libxlu_disk_l.c +1 -1
@@ -1261,7 +1261,7 @@ case 8:/* rule 8 can match eol */ YY_RULE_SETUP#line 142 "libxlu_disk_l.l" -{STRIP (',' );setbackendtype (DPC ,yytext); } +{STRIP (',' );setbackendtype (DPC ,FROMEQUALS ); }YY_BREAK case 9:/* rule 9 can match eol */
tools/libxl/libxlu_disk_l.l +1 -1
@@ -139,7 +139,7 @@ devtype=disk,? { DPC->disk->is_cdrom = 0 devtype=[^,]*,? { xlu__disk_err(DPC,yytext,"unknown value for type"); } access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); } -backendtype=[^,]*? { STRIP(','); setbackendtype(DPC,yytext); } +backendtype=[^,]*? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); } script=[^,]*,? { STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
Re: [PATCH 5 of 6] libxlu: correctly parse disk "backendtype" field
Ian Campbell writes ("[Xen-devel] [PATCH 5 of 6] libxlu: correctly parse disk "b> libxlu: correctly parse disk "backendtype" field
Quoted 3 lines
Currently it tries to parse the value from the full "backendtype=FOO" string but really it needs to parse from the equals.
I have applioed all of this apart from the test case, which is in the pre-json format, right away, since it's a bugfix. Please retain the test case hunk in your series.
Thanks,
Ian.
PATCH 1/1 libxlu: correctly parse disk "backendtype" field
In reply to Ian CampbellThe expression for backendtype was missing a comma.
tools/libxl/libxlu_disk_l.l +1 -1
@@ -139,7 +139,7 @@ devtype=disk,? { DPC->disk->is_cdrom = 0; } devtype=[^,]*,? { xlu__disk_err(DPC,yytext,"unknown value for type"); } access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); } -backendtype=[^,]*? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } +backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); } script=[^,]*,? { STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
Re: [PATCH] libxlu: correctly parse disk "backendtype" field
Quoted 1 line · Daniel De Graaf
The expression for backendtype was missing a comma.
This appears correct even though things seem to work without, at least:
xl -N block-attach 0 backendtype=phy,vdev=xvdb,access=w,target=/dev/vg/guest-volume works as expected. Do you have a test case which fails or did you just find by code inspection?
(we need a signed-off-by I suspect)
Quoted 12 lines
tools/libxl/libxlu_disk_l.l +1
-1@@ -139,7 +139,7 @@ devtype=disk,? { DPC->disk->is_cdrom = 0; } devtype=[^,]*,? { xlu__disk_err(DPC,yytext,"unknown value for type"); } access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); } -backendtype=[^,]*? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } +backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); } script=[^,]*,? { STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
Re: [PATCH] libxlu: correctly parse disk "backendtype" field
Quoted 12 lines · Ian Campbell
Quoted 1 line · Daniel De Graaf
The expression for backendtype was missing a comma.
This appears correct even though things seem to work without, at least:
xl -N block-attach 0 backendtype=phy,vdev=xvdb,access=w,target=/dev/vg/guest-volume works as expected. Do you have a test case which fails or did you just find by code inspection?Acked-by Ian Campbell <[email protected]>(we need a signed-off-by I suspect)
Found when xl was parsing a configuration file with the disk line:
disk = [ 'backendtype=phy,backend=3,vdev=xvda,access=w,target=/dev/clam/vm1' ]
This is after adding another patch allowing backend domain ID for disk/vif to be changed in the config, so it may or may not show up on its own; I didn't test the new syntax until I needed it.
Quoted 13 lines
Quoted 12 lines
tools/libxl/libxlu_disk_l.l +1
-1@@ -139,7 +139,7 @@ devtype=disk,? { DPC->disk->is_cdrom = 0; } devtype=[^,]*,? { xlu__disk_err(DPC,yytext,"unknown value for type"); } access=[^,]*,? { STRIP(','); setaccess(DPC, FROMEQUALS); } -backendtype=[^,]*? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } +backendtype=[^,]*,? { STRIP(','); setbackendtype(DPC,FROMEQUALS); } vdev=[^,]*,? { STRIP(','); SAVESTRING("vdev", vdev, FROMEQUALS); } script=[^,]*,? { STRIP(','); SAVESTRING("script", script, FROMEQUALS); }
Ian Jackson
Acked-byRe: [PATCH] libxlu: correctly parse disk "backendtype" field
Daniel De Graaf writes ("Re: [PATCH] libxlu: correctly parse disk "backendtype" field"):
Quoted 3 lines · Ian Campbell
Quoted 2 lines · Daniel De Graaf
Quoted 1 line
The expression for backendtype was missing a comma.
Good catch, thanks.
Quoted 1 line
Quoted 1 line
This appears correct even though things seem to work without, at least:
Daniel's fix is correct. Without the ",?", the "backendtype=..." match would not eat the trailing comma. The comma would then be matched by the positional parameter rule, with effects ranging from unhelpful to fatal. Ie, it would parse as if you'd written
backendtype=<something>,,vdev=<blah>
etc.
Quoted 1 line
Quoted 1 line
(we need a signed-off-by I suspect)
Thanks, yes.
Quoted 1 line
Signed-off-by Daniel De Graaf <[email protected]>
Ian.
PATCH 1/1 libxl: probe disk backend type in libxl_device_disk_add
In reply to Ian Campbell# HG changeset patch
# User Ian Campbell <[email protected]> # Date 1317313222 -3600
# Node ID 966d33c61d0b1dcf8545253b198e185c3757bc65 # Parent 50cd0fd187b39a263680d8a4b4f1a8511c7c04ee
libxl: probe disk backend type in libxl_device_disk_add
Without this "xl block-attach" does not work. On create do_domain_create already catches this.
tools/libxl/libxl.c +3 -0
@@ -926,6 +926,9 @@ int libxl_device_disk_add(libxl_ctx *ctxlibxl__device device;int major, minor, rc; + rc =libxl__device_disk_set_backend (&gc, disk); +if (rc) goto out; + front =flexarray_make (16 ,1 );if (!front) { rc =ERROR_NOMEM ;