Re: [Qemu-devel] [PATCH] hw: Add support for new LSI Logic devices.

[Paolo Bonzini <pbonzini@redhat.com>]

Il 12/09/2012 01:50, Michael S. Tsirkin ha scritto:
> +static void lsilogic_abort_command(LsilogicCmd *cmd)
> +{
> +    if (cmd->req) {
> +        scsi_req_cancel(cmd->req);
> +        cmd->req = NULL;
> +    }
> +}

This only needs to be cmd->req = NULL.

> 
> +    if (cmd) {
> +        lsilogic_abort_command(cmd);
> +    } else {
> +        scsi_req_unref(req);
> +    }

This should be

    if (cmd && cmd->req) {
        scsi_req_unref(req);
        cmd->req = NULL;
    }

>> +                                cmd->iov_size);
>> +                    }
>> +                    cmd->iov_size = len;
>> +                }
>> +                scsi_req_continue(cmd->req);
>> +            }
>> +            if (len > 0) {
>> +                if (is_write) {
>> +                    trace_lsilogic_scsi_write_start(cmd->index, len);
>> +                } else {
>> +                    trace_lsilogic_scsi_read_start(cmd->index, len);
>> +                }
>> +            } else {
>> +                trace_lsilogic_scsi_nodata(cmd->index);
>> +            }

I think the second if needs to go before scsi_req_continue, otherwise
the trace will be a bit confused.

The SCSI parts otherwise look good.  I don't think there's much value in
keeping the coding standards for code that comes from elsewhere (I mean
mostly the struct definitions).

For the obvious bikeshedding, please call this lsisas1068.[ch].

Paolo