Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler

[Hannes Reinecke]

On 11/3/26 2:51 PM, Stefan Hajnoczi wrote:
> On Tue, Nov 03, 2026 at 08:27:52AM +0100, Hannes Reinecke wrote:
> > On 11/2/26 9:06 PM, Stefan Hajnoczi wrote:
> > > On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
> > > > Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
> > > > abort and device reset both fail the same way (same dead ctrl vq), and
> > > > since virtio_scsi implements neither target nor bus reset, EH falls
> > > > through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
> > > > buffers without any guarantee the device has actually stopped touching
> > > > them. If bus mastering (or whatever broke the transport) comes back
> > > > later, the device can still write into memory the kernel has already
> > > > reused.
> > > >
> > > > Add a real eh_host_reset_handler instead. virtio_reset_device() is the
> > > > guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
> > > > device will not touch guest memory again, so it is safe to hand
> > > > outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
> > > > virtscsi_init() -- the exact sequence already used across
> > > > suspend/resume -- to tear the virtqueues down and rebuild them.
> > > >
> > > > Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
> > > > mid-write): EH now runs abort -> device reset -> host reset -> abort ->
> > > > device reset -> host reset -> offline, then dd's stuck fsync returns
> > > > EIO and D-state drains to 0, in ~244s total. No task remains
> > > > uninterruptibly blocked.
> > >
> > > By the way, there is a known limitation in the QEMU virtio-scsi
> > > implementation where some reset-related operations are implemented
> > > synchronously (can block vCPUs and QEMU activity while waiting for I/O
> > > to drain in some situations) instead of asynchronously. Recently there
> > > has been work in QEMU to make VIRTIO device reset asynchronous, for
> > > example, but it needs to be implemented on a per-device basis and
> > > virtio-scsi hasn't taken advantage of it yet.
> > >
> > > Please let me know if you'd like to discuss or contribute. It's
> > > something that is on my radar but not until 2027.
> > >
> > > > Suggested-by: Hannes Reinecke <hare@xxxxxxx>
> > > > Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@xxxxxxxxx>
> > > > ---
> > > > v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
> > > > Reinecke in review of the original RFC, instead of relying solely on
> > > > the EH loop falling through to offline.
> > > > ---
> > > >    drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
> > > >    1 file changed, 24 insertions(+)
> > > >
> > > > diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> > > > index b4f20c487718..9698f4f91cc6 100644
> > > > --- a/drivers/scsi/virtio_scsi.c
> > > > +++ b/drivers/scsi/virtio_scsi.c
> > > > @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
> > > >    	return SCSI_EH_NOT_HANDLED;
> > > >    }
> > > > +static int virtscsi_host_reset(struct scsi_cmnd *sc);
> > > > +
> > > >    static const struct scsi_host_template virtscsi_host_template = {
> > > >    	.module = THIS_MODULE,
> > > >    	.name = "Virtio SCSI HBA",
> > > > @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
> > > >    	.eh_abort_handler = virtscsi_abort,
> > > >    	.eh_device_reset_handler = virtscsi_device_reset,
> > > >    	.eh_timed_out = virtscsi_eh_timed_out,
> > > > +	.eh_host_reset_handler = virtscsi_host_reset,
> > > >    	.sdev_init = virtscsi_device_alloc,
> > > >    	.dma_boundary = UINT_MAX,
> > > > @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
> > > >    	return err;
> > > >    }
> > > > +/*
> > > > + * No bus/target reset in virtio-scsi, so EH lands here after device
> > >
> > > The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
> > > good to me, but I would like clarification on this comment. virtio-scsi
> > > does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
> > > the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
> > > doesn't have target reset? And do you want to go ahead and implement
> > > .eh_target_reset_handler() using I_T_NEXUS_RESET?
> > Would probably not a bad idea to use I_T_NEXUS_RESET here.
> > Not sure what it does internally, though; _is_ there a nexus
> > to reset?
>
> Whether there is any benefit to I_T_NEXUS_RESET may depend on the device
> implementation. In QEMU:
> - LOGICAL_UNIT_RESET resets just 1 LUN
> - I_T_NEXUS_RESET resets all LUNs
> - VIRTIO device reset also resets the HBA itself
>
> In practice QEMU's I_T_NEXUS_RESET is unlikely to fix things if
> LOGICAL_UNIT_RESET wasn't sufficient. Maybe it will help on other device
> implementations though.
>
> I think the Linux driver mapping should be:
> - .eh_device_reset_handler() -> LOGICAL_UNIT_RESET
> - .eh_target_reset_handler() -> I_T_NEXUS_RESET
> - .eh_host_reset_handler() -> VIRTIO device reset
I guess 'target_reset' is pretty much pointless for modern setups.
If lun reset doesn't work it's pretty much guaranteed that target_reset
won't help, either.
(Unless you are on SAS, then it might, as I_T nexus reset actually
resets the SAS link and such does something useful).
Problem is that in most cases LUN reset fails as the _communication_
to the LUN is interrupted, and hence sending another TMF across the
same communication channel won't help, either.
For virtio that would mean that we'd have to reset the virtio queues;
can we even do that in a meaningful fashion?

Cheers,

Hannes
--
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@xxxxxxx                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich