Markus Armbruster writes:
> Lluís Vilanova <vilan...@ac.upc.edu> writes:
>> Signed-off-by: Lluís Vilanova <vilan...@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
