Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
instrument/Makefile.objs | 1 +
instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
qapi-schema.json | 3 ++
qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 167 insertions(+)
create mode 100644 instrument/qmp.c
create mode 100644 qapi/instrument.json
diff --git a/instrument/Makefile.objs b/instrument/Makefile.objs
index aa6db29ff4..757a247321 100644
--- a/instrument/Makefile.objs
+++ b/instrument/Makefile.objs
@@ -44,3 +44,4 @@ $(obj)/qemu-instr/events.h-timestamp: $(BUILD_DIR)/trace-events-all $(BUILD_DIR)
target-obj-y += control.o
target-obj-y += cmdline.o
+target-obj-y += qmp.o
diff --git a/instrument/qmp.c b/instrument/qmp.c
new file mode 100644
index 0000000000..3f577e0c78
--- /dev/null
+++ b/instrument/qmp.c
@@ -0,0 +1,71 @@
+/*
+ * QMP interface for dynamic trace instrumentation control commands.
+ *
+ * Copyright (C) 2012-2017 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/osdep.h"
+#include "qemu-common.h"
+#include "qapi/qmp/qerror.h"
+#include "qmp-commands.h"
+
+#include <dlfcn.h>
+
+#include "instrument/control.h"
+
+
+
+InstrLoadResult *qmp_instr_load(const char * path,
+ bool have_args, StringList * args,
+ Error **errp)
+{
+ int argc = 0;
+ const char **argv = NULL;
+
+ StringList *entry = have_args ? args : NULL;
+ while (entry != NULL) {
+ argv = realloc(argv, sizeof(*argv) * (argc + 1));
+ argv[argc] = entry->value->str;
+ argc++;
+ entry = entry->next;
+ }
+
+ InstrLoadResult *res = g_malloc0(sizeof(*res));
+ res->code = instr_load(path, argc, argv, &res->handle);
+ switch (res->code) {
+ case INSTR_LOAD_CODE_OK:
+ case INSTR_LOAD_CODE_UNAVAILABLE:
+ break;
+ case INSTR_LOAD_CODE_ERROR:
+ res->msg = dlerror();
+ break;
+ default:
+ fprintf(stderr, "Unknown instrumentation load code: %d", res->code);
+ exit(1);
+ break;
+ }
+ return res;
+}
+
+InstrUnloadResult *qmp_instr_unload(int64_t handle, Error **errp)
+{
+ InstrUnloadResult *res = g_malloc0(sizeof(*res));
+ res->code = instr_unload(handle);
+ switch (res->code) {
+ case INSTR_UNLOAD_OK:
+ case INSTR_UNLOAD_UNAVAILABLE:
+ case INSTR_UNLOAD_INVALID:
+ break;
+ case INSTR_UNLOAD_CODE_ERROR:
+ res->msg = dlerror();
+ break;
+ default:
+ fprintf(stderr, "Unknown instrumentation unload code: %d", res->code);
+ exit(1);
+ break;
+ }
+ return res;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index ab438ead70..6c4f237af8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -90,6 +90,9 @@
# QAPI introspection
{ 'include': 'qapi/introspect.json' }
+# Instrumentation commands
+{ 'include': 'qapi/instrument.json' }
+
##
# = QMP commands
##
diff --git a/qapi/instrument.json b/qapi/instrument.json
new file mode 100644
index 0000000000..f0d725e9a9
--- /dev/null
+++ b/qapi/instrument.json
@@ -0,0 +1,92 @@
+# *-*- Mode: Python -*-*
+#
+# QAPI trace instrumentation control commands.
+#
+# Copyright (C) 2012-2017 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.
+
+##
+# @InstrLoadCode:
+#
+# Result code of an 'instr-load' command.
+#
+# @ok: Correctly loaded.
+# @unavailable: Service not available.
+# @error: Error with libdl (see 'msg').
+#
+# Since: 2.10
+##
+{ 'enum': 'InstrLoadCode',
+ 'data': [ 'ok', 'unavailable', 'error' ] }
+
+##
+# @InstrLoadResult:
+#
+# Result of an 'instr-load' command.
+#
+# @code: Result code.
+# @msg: Additional error message.
+# @handle: Instrumentation library identifier (undefined in case of error).
+#
+# Since: 2.10
+##
+{ 'struct': 'InstrLoadResult',
+ 'data': { 'code': 'InstrLoadCode', 'msg': 'str', 'handle': 'int' } }
+
+##
+# @instr-load:
+#
+# Load an instrumentation library.
+#
+# @path: path to the dynamic instrumentation library
+# @args: arguments to the dynamic instrumentation library
+#
+# Since: 2.10
+##
+{ 'command': 'instr-load',
+ 'data': { 'path': 'str', '*args': ['String'] },
+ 'returns': 'InstrLoadResult' }
+
+
+##
+# @InstrUnloadCode:
+#
+# Result code of an 'instr-unload' command.
+#
+# @ok: Correctly unloaded.
+# @unavailable: Service not available.
+# @invalid: Invalid handle.
+# @error: Error with libdl (see 'msg').
+#
+# Since: 2.10
+##
+{ 'enum': 'InstrUnloadCode',
+ 'data': [ 'ok', 'unavailable', 'invalid', 'error' ] }
+
+##
+# @InstrUnloadResult:
+#
+# Result of an 'instr-unload' command.
+#
+# @code: Result code.
+# @msg: Additional error message.
+#
+# Since: 2.10
+##
+{ 'struct': 'InstrUnloadResult',
+ 'data': { 'code': 'InstrUnloadCode', 'msg': 'str' } }
+
+##
+# @instr-unload:
+#
+# Unload an instrumentation library.
+#
+# @handle: Instrumentation library identifier (see #InstrLoadResult).
+#
+# Since: 2.10
+##
+{ 'command': 'instr-unload',
+ 'data': { 'handle': 'int' },
+ 'returns': 'InstrUnloadResult' }
On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> instrument/Makefile.objs | 1 +
> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
> qapi-schema.json | 3 ++
> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 167 insertions(+)
> create mode 100644 instrument/qmp.c
> create mode 100644 qapi/instrument.json
Adding new files; but I don't see a patch to MAINTAINERS to cover
instrument/*.
> +++ b/qapi/instrument.json
> @@ -0,0 +1,92 @@
> +# *-*- Mode: Python -*-*
> +#
> +# QAPI trace instrumentation control commands.
> +#
> +# Copyright (C) 2012-2017 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.
> +
> +##
> +# @InstrLoadCode:
> +#
> +# Result code of an 'instr-load' command.
> +#
> +# @ok: Correctly loaded.
> +# @unavailable: Service not available.
> +# @error: Error with libdl (see 'msg').
> +#
> +# Since: 2.10
This is a new feature, and you've missed soft freeze. You'll want to
use 2.11 throughout the file.
> +##
> +{ 'enum': 'InstrLoadCode',
> + 'data': [ 'ok', 'unavailable', 'error' ] }
> +
> +##
> +# @InstrLoadResult:
> +#
> +# Result of an 'instr-load' command.
> +#
> +# @code: Result code.
> +# @msg: Additional error message.
Worth a comment that the message is for human consumption, and should
not be further parsed by machine?
Should msg be optional, present only when there is an error?
> +# @handle: Instrumentation library identifier (undefined in case of error).
Should it be an optional member, omitted when there is an error?
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'InstrLoadResult',
> + 'data': { 'code': 'InstrLoadCode', 'msg': 'str', 'handle': 'int' } }
> +
> +##
> +# @instr-load:
> +#
> +# Load an instrumentation library.
> +#
> +# @path: path to the dynamic instrumentation library
> +# @args: arguments to the dynamic instrumentation library
> +#
> +# Since: 2.10
> +##
> +{ 'command': 'instr-load',
> + 'data': { 'path': 'str', '*args': ['String'] },
Why are you double-nesting things? It's a lot nicer to use ['str']
(that is, your way requires
"arguments":{"path":"/some/path",
"args": [ { "str": "string1" }, { "str": "string2" } ] }
whereas mine only needs:
"arguments":{"path":"/some/path", "args":[ "string1", "string2" ]}
> + 'returns': 'InstrLoadResult' }
> +
> +
> +##
> +# @InstrUnloadCode:
> +#
> +# Result code of an 'instr-unload' command.
> +#
> +# @ok: Correctly unloaded.
> +# @unavailable: Service not available.
> +# @invalid: Invalid handle.
> +# @error: Error with libdl (see 'msg').
> +#
> +# Since: 2.10
> +##
> +{ 'enum': 'InstrUnloadCode',
> + 'data': [ 'ok', 'unavailable', 'invalid', 'error' ] }
> +
> +##
> +# @InstrUnloadResult:
> +#
> +# Result of an 'instr-unload' command.
> +#
> +# @code: Result code.
> +# @msg: Additional error message.
Again, should msg be optional? Document that it is only for human
consumption.
> +#
> +# Since: 2.10
> +##
> +{ 'struct': 'InstrUnloadResult',
> + 'data': { 'code': 'InstrUnloadCode', 'msg': 'str' } }
> +
> +##
> +# @instr-unload:
> +#
> +# Unload an instrumentation library.
> +#
> +# @handle: Instrumentation library identifier (see #InstrLoadResult).
> +#
> +# Since: 2.10
> +##
> +{ 'command': 'instr-unload',
> + 'data': { 'handle': 'int' },
> + 'returns': 'InstrUnloadResult' }
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Eric Blake writes:
> On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> instrument/Makefile.objs | 1 +
>> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
>> qapi-schema.json | 3 ++
>> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 167 insertions(+)
>> create mode 100644 instrument/qmp.c
>> create mode 100644 qapi/instrument.json
> Adding new files; but I don't see a patch to MAINTAINERS to cover
> instrument/*.
Who should I put as a maintainer? Or does this go to the general maintainer(s)?
>> +++ b/qapi/instrument.json
>> @@ -0,0 +1,92 @@
>> +# *-*- Mode: Python -*-*
>> +#
>> +# QAPI trace instrumentation control commands.
>> +#
>> +# Copyright (C) 2012-2017 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.
>> +
>> +##
>> +# @InstrLoadCode:
>> +#
>> +# Result code of an 'instr-load' command.
>> +#
>> +# @ok: Correctly loaded.
>> +# @unavailable: Service not available.
>> +# @error: Error with libdl (see 'msg').
>> +#
>> +# Since: 2.10
> This is a new feature, and you've missed soft freeze. You'll want to
> use 2.11 throughout the file.
Thx!
>> +##
>> +{ 'enum': 'InstrLoadCode',
>> + 'data': [ 'ok', 'unavailable', 'error' ] }
>> +
>> +##
>> +# @InstrLoadResult:
>> +#
>> +# Result of an 'instr-load' command.
>> +#
>> +# @code: Result code.
>> +# @msg: Additional error message.
> Worth a comment that the message is for human consumption, and should
> not be further parsed by machine?
> Should msg be optional, present only when there is an error?
True.
>> +# @handle: Instrumentation library identifier (undefined in case of error).
> Should it be an optional member, omitted when there is an error?
Also true.
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'InstrLoadResult',
>> + 'data': { 'code': 'InstrLoadCode', 'msg': 'str', 'handle': 'int' } }
>> +
>> +##
>> +# @instr-load:
>> +#
>> +# Load an instrumentation library.
>> +#
>> +# @path: path to the dynamic instrumentation library
>> +# @args: arguments to the dynamic instrumentation library
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'command': 'instr-load',
>> + 'data': { 'path': 'str', '*args': ['String'] },
> Why are you double-nesting things? It's a lot nicer to use ['str']
> (that is, your way requires
> "arguments":{"path":"/some/path",
> "args": [ { "str": "string1" }, { "str": "string2" } ] }
> whereas mine only needs:
> "arguments":{"path":"/some/path", "args":[ "string1", "string2" ]}
Aha, you mean the definition should be this instead?
{ 'command': 'instr-load',
'data': { 'path': 'str', '*args': ['str'] },
'returns': 'InstrLoadResult' }
>> + 'returns': 'InstrLoadResult' }
>> +
>> +
>> +##
>> +# @InstrUnloadCode:
>> +#
>> +# Result code of an 'instr-unload' command.
>> +#
>> +# @ok: Correctly unloaded.
>> +# @unavailable: Service not available.
>> +# @invalid: Invalid handle.
>> +# @error: Error with libdl (see 'msg').
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'enum': 'InstrUnloadCode',
>> + 'data': [ 'ok', 'unavailable', 'invalid', 'error' ] }
>> +
>> +##
>> +# @InstrUnloadResult:
>> +#
>> +# Result of an 'instr-unload' command.
>> +#
>> +# @code: Result code.
>> +# @msg: Additional error message.
> Again, should msg be optional? Document that it is only for human
> consumption.
Ok.
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'struct': 'InstrUnloadResult',
>> + 'data': { 'code': 'InstrUnloadCode', 'msg': 'str' } }
>> +
>> +##
>> +# @instr-unload:
>> +#
>> +# Unload an instrumentation library.
>> +#
>> +# @handle: Instrumentation library identifier (see #InstrLoadResult).
>> +#
>> +# Since: 2.10
>> +##
>> +{ 'command': 'instr-unload',
>> + 'data': { 'handle': 'int' },
>> + 'returns': 'InstrUnloadResult' }
>>
>>
Thanks,
Lluis
On 07/25/2017 03:24 AM, Lluís Vilanova wrote:
> Eric Blake writes:
>
>> On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> ---
>>> instrument/Makefile.objs | 1 +
>>> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++
>>> qapi-schema.json | 3 ++
>>> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 167 insertions(+)
>>> create mode 100644 instrument/qmp.c
>>> create mode 100644 qapi/instrument.json
>
>> Adding new files; but I don't see a patch to MAINTAINERS to cover
>> instrument/*.
>
> Who should I put as a maintainer? Or does this go to the general maintainer(s)?
You can be the maintainer if you'd like; or see if Stefan is okay
including it as part of the trace files, since it trace-related.
>>> +##
>>> +{ 'command': 'instr-load',
>>> + 'data': { 'path': 'str', '*args': ['String'] },
>
>> Why are you double-nesting things? It's a lot nicer to use ['str']
> Aha, you mean the definition should be this instead?
>
> { 'command': 'instr-load',
> 'data': { 'path': 'str', '*args': ['str'] },
> 'returns': 'InstrLoadResult' }
Yes.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Eric Blake writes: > On 07/25/2017 03:24 AM, Lluís Vilanova wrote: >> Eric Blake writes: >> >>> On 07/24/2017 12:46 PM, Lluís Vilanova wrote: >>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu> >>>> --- >>>> instrument/Makefile.objs | 1 + >>>> instrument/qmp.c | 71 ++++++++++++++++++++++++++++++++++++ >>>> qapi-schema.json | 3 ++ >>>> qapi/instrument.json | 92 ++++++++++++++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 167 insertions(+) >>>> create mode 100644 instrument/qmp.c >>>> create mode 100644 qapi/instrument.json >> >>> Adding new files; but I don't see a patch to MAINTAINERS to cover >>> instrument/*. >> >> Who should I put as a maintainer? Or does this go to the general maintainer(s)? > You can be the maintainer if you'd like; or see if Stefan is okay > including it as part of the trace files, since it trace-related. I can certainly do that. I was just wondering if the project prefers to have someone on the "core team" as a maintainer. Stefan? Thanks, Lluis
© 2016 - 2025 Red Hat, Inc.