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

[Paolo Bonzini <pbonzini@redhat.com>]

On 06/07/2017 10:45, Vladimir Sementsov-Ogievskiy wrote:
>>> +do_nbd_trip_read(uint32_t len) "Read %" PRIu32" byte(s)"
>> This one is good.
> 
> why do you like this and do not like nbd_trip_write_zeros?

I'm not sure I understand: this one is after blk_pread returns.  It
tells you that the socket is about to receive the payload.

For write, nothing special happened since do_nbd_trip_cmd_write.

For write zeroes, nothing special happened since
nbd_co_receive_request_decode_type.

>>
>>> +do_nbd_trip_cmd_write(void) "Request type is WRITE"
>>> +do_nbd_trip_cmd_write_readonly(void) "Server is read-only, return
>>> error"
>> These can be removed.
> 
> I think the second is informative, isn't it?

Can you instead add a trace point at the beginning of nbd_co_send_reply,
with handle/error/len?  Then you can remove do_nbd_trip_read and
do_nbd_trip_complete, too.

>>> +do_nbd_trip_write(void) "Writing to device"
>> Please add the handle here.
> 
> handle will be printed in nbd_co_receive_request_decode_type..

You need it in case there are multiple requests in flight.

>>> +do_nbd_trip_cmd_write_zeroes(void) "Request type is WRITE_ZEROES"
>>> +do_nbd_trip_write_zeroes(void) "Writing to device"
>>> +do_nbd_trip_cmd_flush(void) "Request type is FLUSH"
>>> +do_nbd_trip_cmd_trim(void) "Request type is TRIM"
>> Can all be removed.
> 
> then, remove nbd_trip_write too ?

Write is different because it happens after nbd_read(client->ioc,
req->data, request->len, NULL) has completed.  You can add a tracepoint
to nbd_co_receive_request instead.

Paolo