On 3/28/23 11:36 AM, Mike Snitzer wrote:
> On Fri, Mar 24 2023 at 2:17P -0400,
> Mike Christie <michael.christie@xxxxxxxxxx> wrote:
>
>> Add callouts for reading keys and reservations. This allows LIO to support
>> the READ_KEYS and READ_RESERVATION commands and will allow dm-multipath
>> to optimize it's error handling so it can check if it's getting an error
>> because there's an existing reservation or if we need to retry different
>> paths.
>
> Not seeing anything in DM's path selectors that adds these
> optimizations. Is there accompanying multipath-tools callouts to get
> at this data to optimize path selection (via DM table reloads)?
>
You can ignore that comment. The comment was meant for the dm pr callouts
and not for normal IO/path handling, and now I think I can fix in a different
way.
I originally added the comment about better dm error handling for something
like __dm_pr_register getting a failure when it did:
ret = ops->pr_register(dev->bdev, pr->old_key, pr->new_key, pr->flags);
Right now, we fail the entire operation if just one call on one path fails. With the
pr_read_keys/reservation callouts we could check if we got a failure because there
was an existing reservation vs a retryable/ignorable error.
I forgot I wrote that comment about dm in the mail and we actually don't need the
pr_read_* callouts for that type of thing now, because I ended up changing
the existing callouts to return common error codes last kernel. So I have another
patchset that I'm working on, but am still debating about some issues like:
ret = ops->pr_register(dev->bdev, pr->old_key, pr->new_key, pr->flags);
switch (ret) {
case PR_STS_RESERVATION_CONFLICT:
pr->ret = ret;
return -1;
case PR_STS_RETRY_PATH_FAILURE:
/*
* We probably want to retry like how we do for the pg_init.
*/
....
case PR_STS_PATH_FAILED:
case PR_STS_PATH_FAST_FAILED:
/*
* I'm still not sure what to do here, because if this is the last
* host then we might want to try and register the rest of the paths
* and limp on. It probably needs a user config option.
*/
....
