qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: qemu-block@nongnu.org, hreitz@redhat.com, pbonzini@redhat.com,
	bmarzins@redhat.com, qemu-devel@nongnu.org
Subject: Re: [PATCH] file-posix: Probe paths and retry SG_IO on potential path errors
Date: Thu, 15 May 2025 17:02:46 +0200	[thread overview]
Message-ID: <aCYCFrh_As6XuPQj@redhat.com> (raw)
In-Reply-To: <20250515140142.GA333399@fedora>

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

Am 15.05.2025 um 16:01 hat Stefan Hajnoczi geschrieben:
> On Thu, May 15, 2025 at 10:15:53AM +0200, Kevin Wolf wrote:
> > Am 13.05.2025 um 15:51 hat Stefan Hajnoczi geschrieben:
> > > On Tue, May 13, 2025 at 01:37:30PM +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>
> > > > ---
> > > >  block/file-posix.c | 82 +++++++++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 81 insertions(+), 1 deletion(-)
> > > 
> > > Maybe the probability of retry success would be higher with a delay so
> > > that intermittent issues have time to resolve themselves. Either way,
> > > the patch looks good.
> > 
> > I don't think adding a delay here would be helpful. The point of
> > multipath isn't that you wait until a bad path comes back, but that you
> > just switch to a different path until it is restored.
> 
> That's not what this loop does. DM_MPATH_PROBE_PATHS probes all paths
> and fails when no paths are available. The delay would only apply in the
> case when there are no paths available.
> 
> If the point is not to wait until some path comes back, then why loop at
> all?

DM_MPATH_PROBE_PATHS can only send I/O to paths in the active path
group, so it doesn't fail over to different path groups. If there are no
usable paths left in the current path group, but there are some in
another one, then the ioctl returns 0 and the next SG_IO would switch to
a different path group, which may or may not succeed. If it fails, we
have to probe the paths in that group, too.

> > Ideally, calling DM_MPATH_PROBE_PATHS would just remove all the bad
> > paths instantaneously and we would either be able to send the request
> > using one of the remaining good paths, or know that we have to fail. In
> > practice, the ioctl will probably have to wait for a timeout, so you get
> > a delay anyway.
> 
> True, if the read requests themselves time out rather than failing with
> an immediate error, then no delay is required. I guess both cases can
> happen and userspace has no visibility aside from measuring the time
> spent in the ioctl.

Right. But if they fail with an immediate error, that's really what we
want because it means no I/O that is stuck for a long time.

Kevin

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

  reply	other threads:[~2025-05-15 15:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-13 11:37 [PATCH] file-posix: Probe paths and retry SG_IO on potential path errors Kevin Wolf
2025-05-13 13:51 ` Stefan Hajnoczi
2025-05-15  8:15   ` Kevin Wolf
2025-05-15 14:01     ` Stefan Hajnoczi
2025-05-15 15:02       ` Kevin Wolf [this message]
2025-05-20 14:03         ` Stefan Hajnoczi
2025-05-21 17:46           ` Kevin Wolf
2025-05-21 18:23             ` Benjamin Marzinski
2025-05-14 13:43 ` Hanna Czenczek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aCYCFrh_As6XuPQj@redhat.com \
    --to=kwolf@redhat.com \
    --cc=bmarzins@redhat.com \
    --cc=hreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).