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 15:33 hat Michael Tokarev geschrieben:
> 10.04.2025 16:14, Kevin Wolf wrote:
> > Am 10.04.2025 um 14:37 hat Michael Tokarev geschrieben:
> ...>> 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.
> 
> A (most likely small) issue here: 622a70161a "scsi-block: Don't skip
> callback for sgio error status/driver_status" is on top of an earlier
> commit, 1404226804 "scsi: don't lock AioContext in I/O code path",
> but does not actually *require* it, since it removes whole code block
> where a locking has been removed earlier by 1404226804.

Right, a backport should just remove the locking with it. That seems to
be a trivial conflict resolution.

> Also the next comment commit, 8a0495624f "scsi-disk: Add warning comments
> that host_status errors take a shortcut", clashes with e7fc3c4a8cc "scsi:
> remove outdated AioContext lock comment".

It's only comments anyway, but the correct conflict resolution here is
to keep both comments.

> This seems a bit too fragile for 8.2, don't you think?  And I haven't even
> tried to check 7.2 yet :)

Both of the merge conflicts you mentioned above don't really seem like
problems to me. Did you find any semantic merge conflicts? That's when I
would start to be concerned about fragility.

> https://gitlab.com/mjt0k/qemu/-/tree/staging-8.2 for the current result
> (not yet tested) - I dislike my comment handling at scsi_dma_complete().

Seems you already resolved the conflicts the same was as I suggested
above.

Kevin