[PATCH] multipath: fix multipath locking

[PATCH] multipath: fix multipath locking

[Benjamin Marzinski]

In lock_multipath(), if multipathd fails halfway though locking the the path
devices, it doesn't unlock the ones that it already has locked.  This
patch fixes that.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/configure.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Index: multipath-tools/libmultipath/configure.c
===================================================================
--- multipath-tools.orig/libmultipath/configure.c
+++ multipath-tools/libmultipath/configure.c
@@ -284,6 +284,7 @@ lock_multipath (struct multipath * mpp, 
 	struct pathgroup * pgp;
 	struct path * pp;
 	int i, j;
+	int x, y;
 
 	if (!mpp || !mpp->pg)
 		return 0;
@@ -294,12 +295,25 @@ lock_multipath (struct multipath * mpp, 
 		vector_foreach_slot(pgp->paths, pp, j) {
 			if (lock && flock(pp->fd, LOCK_EX | LOCK_NB) &&
 			    errno == EWOULDBLOCK)
-				return 1;
+				goto fail;
 			else if (!lock)
 				flock(pp->fd, LOCK_UN);
 		}
 	}
 	return 0;
+fail:
+	vector_foreach_slot (mpp->pg, pgp, x) {
+		if (x > i)
+			return 1;
+		if (!pgp->paths)
+			continue;
+		vector_foreach_slot(pgp->paths, pp, y) {
+			if (x == i && y > j)
+				return 1;
+			flock(pp->fd, LOCK_UN);
+		}
+	}
+	return 1;
 }
 
 /*

Re: [PATCH] multipath: fix multipath locking

[Malahal Naineni]

Benjamin Marzinski [bmarzins@redhat.com] wrote:
> @@ -294,12 +295,25 @@ lock_multipath (struct multipath * mpp, 
>  		vector_foreach_slot(pgp->paths, pp, j) {
>  			if (lock && flock(pp->fd, LOCK_EX | LOCK_NB) &&
>  			    errno == EWOULDBLOCK)
> -				return 1;
> +				goto fail;
>  			else if (!lock)
>  				flock(pp->fd, LOCK_UN);
>  		}
>  	}
>  	return 0;
> +fail:
> +	vector_foreach_slot (mpp->pg, pgp, x) {
> +		if (x > i)
> +			return 1;
> +		if (!pgp->paths)
> +			continue;
> +		vector_foreach_slot(pgp->paths, pp, y) {
> +			if (x == i && y > j)

Path 'j' has failed to get the lock dropping us to the fail label.
We should NOT be unlocking for path "j", so the above if expression
should be "(x ==i && y >= j)" instead.

Otherwise, it looks good.

> +				return 1;
> +			flock(pp->fd, LOCK_UN);
> +		}
> +	}
> +	return 1;

Re: [PATCH] multipath: fix multipath locking

[Christophe Varoqui]

Applied with fix,
Thanks.

On jeu., 2010-11-04 at 22:59 -0700, Malahal Naineni wrote:
> Benjamin Marzinski [bmarzins@redhat.com] wrote:
> > @@ -294,12 +295,25 @@ lock_multipath (struct multipath * mpp, 
> >  		vector_foreach_slot(pgp->paths, pp, j) {
> >  			if (lock && flock(pp->fd, LOCK_EX | LOCK_NB) &&
> >  			    errno == EWOULDBLOCK)
> > -				return 1;
> > +				goto fail;
> >  			else if (!lock)
> >  				flock(pp->fd, LOCK_UN);
> >  		}
> >  	}
> >  	return 0;
> > +fail:
> > +	vector_foreach_slot (mpp->pg, pgp, x) {
> > +		if (x > i)
> > +			return 1;
> > +		if (!pgp->paths)
> > +			continue;
> > +		vector_foreach_slot(pgp->paths, pp, y) {
> > +			if (x == i && y > j)
> 
> Path 'j' has failed to get the lock dropping us to the fail label.
> We should NOT be unlocking for path "j", so the above if expression
> should be "(x ==i && y >= j)" instead.
> 
> Otherwise, it looks good.
> 
> > +				return 1;
> > +			flock(pp->fd, LOCK_UN);
> > +		}
> > +	}
> > +	return 1;
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel