Re: [PATCH for-10.0] scsi-disk: Apply error policy for host_status errors again

[Kevin Wolf <kwolf@redhat.com>]

Am 10.04.2025 um 14:37 hat Michael Tokarev geschrieben:
> 07.04.2025 18:59, Kevin Wolf пишет:
> > Originally, all failed SG_IO requests called scsi_handle_rw_error() to
> > apply the configured error policy. However, commit f3126d65, which was
> > supposed to be a mere refactoring for scsi-disk.c, broke this and
> > accidentally completed the SCSI request without considering the error
> > policy any more if the error was signalled in the host_status field.
> > 
> > Apart from the commit message not describing the chance as intended,
> > errors indicated in host_status are also obviously backend errors and
> > not something the guest must deal with indepdently of the error policy.
> > 
> > This behaviour means that some recoverable errors (such as a path error
> > in multipath configurations) were reported to the guest anyway, which
> > might not expect it and might consider its disk broken.
> > 
> > Make sure that we apply the error policy again for host_status errors,
> > too. This addresses an existing FIXME comment and allows us to remove
> > some comments warning that callbacks weren't always called. With this
> > fix, they are called in all cases again.
> > 
> > The return value passed to the request callback doesn't have more free
> > values that could be used to indicate host_status errors as well as SAM
> > status codes and negative errno. Store the value in the host_status
> > field of the SCSIRequest instead and use -ENODEV as the return value (if
> > a path hasn't been reachable for a while, blk_aio_ioctl() will return
> > -ENODEV instead of just setting host_status, so just reuse it here -
> > it's not necessarily entirely accurate, but it's as good as any errno).
> > 
> > Cc: qemu-stable@nongnu.org
> > Fixes: f3126d65b393 ('scsi: move host_status handling into SCSI drivers')
> 
> Does it make sense to apply this one for older stable qemu series?
> In particular, in 8.2, we lack cfe0880835cd3
> "scsi-disk: Use positive return value for status in dma_readv/writev",
> which seems to be relevant here.  Or should I pick up cfe0880835cd3 too,
> maybe together with 8a0495624f (a no-op, just to make this patch to apply
> cleanly) and probably 9da6bd39f924?

Yes, I think it makes sense to pick all of them up (and 622a7016 in the
middle, too), they were part of one series:

https://patchew.org/QEMU/20240731123207.27636-1-kwolf@redhat.com/

And this patch builds on top of that series, so rebasing it correctly
might not be trivial without the previous series.

Kevin