Re: [PATCH v2] file-posix: Probe paths and retry SG_IO on potential path errors

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

Am 22.05.2025 um 16:46 hat Stefan Hajnoczi geschrieben:
> On Thu, May 22, 2025 at 03:08:03PM +0200, Kevin Wolf wrote:
> > When scsi-block is used on a host multipath device, it runs into the
> > problem that the kernel dm-mpath doesn't know anything about SCSI or
> > SG_IO and therefore can't decide if a SG_IO request returned an error
> > and needs to be retried on a different path. Instead of getting working
> > failover, an error is returned to scsi-block and handled according to
> > the configured error policy. Obviously, this is not what users want,
> > they want working failover.
> > 
> > QEMU can parse the SG_IO result and determine whether this could have
> > been a path error, but just retrying the same request could just send it
> > to the same failing path again and result in the same error.
> > 
> > With a kernel that supports the DM_MPATH_PROBE_PATHS ioctl on dm-mpath
> > block devices (queued in the device mapper tree for Linux 6.16), we can
> > tell the kernel to probe all paths and tell us if any usable paths
> > remained. If so, we can now retry the SG_IO ioctl and expect it to be
> > sent to a working path.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > v2:
> > - Add a comment to explain retry scenarios [Stefan]
> > - Handle -EAGAIN returned for suspended devices [Ben]

> >  #if defined(__linux__)
> > +#if defined(DM_MPATH_PROBE_PATHS)
> > +static bool sgio_path_error(int ret, sg_io_hdr_t *io_hdr)
> > +{
> > +    if (ret < 0) {
> > +        switch (ret) {
> > +        case -ENODEV:
> > +            return true;
> > +        case -EAGAIN:
> > +            /*
> > +             * The device is probably suspended. This happens while the dm table
> > +             * is reloaded, e.g. because a path is added or removed. This is an
> > +             * operation that should complete within 1ms, so just wait a bit and
> > +             * retry.
> > +             *
> > +             * If the device was suspended for another reason, we'll wait and
> > +             * retry SG_IO_MAX_RETRIES times. This is a tolerable delay before
> > +             * we return an error and potentially stop the VM.
> > +             */
> > +            qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 1000000);
> 
> sgio_path_error() is missing coroutine_fn.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks, good catch. I already applied the patch and fixed it up to make
sure that I won't forget.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]