| Message ID | 1357653259-62650-42-git-send-email-hare@suse.de (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
On mar., 2013-01-08 at 14:54 +0100, Hannes Reinecke wrote: > During initial configuration the CLI thread is already running, > so we need to lock the vectors here to not race with the > 'reconfigure' CLI command. > Are you sure of the 41 - 42 patches ? - 41 moves the vecs locking from reconfigure caller to reconfigure - 42 removes the locking from reconfigure Regards, Christophe Varoqui www.opensvc.com > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > multipathd/main.c | 4 +--- > 1 files changed, 1 insertions(+), 3 deletions(-) > > diff --git a/multipathd/main.c b/multipathd/main.c > index 395307e..8917499 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs) > struct config * old = conf; > int retval = 1; > > - lock(vecs->lock); > /* > * free old map and path vectors ... they use old conf state > */ > @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs) > retval = 0; > } > > - unlock(vecs->lock); > return retval; > } > > @@ -1641,9 +1639,9 @@ child (void * param) > /* > * fetch and configure both paths and multipaths > */ > - lock(vecs->lock); > running_state = DAEMON_CONFIGURE; > > + lock(vecs->lock); > if (configure(vecs, 1)) { > unlock(vecs->lock); > condlog(0, "failure during configuration"); -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On 01/09/2013 12:37 AM, Christophe Varoqui wrote: > On mar., 2013-01-08 at 14:54 +0100, Hannes Reinecke wrote: >> During initial configuration the CLI thread is already running, >> so we need to lock the vectors here to not race with the >> 'reconfigure' CLI command. >> > Are you sure of the 41 - 42 patches ? > > - 41 moves the vecs locking from reconfigure caller to reconfigure > - 42 removes the locking from reconfigure > > Regards, > Christophe Varoqui > www.opensvc.com > >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> multipathd/main.c | 4 +--- >> 1 files changed, 1 insertions(+), 3 deletions(-) >> >> diff --git a/multipathd/main.c b/multipathd/main.c >> index 395307e..8917499 100644 >> --- a/multipathd/main.c >> +++ b/multipathd/main.c >> @@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs) >> struct config * old = conf; >> int retval = 1; >> >> - lock(vecs->lock); >> /* >> * free old map and path vectors ... they use old conf state >> */ >> @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs) >> retval = 0; >> } >> >> - unlock(vecs->lock); >> return retval; >> } >> >> @@ -1641,9 +1639,9 @@ child (void * param) >> /* >> * fetch and configure both paths and multipaths >> */ >> - lock(vecs->lock); >> running_state = DAEMON_CONFIGURE; >> >> + lock(vecs->lock); >> if (configure(vecs, 1)) { >> unlock(vecs->lock); >> condlog(0, "failure during configuration"); > > > Yes. Problem is we're already taking the lock prior to running any CLI commands in multipathd/main.c:uxsock_trigger(). So we cannot take the lock in reconfigure(). We also should not take the lock from the interrupt handler, as we might be in god knows which state. If you're bothered by this I'd rather remove the SIGHUP signal handler; we can achieve very much the same with the 'reconfigure' CLI command. Cheers, Hannes
diff --git a/multipathd/main.c b/multipathd/main.c index 395307e..8917499 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1389,7 +1389,6 @@ reconfigure (struct vectors * vecs) struct config * old = conf; int retval = 1; - lock(vecs->lock); /* * free old map and path vectors ... they use old conf state */ @@ -1410,7 +1409,6 @@ reconfigure (struct vectors * vecs) retval = 0; } - unlock(vecs->lock); return retval; } @@ -1641,9 +1639,9 @@ child (void * param) /* * fetch and configure both paths and multipaths */ - lock(vecs->lock); running_state = DAEMON_CONFIGURE; + lock(vecs->lock); if (configure(vecs, 1)) { unlock(vecs->lock); condlog(0, "failure during configuration");
During initial configuration the CLI thread is already running, so we need to lock the vectors here to not race with the 'reconfigure' CLI command. Signed-off-by: Hannes Reinecke <hare@suse.de> --- multipathd/main.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)