On 10/13/2015 11:10 AM, Lluís Vilanova wrote: > Signed-off-by: Lluís Vilanova <[email protected]>
If you'd send with 'qemu format-patch/send-email -v1', then your subject line would be formatted [PATCH v1 3/8] instead of the confusing results you got by omitting spaces [PATCHv13/8] (this is v13? out of 8?). Also, no need to include v1 (it's fairly obvious that an unversioned patch is the first), you really only need the designation for -v2 and beyond. The one-line commit subject correctly explains the 'what', but there is no commit body explaining the 'why'. While a commit body is not mandatory, it usually helps. > +++ b/qapi/trace.json > @@ -1,6 +1,6 @@ > # -*- mode: python -*- > # > -# Copyright (C) 2011-2014 Lluís Vilanova <[email protected]> > +# Copyright (C) 2011-2015 Lluís Vilanova <[email protected]> > # > # This work is licensed under the terms of the GNU GPL, version 2 or later. > # See the COPYING file in the top-level directory. > @@ -29,11 +29,12 @@ > # > # @name: Event name. > # @state: Tracing state. > +# @vcpu: Whether this is a per-vCPU event. > # Missing a '(since 2.5)' comment on the @vcpu line. > # Since 2.2 > ## > { 'struct': 'TraceEventInfo', > - 'data': {'name': 'str', 'state': 'TraceEventState'} } > + 'data': {'name': 'str', 'state': 'TraceEventState', 'vcpu': 'bool'} } This is okay from the qapi interface perspective (events are output, so unconditional addition of new fields won't break existing clients). I did not closely review the rest, however, I did spot this: > +++ b/trace/qmp.c > @@ -22,6 +22,8 @@ TraceEventInfoList *qmp_trace_event_get_state(const char > *name, Error **errp) > while ((ev = trace_event_pattern(name, ev)) != NULL) { > TraceEventInfoList *elem = g_new(TraceEventInfoList, 1); > elem->value = g_new(TraceEventInfo, 1); > + elem->value->vcpu = > + trace_event_get_cpu_id(ev) == TRACE_CPU_EVENT_COUNT ? false : > true; I'm not a fan of the over-verbose 'cond ? false : true'. It can almost always be written '!cond' with just as much clarity. In your case: elem->value->vcpu = trace_event_get_cpu_id(ev) != TRACE_CPU_EVENT_COUNT; -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
