Re: [Qemu-devel] [PATCH 1/2] scsi-disk: Convert from DPRINTF() macro to trace events

[Laurent Vivier]

On 07/12/2018 16:37, Philippe Mathieu-Daudé wrote:
> On 12/7/18 2:17 PM, Laurent Vivier wrote:
>> Signed-off-by: Laurent Vivier <address@hidden>
>> ---
>>  hw/scsi/scsi-disk.c  | 105 +++++++++++++++++++++----------------------
>>  hw/scsi/trace-events |  29 ++++++++++++
>>  2 files changed, 81 insertions(+), 53 deletions(-)
...
>>  
>> +static void scsi_disk_new_request_dump(uint32_t lun, uint32_t tag, uint8_t 
>> *buf,
>> +                                       void (*trace_fn)(uint32_t, uint32_t,
>> +                                                        char const *))
>> +{
>> +    int i;
>> +    int len = scsi_cdb_length(buf);
>> +    char *line_buffer, *p;
>> +
>> +    line_buffer = g_malloc(len * 5 + 1);
>> +
>> +    for (i = 0, p = line_buffer; i < len; i++) {
>> +        p += sprintf(p, " 0x%02x", buf[i]);
>> +    }
>> +    trace_fn(lun, tag, line_buffer);
>> +
>> +    g_free(line_buffer);
>> +}
>> +
>>  static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t 
>> lun,
>>                                       uint8_t *buf, void *hba_private)
>>  {
>> @@ -2512,16 +2518,9 @@ static SCSIRequest *scsi_new_request(SCSIDevice *d, 
>> uint32_t tag, uint32_t lun,
>>      }
>>      req = scsi_req_alloc(ops, &s->qdev, tag, lun, hba_private);
>>  
>> -#ifdef DEBUG_SCSI
>> -    DPRINTF("Command: lun=%d tag=0x%x data=0x%02x", lun, tag, buf[0]);
>> -    {
>> -        int i;
>> -        for (i = 1; i < scsi_cdb_length(buf); i++) {
>> -            printf(" 0x%02x", buf[i]);
>> -        }
>> -        printf("\n");
>> +    if (trace_event_get_state_backends(TRACE_SCSI_DISK_NEW_REQUEST)) {
>> +        scsi_disk_new_request_dump(lun, tag, buf, 
>> trace_scsi_disk_new_request);
> 
> TIL about trace_event_get_state_backends(), I'll use it to clean other
> hexdump traces, thanks :)

I've copied that from gdbstub.c where there is an hexdump() function
using it.

I'm going to send a v2 of this patch, because I think it is not needed
in this case to pass the function in the parameters.

Thanks,
Laurent