[0/3] handle transitioning devices in TUR checker

Message

Benjamin Marzinski March 7, 2023, 10:49 p.m. UTC

This patchset is based on Brian Bunker's "libmultipath: return
'ghost' state when port is in transition" patch:

https://listman.redhat.com/archives/dm-devel/2023-February/053344.html
https://github.com/opensvc/multipath-tools/pull/60

Instead of setting the state to PATH_GHOST, it uses PATH_PENDING. The
other two patches are small cleanups to the TUR checker that I noticed
while writing the first patch.

Benjamin Marzinski (3):
  libmultipath: return 'pending' state when port is in transition
  libmultipath: set init failure message when init fails
  libmultipath: reset nr_timeouts if we freed the context

 libmultipath/checkers/tur.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

Comments

[e-mail] March 9, 2023, 8:38 a.m. UTC | #1

Brian,

could you give this patch set a test in your environment?

On Tue, 2023-03-07 at 16:49 -0600, Benjamin Marzinski wrote:
> This patchset is based on Brian Bunker's "libmultipath: return
> 'ghost' state when port is in transition" patch:
> 
> https://listman.redhat.com/archives/dm-devel/2023-February/053344.html
> https://github.com/opensvc/multipath-tools/pull/60
> 
> Instead of setting the state to PATH_GHOST, it uses PATH_PENDING. The
> other two patches are small cleanups to the TUR checker that I
> noticed
> while writing the first patch.
> 
> Benjamin Marzinski (3):
>   libmultipath: return 'pending' state when port is in transition
>   libmultipath: set init failure message when init fails
>   libmultipath: reset nr_timeouts if we freed the context
> 
>  libmultipath/checkers/tur.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 

I'd like to wait for Brian's results. From code inspection, and with
Ben's explanation for the 3/3 logic,

For the set:
Reviewed-by: Martin Wilck <[e-mail]>

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Brian Bunker March 10, 2023, 7:55 p.m. UTC | #2

> On Mar 9, 2023, at 12:38 AM, [e-mail] wrote:
> 
> Brian,
> 
> could you give this patch set a test in your environment?
> 
> On Tue, 2023-03-07 at 16:49 -0600, Benjamin Marzinski wrote:
>> This patchset is based on Brian Bunker's "libmultipath: return
>> 'ghost' state when port is in transition" patch:
>> 
>> https://listman.redhat.com/archives/dm-devel/2023-February/053344.html
>> https://github.com/opensvc/multipath-tools/pull/60
>> 
>> Instead of setting the state to PATH_GHOST, it uses PATH_PENDING. The
>> other two patches are small cleanups to the TUR checker that I
>> noticed
>> while writing the first patch.
>> 
>> Benjamin Marzinski (3):
>>   libmultipath: return 'pending' state when port is in transition
>>   libmultipath: set init failure message when init fails
>>   libmultipath: reset nr_timeouts if we freed the context
>> 
>>  libmultipath/checkers/tur.c | 24 +++++++++++++++++++-----
>>  1 file changed, 19 insertions(+), 5 deletions(-)
>> 
> 
> I'd like to wait for Brian's results. From code inspection, and with
> Ben's explanation for the 3/3 logic,
> 
> For the set:
> Reviewed-by: Martin Wilck <[e-mail]>
Ben and Martin,

This works well for me against my array. A couple of things in this part

Can you make the above match your formatting since it is better:
if( asc == 0x04 && ascq == 0x0b){

if (asc == 0x04 && ascq == 0x0a) {

And I don’t think you need else if since the line above it returns in its
case, so else is not hit if the first if is true:

    return PATH_GHOST;
} else if (asc == 0x04 && ascq == 0x0a) {

Thanks,
Brian



--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Benjamin Marzinski March 14, 2023, 8:45 p.m. UTC | #3

On Fri, Mar 10, 2023 at 11:55:19AM -0800, Brian Bunker wrote:
> > On Mar 9, 2023, at 12:38 AM, [e-mail] wrote:
> > 
> > Brian,
> > 
> > could you give this patch set a test in your environment?
> > 
> > On Tue, 2023-03-07 at 16:49 -0600, Benjamin Marzinski wrote:
> >> This patchset is based on Brian Bunker's "libmultipath: return
> >> 'ghost' state when port is in transition" patch:
> >> 
> >> https://listman.redhat.com/archives/dm-devel/2023-February/053344.html
> >> https://github.com/opensvc/multipath-tools/pull/60
> >> 
> >> Instead of setting the state to PATH_GHOST, it uses PATH_PENDING. The
> >> other two patches are small cleanups to the TUR checker that I
> >> noticed
> >> while writing the first patch.
> >> 
> >> Benjamin Marzinski (3):
> >>   libmultipath: return 'pending' state when port is in transition
> >>   libmultipath: set init failure message when init fails
> >>   libmultipath: reset nr_timeouts if we freed the context
> >> 
> >>  libmultipath/checkers/tur.c | 24 +++++++++++++++++++-----
> >>  1 file changed, 19 insertions(+), 5 deletions(-)
> >> 
> > 
> > I'd like to wait for Brian's results. From code inspection, and with
> > Ben's explanation for the 3/3 logic,
> > 
> > For the set:
> > Reviewed-by: Martin Wilck <[e-mail]>
> Ben and Martin,
> 
> This works well for me against my array. A couple of things in this part
> 
> Can you make the above match your formatting since it is better:
> if( asc == 0x04 && ascq == 0x0b){
> 
> if (asc == 0x04 && ascq == 0x0a) {
> 
> And I don’t think you need else if since the line above it returns in its
> case, so else is not hit if the first if is true:
> 
>     return PATH_GHOST;
> } else if (asc == 0x04 && ascq == 0x0a) {
> 

Sure.

-Ben

> Thanks,
> Brian
> 
> 
--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel