Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader

From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org, "Emilio G. Cota" <cota@braap.org>,
	Markus Armbruster <armbru@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 11/13] instrument: [qapi] Add library loader
Date: Tue, 25 Jul 2017 06:30:42 -0500	[thread overview]
Message-ID: <3a8accf6-2805-e2fa-1d58-1db85558c7a5@redhat.com> (raw)
In-Reply-To: <87wp6wgbzw.fsf@frigg.lan>

[-- Attachment #1: Type: text/plain, Size: 1377 bytes --]

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]