Re: [Qemu-devel] [PATCH 2/2] add some virtio-scsi trace events

[Laszlo Ersek <lersek@redhat.com>]

On 08/29/13 15:59, Paolo Bonzini wrote:
> Il 29/08/2013 15:37, Laszlo Ersek ha scritto:
>> The events that log a hexdump of the cdb and the sense buffer are disabled
>> by default, because they require more processing than a simple trace_XXX()
>> function call.
>>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>
> trace_event_get_state doesn't work with the dtrace backend.  Can you
> prepare v2 without those tracepoints?

(a) I was just following "docs/tracing.txt"; see the last section in
there.

(b) If I kill those two events, this is what you get:

Incoming request:

  virtio_scsi_handle_cmd_enter vdev 0x7f1be4054468 vq 0x7f1be4054b60
  virtio_scsi_pop_req req 0x7f1be5d77650
  virtio_scsi_handle_cmd_exit vdev 0x7f1be4054468 vq 0x7f1be4054b60

This gives you the address of the request, so you can look up the
corresponding completion. However you don't know the actual command
(cdb) without "virtio_scsi_dump_cmd_req".

Then,

  virtio_scsi_command_complete_enter req 0x7f1be5d77650 status 2 resid 0
  virtio_scsi_complete_req_enter req 0x7f1be5d77650
  virtio_scsi_complete_req_exit req 0x7f1be5d77650
  virtio_scsi_command_complete_exit req 0x7f1be5d77650 status 2 resid 0 sense_len 18

this will not tell you the virtio-scsi transport response code, or the
actual sense data.

(c) The way I submitted the series, the events in question are disabled
in "trace-events". Check out the functions themselves: they are
protected (ie. even the trace_event_get_state() calls are protected)
with preprocessing directives. I did it this way because I call them in
several places, and I wanted to keep the #if's centralized.

The function calls should be completely eliminated when the events are
disabled in "trace-events" (which is what the patch contains). They
should incur no performance penalty, and also not allow
trace_event_get_state() to conflict with dtrace -- if you build with the
dtrace backend, just leave these two disabled in "trace-events".

(d) I don't have more time for this, sorry.

Thanks,
Laszlo