| Message ID | 150472050004.24907.4613952373869620144.stgit@frigg.lan |
|---|---|
| State | New |
| Headers | show |
| Series | instrument: Add basic event instrumentation | expand |
Lluís Vilanova <vilanova@ac.upc.edu> writes: > Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> > --- > hmp-commands.hx | 28 ++++++++++++++++++++++++++ > monitor.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 1941e19932..703d7262f5 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1858,6 +1858,34 @@ ETEXI > .sub_table = info_cmds, > }, > > + { > + .name = "instr-load", > + .args_type = "path:F,args:s?", > + .params = "path [arg]", > + .help = "load an instrumentation library", > + .cmd = hmp_instr_load, > + }, > + > +STEXI > +@item instr-load @var{path} [args=value[,...]] > +@findex instr-load > +Load an instrumentation library. > +ETEXI > + > + { > + .name = "instr-unload", > + .args_type = "handle:i", > + .params = "handle", > + .help = "unload an instrumentation library", > + .cmd = hmp_instr_unload, > + }, > + > +STEXI > +@item instr-unload > +@findex instr-unload > +Unload an instrumentation library. > +ETEXI > + > STEXI > @end table > ETEXI Want #ifdef CONFIG_INSTRUMENT, see my review of the previous patch. See also my remark there on returning handles vs. passing in IDs. > diff --git a/monitor.c b/monitor.c > index e0f880107f..8a7684f860 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2319,6 +2319,66 @@ int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) > return fd; > } > > +static void hmp_instr_load(Monitor *mon, const QDict *qdict) > +{ > + const char *path = qdict_get_str(qdict, "path"); > + const char *str = qdict_get_try_str(qdict, "args"); > + strList args; Blank line between last declaration and first statement, please. > + args.value = (str == NULL) ? NULL : (char *)str; What's wrong with args.value = (char *)str; ? > + args.next = NULL; > + InstrLoadResult *res = qmp_instr_load(path, args.value != NULL, > + args.value != NULL ? &args : NULL, > + NULL); > + switch (res->code) { > + case INSTR_LOAD_CODE_OK: > + monitor_printf(mon, "Handle: %"PRId64"\n", res->handle); > + monitor_printf(mon, "OK\n"); > + break; > + case INSTR_LOAD_CODE_TOO_MANY: > + monitor_printf(mon, "Too many instrumentation libraries already loaded\n"); > + break; > + case INSTR_LOAD_CODE_ERROR: > + monitor_printf(mon, "Instrumentation library returned a non-zero value during initialization"); > + break; > + case INSTR_LOAD_CODE_DLERROR: > + monitor_printf(mon, "Error loading library: %s\n", res->msg); > + break; > + case INSTR_LOAD_CODE_UNAVAILABLE: > + monitor_printf(mon, "Service not available\n"); > + break; > + default: > + fprintf(stderr, "Unknown instrumentation load code: %d", res->code); > + exit(1); Impossible conditions should be assertion failures, but it's a moot point because... > + break; > + } > + qapi_free_InstrLoadResult(res); > +} With qmp_instr_load() fixed to set an error on failure, this becomes something like InstrLoadResult *res = qmp_instr_load(path, args.value != NULL, args.value != NULL ? &args : NULL, &err); if (err) { error_report_err(err); } else { monitor_printf(mon, "Handle: %"PRId64"\n", res->handle); monitor_printf(mon, "OK\n"); } qapi_free_InstrLoadResult(res); > + > +static void hmp_instr_unload(Monitor *mon, const QDict *qdict) > +{ > + int64_t handle = qdict_get_int(qdict, "handle"); > + InstrUnloadResult *res = qmp_instr_unload(handle, NULL); > + switch (res->code) { > + case INSTR_UNLOAD_CODE_OK: > + monitor_printf(mon, "OK\n"); > + break; > + case INSTR_UNLOAD_CODE_INVALID: > + monitor_printf(mon, "Invalid handle\n"); > + break; > + case INSTR_UNLOAD_CODE_DLERROR: > + monitor_printf(mon, "Error unloading library: %s\n", res->msg); > + break; > + case INSTR_UNLOAD_CODE_UNAVAILABLE: > + monitor_printf(mon, "Service not available\n"); > + break; > + default: > + fprintf(stderr, "Unknown instrumentation unload code: %d", res->code); > + exit(1); > + break; > + } > + qapi_free_InstrUnloadResult(res); > +} > + > /* Please update hmp-commands.hx when adding or changing commands */ > static mon_cmd_t info_cmds[] = { > #include "hmp-commands-info.h" Likewise.
Markus Armbruster writes: > Lluís Vilanova <vilanova@ac.upc.edu> writes: >> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> >> --- >> hmp-commands.hx | 28 ++++++++++++++++++++++++++ >> monitor.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 88 insertions(+) >> >> diff --git a/hmp-commands.hx b/hmp-commands.hx >> index 1941e19932..703d7262f5 100644 >> --- a/hmp-commands.hx >> +++ b/hmp-commands.hx >> @@ -1858,6 +1858,34 @@ ETEXI >> .sub_table = info_cmds, >> }, >> >> + { >> + .name = "instr-load", >> + .args_type = "path:F,args:s?", >> + .params = "path [arg]", >> + .help = "load an instrumentation library", >> + .cmd = hmp_instr_load, >> + }, >> + >> +STEXI >> +@item instr-load @var{path} [args=value[,...]] >> +@findex instr-load >> +Load an instrumentation library. >> +ETEXI >> + >> + { >> + .name = "instr-unload", >> + .args_type = "handle:i", >> + .params = "handle", >> + .help = "unload an instrumentation library", >> + .cmd = hmp_instr_unload, >> + }, >> + >> +STEXI >> +@item instr-unload >> +@findex instr-unload >> +Unload an instrumentation library. >> +ETEXI >> + >> STEXI >> @end table >> ETEXI > Want #ifdef CONFIG_INSTRUMENT, see my review of the previous patch. > See also my remark there on returning handles vs. passing in IDs. >> diff --git a/monitor.c b/monitor.c >> index e0f880107f..8a7684f860 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -2319,6 +2319,66 @@ int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) >> return fd; >> } >> >> +static void hmp_instr_load(Monitor *mon, const QDict *qdict) >> +{ >> + const char *path = qdict_get_str(qdict, "path"); >> + const char *str = qdict_get_try_str(qdict, "args"); >> + strList args; > Blank line between last declaration and first statement, please. >> + args.value = (str == NULL) ? NULL : (char *)str; > What's wrong with > args.value = (char *)str; > ? >> + args.next = NULL; >> + InstrLoadResult *res = qmp_instr_load(path, args.value != NULL, >> + args.value != NULL ? &args : NULL, >> + NULL); >> + switch (res->code) { >> + case INSTR_LOAD_CODE_OK: >> + monitor_printf(mon, "Handle: %"PRId64"\n", res->handle); >> + monitor_printf(mon, "OK\n"); >> + break; >> + case INSTR_LOAD_CODE_TOO_MANY: >> + monitor_printf(mon, "Too many instrumentation libraries already loaded\n"); >> + break; >> + case INSTR_LOAD_CODE_ERROR: >> + monitor_printf(mon, "Instrumentation library returned a non-zero value during initialization"); >> + break; >> + case INSTR_LOAD_CODE_DLERROR: >> + monitor_printf(mon, "Error loading library: %s\n", res->msg); >> + break; >> + case INSTR_LOAD_CODE_UNAVAILABLE: >> + monitor_printf(mon, "Service not available\n"); >> + break; >> + default: >> + fprintf(stderr, "Unknown instrumentation load code: %d", res->code); >> + exit(1); > Impossible conditions should be assertion failures, but it's a moot point > because... >> + break; >> + } >> + qapi_free_InstrLoadResult(res); >> +} > With qmp_instr_load() fixed to set an error on failure, this becomes > something like > InstrLoadResult *res = qmp_instr_load(path, args.value != NULL, > args.value != NULL ? &args : NULL, > &err); > if (err) { > error_report_err(err); > } else { > monitor_printf(mon, "Handle: %"PRId64"\n", res->handle); > monitor_printf(mon, "OK\n"); > } > qapi_free_InstrLoadResult(res); >> + >> +static void hmp_instr_unload(Monitor *mon, const QDict *qdict) >> +{ >> + int64_t handle = qdict_get_int(qdict, "handle"); >> + InstrUnloadResult *res = qmp_instr_unload(handle, NULL); >> + switch (res->code) { >> + case INSTR_UNLOAD_CODE_OK: >> + monitor_printf(mon, "OK\n"); >> + break; >> + case INSTR_UNLOAD_CODE_INVALID: >> + monitor_printf(mon, "Invalid handle\n"); >> + break; >> + case INSTR_UNLOAD_CODE_DLERROR: >> + monitor_printf(mon, "Error unloading library: %s\n", res->msg); >> + break; >> + case INSTR_UNLOAD_CODE_UNAVAILABLE: >> + monitor_printf(mon, "Service not available\n"); >> + break; >> + default: >> + fprintf(stderr, "Unknown instrumentation unload code: %d", res->code); >> + exit(1); >> + break; >> + } >> + qapi_free_InstrUnloadResult(res); >> +} >> + >> /* Please update hmp-commands.hx when adding or changing commands */ >> static mon_cmd_t info_cmds[] = { >> #include "hmp-commands-info.h" > Likewise. Done, thanks! Lluis
diff --git a/hmp-commands.hx b/hmp-commands.hx index 1941e19932..703d7262f5 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1858,6 +1858,34 @@ ETEXI .sub_table = info_cmds, }, + { + .name = "instr-load", + .args_type = "path:F,args:s?", + .params = "path [arg]", + .help = "load an instrumentation library", + .cmd = hmp_instr_load, + }, + +STEXI +@item instr-load @var{path} [args=value[,...]] +@findex instr-load +Load an instrumentation library. +ETEXI + + { + .name = "instr-unload", + .args_type = "handle:i", + .params = "handle", + .help = "unload an instrumentation library", + .cmd = hmp_instr_unload, + }, + +STEXI +@item instr-unload +@findex instr-unload +Unload an instrumentation library. +ETEXI + STEXI @end table ETEXI diff --git a/monitor.c b/monitor.c index e0f880107f..8a7684f860 100644 --- a/monitor.c +++ b/monitor.c @@ -2319,6 +2319,66 @@ int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp) return fd; } +static void hmp_instr_load(Monitor *mon, const QDict *qdict) +{ + const char *path = qdict_get_str(qdict, "path"); + const char *str = qdict_get_try_str(qdict, "args"); + strList args; + args.value = (str == NULL) ? NULL : (char *)str; + args.next = NULL; + InstrLoadResult *res = qmp_instr_load(path, args.value != NULL, + args.value != NULL ? &args : NULL, + NULL); + switch (res->code) { + case INSTR_LOAD_CODE_OK: + monitor_printf(mon, "Handle: %"PRId64"\n", res->handle); + monitor_printf(mon, "OK\n"); + break; + case INSTR_LOAD_CODE_TOO_MANY: + monitor_printf(mon, "Too many instrumentation libraries already loaded\n"); + break; + case INSTR_LOAD_CODE_ERROR: + monitor_printf(mon, "Instrumentation library returned a non-zero value during initialization"); + break; + case INSTR_LOAD_CODE_DLERROR: + monitor_printf(mon, "Error loading library: %s\n", res->msg); + break; + case INSTR_LOAD_CODE_UNAVAILABLE: + monitor_printf(mon, "Service not available\n"); + break; + default: + fprintf(stderr, "Unknown instrumentation load code: %d", res->code); + exit(1); + break; + } + qapi_free_InstrLoadResult(res); +} + +static void hmp_instr_unload(Monitor *mon, const QDict *qdict) +{ + int64_t handle = qdict_get_int(qdict, "handle"); + InstrUnloadResult *res = qmp_instr_unload(handle, NULL); + switch (res->code) { + case INSTR_UNLOAD_CODE_OK: + monitor_printf(mon, "OK\n"); + break; + case INSTR_UNLOAD_CODE_INVALID: + monitor_printf(mon, "Invalid handle\n"); + break; + case INSTR_UNLOAD_CODE_DLERROR: + monitor_printf(mon, "Error unloading library: %s\n", res->msg); + break; + case INSTR_UNLOAD_CODE_UNAVAILABLE: + monitor_printf(mon, "Service not available\n"); + break; + default: + fprintf(stderr, "Unknown instrumentation unload code: %d", res->code); + exit(1); + break; + } + qapi_free_InstrUnloadResult(res); +} + /* Please update hmp-commands.hx when adding or changing commands */ static mon_cmd_t info_cmds[] = { #include "hmp-commands-info.h"
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> --- hmp-commands.hx | 28 ++++++++++++++++++++++++++ monitor.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+)