Re: [Qemu-devel] [PATCH] scsi-disk: Fix crash if underlying host file or disk returns error.

[Kevin Wolf]

Am 21.11.2018 um 19:31 hat Paolo Bonzini geschrieben:
> On 21/11/18 13:47, Richard W.M. Jones wrote:
> > Commit 40dce4ee6 "scsi-disk: fix rerror/werror=ignore" introduced a
> > bug which causes qemu to crash with the assertion error below if the
> > host file or disk returns an error:
> > 
> >   qemu-system-x86_64: hw/scsi/scsi-bus.c:1374: scsi_req_complete:
> >   Assertion `req->status == -1' failed.
> > 
> > Kevin Wolf suggested this fix:
> > 
> >   < kwolf> Hm, should the final return false; in that patch
> >            actually be a return true?
> >   < kwolf> Because I think he didn't intend to change anything
> >            except BLOCK_ERROR_ACTION_IGNORE
> > 
> > Signed-off-by: Richard W.M. Jones <address@hidden>
> > Buglink: https://bugs.launchpad.net/qemu/+bug/1804323
> > ---
> >  hw/scsi/scsi-disk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> > index 6eb258d3f3..0e9027c8f3 100644
> > --- a/hw/scsi/scsi-disk.c
> > +++ b/hw/scsi/scsi-disk.c
> > @@ -482,7 +482,7 @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int 
> > error, bool acct_failed)
> >      if (action == BLOCK_ERROR_ACTION_STOP) {
> >          scsi_req_retry(&r->req);
> >      }
> > -    return false;
> > +    return true;
> >  }
> >  
> >  static void scsi_write_complete_noio(SCSIDiskReq *r, int ret)
> > 
> 
> Looks good.  I was confused because now the function always returns
> true.  "If an arm was returning true, the other must be returning false...".

Yes. And we should probably simplify the function by removing the return
value, but the minimal fix is better for 3.1.

Kevin