Re: [Qemu-devel] [PATCH 19/19] nbd: use generic trace subsystem instead of TRACE macro

[Vladimir Sementsov-Ogievskiy]

05.06.2017 18:23, Eric Blake wrote:
> [adding Stefan as trace maintainer]
>
> On 05/30/2017 09:30 AM, Vladimir Sementsov-Ogievskiy wrote:
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> > ---
> >   Makefile.objs      |  1 +
> >   nbd/client.c       | 77 ++++++++++++++++++++++++----------------------------
> >   nbd/nbd-internal.h | 19 -------------
> >   nbd/server.c       | 79 ++++++++++++++++++++++++++----------------------------
> >   nbd/trace-events   | 67 +++++++++++++++++++++++++++++++++++++++++++++
> >   5 files changed, 141 insertions(+), 102 deletions(-)

[...]

> >           goto fail;
> >       }
> >       magic = be64_to_cpu(magic);
> > -    TRACE("Magic is 0x%" PRIx64, magic);
> > +    trace_nbd_receive_negotiate_magic2(magic);
> In fact, I think you only need one trace function for tracing an 8-byte
> magic number, that can be called from more than one spot.

You've touched interesting point: is it normal to create 
trace-functions, used as different trace-points? So should 
trace-point-name describe point in the program or not? I've thought it 
should.

> > @@ -501,15 +499,16 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char 
> > *name, uint16_t *flags,
> >               goto fail;
> >           }
> >           globalflags = be16_to_cpu(globalflags);
> > -        TRACE("Global flags are %" PRIx32, globalflags);
> > +        trace_nbd_receive_negotiate_server_flags(
> > +            globalflags,
> > +            !!(globalflags & NBD_FLAG_FIXED_NEWSTYLE),
> > +            !!(globalflags & NBD_FLAG_NO_ZEROES));
> Why do we have to trace particular b


--
Best regards,
Vladimir