Re: [Qemu-devel] [PATCH v3 14/24] instrument: Add internal control interface

[Eric Blake]

On 04/21/2013 01:12 PM, Lluís Vilanova wrote:
> This interface provides two sets of operations:
> 
> * Loading/unloading a trace instrumentation library.
> 
> * Controls the instrumentation callbacks of the tracing events.
> 
> Note that in the case of static instrumentation, the library is not
> loaded/unloaded, but is still properly (de)initialized when QEMU starts and
> exits.

I don't know if the series in general will be accepted, but if so,
here's some interface things to think about:

> +++ b/instrument/qapi-schema.json
> @@ -0,0 +1,26 @@
> +# *-*- Mode: Python -*-*
> +#
> +# QAPI trace instrumentation control commands.
> +#
> +# Copyright (C) 2012-2013 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.
> +
> +##
> +# @InstrType
> +#
> +# Instrumentation type supported by the system.
> +#
> +# @None: No instrumentation support.
> +#
> +# @Static: Static instrumentation support.
> +#
> +# @Dynamic: Dynamic instrumentation support.

Generally, QMP enums are lower case (s/None/none/, and so forth).

> +#
> +# Warning: Keep in sync with #QIType.

If we had the hypothetical tool that converted .json into end-user
documentation, this sort of comment does not belong in the end-user
document.  Do we need to invent a syntax for development-only comments
within .json files for things that are useful during qemu development?
Or is it just better to move the comment to instead be in the C file
that declares the enum, reminding any developer that modifies the enum
to also modify the QMP type that reflects it?  (We went with the latter
approach in the recent query-command-line-options, putting the reminder
comment to keep QMP and C in sync only in the C file.)

> +#
> +# Since: 1.5

This missed 1.5; it will need to be 1.6 now.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature