[01/16] scsi-disk: no need to call scsi_req_data on a short read

In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
the request will be completed successfully instead of showing a failure.
It can even cause a segmentation fault.

An easy way to trigger it is "eject -f cd" during installation (during media
test if the installer does something like that).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/scsi-disk.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

On Wed, Aug 03, 2011 at 10:49:04AM +0200, Paolo Bonzini wrote:
> In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
> the request will be completed successfully instead of showing a failure.
> It can even cause a segmentation fault.
> 
> An easy way to trigger it is "eject -f cd" during installation (during media
> test if the installer does something like that).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

On Wed, Aug 3, 2011 at 9:49 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
> the request will be completed successfully instead of showing a failure.
> It can even cause a segmentation fault.
>
> An easy way to trigger it is "eject -f cd" during installation (during media
> test if the installer does something like that).
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/scsi-disk.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
>
> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
> index f42a5d1..814bf74 100644
> --- a/hw/scsi-disk.c
> +++ b/hw/scsi-disk.c
> @@ -217,9 +217,6 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
>         bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
>         vm_stop(VMSTOP_DISKFULL);
>     } else {
> -        if (type == SCSI_REQ_STATUS_RETRY_READ) {
> -            scsi_req_data(&r->req, 0);
> -        }
>         switch (error) {
>         case ENOMEM:
>             scsi_command_complete(r, CHECK_CONDITION,

Kevin, do you remember why you added this in 5dba48a8?

Stefan

Am 04.08.2011 15:35, schrieb Stefan Hajnoczi:
> On Wed, Aug 3, 2011 at 9:49 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> In fact, if the HBA's transfer_data callback goes on with scsi_req_continue
>> the request will be completed successfully instead of showing a failure.
>> It can even cause a segmentation fault.
>>
>> An easy way to trigger it is "eject -f cd" during installation (during media
>> test if the installer does something like that).
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  hw/scsi-disk.c |    3 ---
>>  1 files changed, 0 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
>> index f42a5d1..814bf74 100644
>> --- a/hw/scsi-disk.c
>> +++ b/hw/scsi-disk.c
>> @@ -217,9 +217,6 @@ static int scsi_handle_rw_error(SCSIDiskReq *r, int error, int type)
>>         bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read);
>>         vm_stop(VMSTOP_DISKFULL);
>>     } else {
>> -        if (type == SCSI_REQ_STATUS_RETRY_READ) {
>> -            scsi_req_data(&r->req, 0);
>> -        }
>>         switch (error) {
>>         case ENOMEM:
>>             scsi_command_complete(r, CHECK_CONDITION,
> 
> Kevin, do you remember why you added this in 5dba48a8?

No, I don't remember anything specific, I just tried to leave the
rerror=report case unchanged. This looks like it's moved code from
scsi_read_complete.

Kevin