[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader |
Date: |
Mon, 24 Jul 2017 13:03:46 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/24/2017 12:46 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
> 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 <address@hidden>
> +#
> +# 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
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 02/13] instrument: [none] Add null instrumentation mode, (continued)
- [Qemu-devel] [PATCH 02/13] instrument: [none] Add null instrumentation mode, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 03/13] instrument: [dynamic] Add dynamic instrumentation mode, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 04/13] instrument: Allow adding the "instrument" property without modifying event files, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 05/13] instrument: [dynamic] Add default public per-event functions, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 06/13] instrument: Add event control interface, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 07/13] instrument: Add generic command line library loader, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 08/13] instrument: [linux-user] Add command line library loader, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 09/13] instrument: [bsd-user] Add command line library loader, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 10/13] instrument: [softmmu] Add command line library loader, Lluís Vilanova, 2017/07/24
- [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader, Lluís Vilanova, 2017/07/24
- Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader,
Eric Blake <=
[Qemu-devel] [PATCH 12/13] instrument: [hmp] Add library loader, Lluís Vilanova, 2017/07/24
[Qemu-devel] [PATCH 13/13] trace: Rename C++-specific names in event arguments, Lluís Vilanova, 2017/07/24
Re: [Qemu-devel] [PATCH 00/13] instrument: Add basic event instrumentation, Stefan Hajnoczi, 2017/07/25