| Message ID | 20130326140139.4471.51043.stgit@fimbulvetr.bsc.es |
|---|---|
| State | New |
| Headers | show |
On 03/26/2013 08:01 AM, Lluís Vilanova wrote: > Add QMP commands to control (un)loading of dynamic instrumentation library. > > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> > --- > include/qapi/qmp/qerror.h | 9 +++++ > instrument/Makefile.objs | 1 + > instrument/qapi-schema.json | 33 ++++++++++++++++++++ > instrument/qmp.c | 59 ++++++++++++++++++++++++++++++++++++ > qapi-schema.json | 2 + > qmp-commands.hx | 71 +++++++++++++++++++++++++++++++++++++++++++ > qmp.c | 4 ++ > 7 files changed, 179 insertions(+) > create mode 100644 instrument/qapi-schema.json > create mode 100644 instrument/qmp.c > > +++ b/instrument/qapi-schema.json > @@ -0,0 +1,33 @@ > +# *-*- Mode: Python -*-* > + > +## > +# @instr-dynamic: > +# > +# Whether dynamic trace instrumentation is available. > +# > +# Since: 1.4 1.5 > +## > +{ 'command': 'instr-dynamic', > + 'returns': 'bool' } > + > +## > +# @instr-load: > +# > +# Load a dynamic instrumentation library. > +# > +# @path: path to the dynamic instrumentation library > +# @args: arguments to the dynamic instrumentation library No trailing underscore here... > +# > +# Since: 1.4 1.5 > +## > +{ 'command': 'instr-load', > + 'data': { 'path': 'str', 'args_': 'str' } } ...but there is one here. Something is wrong. Furthermore, this is gross - how does the receiver know how to break args into chunks? This should be '*args':['str'], taking an optional array of args (omitting is the same as a 0-length array). > +## > +# @instr-unload: > +# > +# Unload the current dynamic instrumentation library. > +# > +# Since: 1.4 1.5 > +## > +{ 'command': 'instr-unload' } Is it possible to load more than one library at a time? If so, then instr-load needs to return a handle, and instr-unload needs to take a handle as an argument. Also, a query-instr command might be useful for showing which library (or libraries) are loaded. > +++ b/instrument/qmp.c > @@ -0,0 +1,59 @@ > +/* > + * QMP interface for dynamic trace instrumentation control commands. > + * > + * Copyright (C) 2012 Lluís Vilanova <vilanova@ac.upc.edu> It's 2013, now. > +++ b/qapi-schema.json > @@ -2360,6 +2360,8 @@ > ## > { 'command': 'device_del', 'data': {'id': 'str'} } > > +input("instrument/qapi-schema.json") > + > ## > # @dump-guest-memory Unusual placement, having it in the middle of the file in no particular alphabetic ordering. I would typically expect to see includes done at the beginning, or maybe at the end, or at least in alphabetical order where the 'instr-*' commands would fall (but the file is already messy, so not entirely your fault). > +instr-load > +---------- > + > +Load a dynamic instrumentation library. > + > +Arguments: > + > +- path: path to the dynamic instrumentation library Missing out on args documentation.
Eric Blake writes: > On 03/26/2013 08:01 AM, Lluís Vilanova wrote: >> Add QMP commands to control (un)loading of dynamic instrumentation library. >> >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> >> --- >> include/qapi/qmp/qerror.h | 9 +++++ >> instrument/Makefile.objs | 1 + >> instrument/qapi-schema.json | 33 ++++++++++++++++++++ >> instrument/qmp.c | 59 ++++++++++++++++++++++++++++++++++++ >> qapi-schema.json | 2 + >> qmp-commands.hx | 71 +++++++++++++++++++++++++++++++++++++++++++ >> qmp.c | 4 ++ >> 7 files changed, 179 insertions(+) >> create mode 100644 instrument/qapi-schema.json >> create mode 100644 instrument/qmp.c >> >> +++ b/instrument/qapi-schema.json >> @@ -0,0 +1,33 @@ >> +# *-*- Mode: Python -*-* >> + >> +## >> +# @instr-dynamic: >> +# >> +# Whether dynamic trace instrumentation is available. >> +# >> +# Since: 1.4 > 1.5 >> +## >> +{ 'command': 'instr-dynamic', >> + 'returns': 'bool' } >> + >> +## >> +# @instr-load: >> +# >> +# Load a dynamic instrumentation library. >> +# >> +# @path: path to the dynamic instrumentation library >> +# @args: arguments to the dynamic instrumentation library > No trailing underscore here... >> +# >> +# Since: 1.4 > 1.5 >> +## >> +{ 'command': 'instr-load', >> + 'data': { 'path': 'str', 'args_': 'str' } } > ...but there is one here. Something is wrong. Maybe there was a problem with using the word "args"? I honestly do not remember the reason for this, so I'll just remove the underscore. > Furthermore, this is > gross - how does the receiver know how to break args into chunks? This > should be '*args':['str'], taking an optional array of args (omitting is > the same as a 0-length array). Ha! Looks like I was really lazy when I implemented this... I'll see if I can find some time to properly implement it and pass an array of strings to the instrumentation library. >> +## >> +# @instr-unload: >> +# >> +# Unload the current dynamic instrumentation library. >> +# >> +# Since: 1.4 > 1.5 >> +## >> +{ 'command': 'instr-unload' } > Is it possible to load more than one library at a time? If so, then > instr-load needs to return a handle, and instr-unload needs to take a > handle as an argument. Also, a query-instr command might be useful for > showing which library (or libraries) are loaded. It's not. Right now you'll get an error if that happens. The rationale behind this is that for multiple libraries to be supported you would need to implement an instrumentation "driver" that loops through a list of callbacks, which is slower than just calling into a single function pointer. I actually thought about writing an "auto-optimizing" version. The idea was to assign the client's callback to the per-event pointer if only one library requested that event, and otherwise assign a pointer to a per-event auto-generated routine that loops through all assigned callbacks when multiple clients have registered for that event. But this looked like too much trouble for an initial implementation. Besides, a single top-level instrumentation library could then provide an interface to load multiple "child" libraries, if that is necessary. >> +++ b/instrument/qmp.c >> @@ -0,0 +1,59 @@ >> +/* >> + * QMP interface for dynamic trace instrumentation control commands. >> + * >> + * Copyright (C) 2012 Lluís Vilanova <vilanova@ac.upc.edu> > It's 2013, now. >> +++ b/qapi-schema.json >> @@ -2360,6 +2360,8 @@ >> ## >> { 'command': 'device_del', 'data': {'id': 'str'} } >> >> +input("instrument/qapi-schema.json") >> + >> ## >> # @dump-guest-memory > Unusual placement, having it in the middle of the file in no particular > alphabetic ordering. I would typically expect to see includes done at > the beginning, or maybe at the end, or at least in alphabetical order > where the 'instr-*' commands would fall (but the file is already messy, > so not entirely your fault). This is probably the result of merges, or that I just changed the instrumentation command names and did not update the position; thanks. BTW, I think that this "input" primitive should be used wherever possible to split the files by subsystem and also have each part in the directory where the implementation resides. >> +instr-load >> +---------- >> + >> +Load a dynamic instrumentation library. >> + >> +Arguments: >> + >> +- path: path to the dynamic instrumentation library > Missing out on args documentation. Is this one also able to handle a list of strings for "args"? If so, then I'm just missing how to expose it in the command-line (maybe as an "accumulable" argument), and we'll be set for having proper argument support. Thanks, Lluis
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index 6c0a18d..67b4528 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -129,6 +129,15 @@ void assert_no_error(Error *err); #define QERR_FEATURE_DISABLED \ ERROR_CLASS_GENERIC_ERROR, "The feature '%s' is not enabled" +#define QERR_INSTR_LOAD_DL \ + ERROR_CLASS_GENERIC_ERROR, "Error loading dynamic library: %s" + +#define QERR_INSTR_LOAD_LOADED \ + ERROR_CLASS_GENERIC_ERROR, "Already loaded" + +#define QERR_INSTR_LOAD_UNLOADED \ + ERROR_CLASS_GENERIC_ERROR, "Already unloaded" + #define QERR_INVALID_BLOCK_FORMAT \ ERROR_CLASS_GENERIC_ERROR, "Invalid block format '%s'" diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs index adae4a9..ad9ca6d 100644 --- a/instrument/Makefile.objs +++ b/instrument/Makefile.objs @@ -67,3 +67,4 @@ endif target-obj-y += control.o common-obj-$(CONFIG_SOFTMMU) += hmp.o +common-obj-$(CONFIG_SOFTMMU) += qmp.o diff --git a/instrument/qapi-schema.json b/instrument/qapi-schema.json new file mode 100644 index 0000000..bf970f6 --- /dev/null +++ b/instrument/qapi-schema.json @@ -0,0 +1,33 @@ +# *-*- Mode: Python -*-* + +## +# @instr-dynamic: +# +# Whether dynamic trace instrumentation is available. +# +# Since: 1.4 +## +{ 'command': 'instr-dynamic', + 'returns': 'bool' } + +## +# @instr-load: +# +# Load a dynamic instrumentation library. +# +# @path: path to the dynamic instrumentation library +# @args: arguments to the dynamic instrumentation library +# +# Since: 1.4 +## +{ 'command': 'instr-load', + 'data': { 'path': 'str', 'args_': 'str' } } + +## +# @instr-unload: +# +# Unload the current dynamic instrumentation library. +# +# Since: 1.4 +## +{ 'command': 'instr-unload' } diff --git a/instrument/qmp.c b/instrument/qmp.c new file mode 100644 index 0000000..2c369aa --- /dev/null +++ b/instrument/qmp.c @@ -0,0 +1,59 @@ +/* + * QMP interface for dynamic trace instrumentation control commands. + * + * Copyright (C) 2012 Lluís Vilanova <vilanova@ac.upc.edu> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu-common.h" +#include "qapi/qmp/qerror.h" +#include "qmp-commands.h" + +#include <dlfcn.h> + +#include "instrument/control.h" + + + +bool qmp_instr_dynamic(Error **errp) +{ + return instr_dynamic(); +} + +void qmp_instr_load(const char * path, const char *args, Error **errp) +{ + InstrLoadError err = instr_load(path, args); + switch (err) { + case INSTR_LOAD_OK: + break; + case INSTR_LOAD_UNAVAILABLE: + error_set(errp, QERR_UNSUPPORTED); + break; + case INSTR_LOAD_LOADED: + error_set(errp, QERR_INSTR_LOAD_LOADED); + break; + case INSTR_LOAD_DL: + error_set(errp, QERR_INSTR_LOAD_DL, dlerror()); + break; + } +} + +void qmp_instr_unload(Error **errp) +{ + InstrLoadError err = instr_unload(); + switch (err) { + case INSTR_UNLOAD_OK: + break; + case INSTR_UNLOAD_UNAVAILABLE: + error_set(errp, QERR_UNSUPPORTED); + break; + case INSTR_UNLOAD_UNLOADED: + error_set(errp, QERR_INSTR_LOAD_UNLOADED); + break; + case INSTR_UNLOAD_DL: + error_set(errp, QERR_INSTR_LOAD_DL, dlerror()); + break; + } +} diff --git a/qapi-schema.json b/qapi-schema.json index 28b070f..1c8ba7d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2360,6 +2360,8 @@ ## { 'command': 'device_del', 'data': {'id': 'str'} } +input("instrument/qapi-schema.json") + ## # @dump-guest-memory # diff --git a/qmp-commands.hx b/qmp-commands.hx index 799adea..f704221 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1488,6 +1488,77 @@ Notes: o Commands that prompt the user for data (eg. 'cont' when the block device is encrypted) don't currently work +instr-dynamic +------------- + +Whether dynamic trace instrumentation is available. + +Arguments: None. + +Example: + +-> { "execute": "instr-dyanmic" } +<- { "return": true } + +EQMP + + { + .name = "instr-dynamic", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_instr_dynamic, + }, + + +SQMP + +instr-load +---------- + +Load a dynamic instrumentation library. + +Arguments: + +- path: path to the dynamic instrumentation library + +Example: + +-> { "execute": "instr-load", "arguments": { "path": "/tmp/libtrace-instrument.so" } } +<- { "return": {} } + +EQMP + + { + .name = "instr-load", + .args_type = "path:s", + .params = "path", + .mhandler.cmd_new = qmp_marshal_input_instr_load, + }, + + +SQMP + +instr-unload +------------ + +Unload the current dynamic instrumentation library. + +Arguments: None. + +Example: + +-> { "execute": "instr-unload" } +<- { "return": {} } + +EQMP + + { + .name = "instr-unload", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_instr_unload, + }, + + +SQMP 3. Query Commands ================= diff --git a/qmp.c b/qmp.c index 55b056b..72abe03 100644 --- a/qmp.c +++ b/qmp.c @@ -24,6 +24,10 @@ #include "hw/qdev.h" #include "sysemu/blockdev.h" #include "qom/qom-qobject.h" +#if defined(TRACE_INSTRUMENT_DYNAMIC) +#include "instrument/qi-qmp-commands.h" +#endif + NameInfo *qmp_query_name(Error **errp) {
Add QMP commands to control (un)loading of dynamic instrumentation library. Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> --- include/qapi/qmp/qerror.h | 9 +++++ instrument/Makefile.objs | 1 + instrument/qapi-schema.json | 33 ++++++++++++++++++++ instrument/qmp.c | 59 ++++++++++++++++++++++++++++++++++++ qapi-schema.json | 2 + qmp-commands.hx | 71 +++++++++++++++++++++++++++++++++++++++++++ qmp.c | 4 ++ 7 files changed, 179 insertions(+) create mode 100644 instrument/qapi-schema.json create mode 100644 instrument/qmp.c