[PATCH 0/4] locking updates

[PATCH 0/4] locking updates

[Zdenek Kabelac]

Locking changes to support Ctrl+C during yes_no_prompt
When we allow interrupts for the command,
and the lock VG, the current code no longer supports
breaks when allowed again in yes_no_prompt.
This code adds extra level of depth to support it.

Zdenek Kabelac (4):
  locking: use sigint functions
  configure: drop siginterrupt
  locking: support stacking
  cleanup: add comment

 configure                  | 11 ---------
 configure.in               |  1 -
 lib/display/display.c      |  2 +-
 lib/locking/file_locking.c | 57 +++++-----------------------------------------
 lib/locking/locking.c      | 49 ++++++++++++++++++++-------------------
 lib/misc/configure.h.in    |  3 ---
 lib/misc/lvm-wrappers.h    | 15 ------------
 7 files changed, 32 insertions(+), 106 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] locking: use sigint functions

[Zdenek Kabelac]

Use sigint_allow/restore function instead of duplicating code.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/locking/file_locking.c | 57 +++++-----------------------------------------
 1 file changed, 6 insertions(+), 51 deletions(-)

diff --git a/lib/locking/file_locking.c b/lib/locking/file_locking.c
index 5e49bc4..ee15156 100644
--- a/lib/locking/file_locking.c
+++ b/lib/locking/file_locking.c
@@ -40,10 +40,6 @@ static struct dm_list _lock_list;
 static char _lock_dir[NAME_LEN];
 static int _prioritise_write_locks;
 
-static sig_t _oldhandler;
-static sigset_t _fullsigset, _intsigset;
-static volatile sig_atomic_t _handler_installed;
-
 static void _undo_flock(const char *file, int fd)
 {
 	struct stat buf1, buf2;
@@ -99,38 +95,6 @@ static void _reset_file_locking(void)
 	_release_lock(NULL, 0);
 }
 
-static void _remove_ctrl_c_handler(void)
-{
-	siginterrupt(SIGINT, 0);
-	if (!_handler_installed)
-		return;
-
-	_handler_installed = 0;
-
-	sigprocmask(SIG_SETMASK, &_fullsigset, NULL);
-	if (signal(SIGINT, _oldhandler) == SIG_ERR)
-		log_sys_error("signal", "_remove_ctrl_c_handler");
-}
-
-static void _trap_ctrl_c(int sig __attribute__((unused)))
-{
-	_remove_ctrl_c_handler();
-	log_error("CTRL-c detected: giving up waiting for lock");
-}
-
-static void _install_ctrl_c_handler(void)
-{
-	_handler_installed = 1;
-
-	if ((_oldhandler = signal(SIGINT, _trap_ctrl_c)) == SIG_ERR) {
-		_handler_installed = 0;
-		return;
-	}
-
-	sigprocmask(SIG_SETMASK, &_intsigset, NULL);
-	siginterrupt(SIGINT, 1);
-}
-
 static int _do_flock(const char *file, int *fd, int operation, uint32_t nonblock)
 {
 	int r = 1;
@@ -151,12 +115,15 @@ static int _do_flock(const char *file, int *fd, int operation, uint32_t nonblock
 		if (nonblock)
 			operation |= LOCK_NB;
 		else
-			_install_ctrl_c_handler();
+			sigint_allow();
 
 		r = flock(*fd, operation);
 		old_errno = errno;
-		if (!nonblock)
-			_remove_ctrl_c_handler();
+		if (!nonblock) {
+			sigint_restore();
+			if (sigint_caught())
+				log_error("Giving up waiting for lock.");
+		}
 
 		if (r) {
 			errno = old_errno;
@@ -370,17 +337,5 @@ int init_file_locking(struct locking_type *locking, struct cmd_context *cmd,
 
 	dm_list_init(&_lock_list);
 
-	if (sigfillset(&_intsigset) || sigfillset(&_fullsigset)) {
-		log_sys_error_suppress(suppress_messages, "sigfillset",
-				       "init_file_locking");
-		return 0;
-	}
-
-	if (sigdelset(&_intsigset, SIGINT)) {
-		log_sys_error_suppress(suppress_messages, "sigdelset",
-				       "init_file_locking");
-		return 0;
-	}
-
 	return 1;
 }
-- 
1.8.3.1

[PATCH 2/4] configure: drop siginterrupt

[Zdenek Kabelac]

Not used anymore

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 configure               | 11 -----------
 configure.in            |  1 -
 lib/misc/configure.h.in |  3 ---
 lib/misc/lvm-wrappers.h | 15 ---------------
 4 files changed, 30 deletions(-)

diff --git a/configure b/configure
index 12272d1..740fd1d 100755
--- a/configure
+++ b/configure
@@ -5499,17 +5499,6 @@ else
 fi
 done
 
-for ac_func in siginterrupt
-do :
-  ac_fn_c_check_func "$LINENO" "siginterrupt" "ac_cv_func_siginterrupt"
-if test "x$ac_cv_func_siginterrupt" = x""yes; then :
-  cat >>confdefs.h <<_ACEOF
-#define HAVE_SIGINTERRUPT 1
-_ACEOF
-
-fi
-done
-
 # The Ultrix 4.2 mips builtin alloca declared by alloca.h only works
 # for constant arguments.  Useless!
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for working alloca.h" >&5
diff --git a/configure.in b/configure.in
index 97ddc1d..631afdc 100644
--- a/configure.in
+++ b/configure.in
@@ -134,7 +134,6 @@ AC_CHECK_FUNCS([ftruncate gethostname getpagesize \
   gettimeofday memset mkdir mkfifo rmdir munmap nl_langinfo setenv setlocale \
   strcasecmp strchr strcspn strspn strdup strncasecmp strerror strrchr \
   strstr strtol strtoul uname], , [AC_MSG_ERROR(bailing out)])
-AC_CHECK_FUNCS(siginterrupt)
 AC_FUNC_ALLOCA
 AC_FUNC_CLOSEDIR_VOID
 AC_FUNC_CHOWN
diff --git a/lib/misc/configure.h.in b/lib/misc/configure.h.in
index aae4f24..e03bf72 100644
--- a/lib/misc/configure.h.in
+++ b/lib/misc/configure.h.in
@@ -282,9 +282,6 @@
 /* Define to 1 if you have the `setlocale' function. */
 #undef HAVE_SETLOCALE
 
-/* Define to 1 if you have the `siginterrupt' function. */
-#undef HAVE_SIGINTERRUPT
-
 /* Define to 1 if you have the <signal.h> header file. */
 #undef HAVE_SIGNAL_H
 
diff --git a/lib/misc/lvm-wrappers.h b/lib/misc/lvm-wrappers.h
index e43f831..d9bf08b 100644
--- a/lib/misc/lvm-wrappers.h
+++ b/lib/misc/lvm-wrappers.h
@@ -32,19 +32,4 @@ int lvm_getpagesize(void);
  */
 int read_urandom(void *buf, size_t len);
 
-#  ifndef HAVE_SIGINTERRUPT
-#    define siginterrupt(sig, flag) \
-	do { \
-		int ret; \
-		struct sigaction act; \
-		(void) sigaction(sig, NULL, &act); \
-		if (flag) \
-			act.sa_flags &= SA_RESTART; \
-		else \
-			act.sa_flags |= SA_RESTART; \
-		ret = sigaction(sig, &act, NULL); \
-		return ret; \
-	while (0)
-#  endif
-
 #endif
-- 
1.8.3.1

[PATCH 3/4] locking: support stacking

[Zdenek Kabelac]

Support 2 levels of nesting signal blocking.

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/locking/locking.c | 49 +++++++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 9183ee6..377d8a4 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -39,8 +39,11 @@ static int _blocking_supported = 0;
 
 static volatile sig_atomic_t _sigint_caught = 0;
 static volatile sig_atomic_t _handler_installed;
-static struct sigaction _oldhandler;
-static int _oldmasked;
+
+/* Support 2 level nesting, increase if needed more */
+#define MAX_SIGINTS 2
+static struct sigaction _oldhandler[MAX_SIGINTS];
+static int _oldmasked[MAX_SIGINTS];
 
 typedef enum {
         LV_NOOP,
@@ -84,50 +87,48 @@ void sigint_allow(void)
 	 * Do not overwrite the backed-up handler data -
 	 * just increase nesting count.
 	 */
-	if (_handler_installed) {
-		_handler_installed++;
+	if (++_handler_installed > MAX_SIGINTS)
 		return;
-	}
 
 	/* Grab old sigaction for SIGINT: shall not fail. */
-	sigaction(SIGINT, NULL, &handler);
+	if (sigaction(SIGINT, NULL, &handler))
+		log_sys_error("sigaction", "SIGINT");
 	handler.sa_flags &= ~SA_RESTART; /* Clear restart flag */
 	handler.sa_handler = _catch_sigint;
 
-	_handler_installed = 1;
-
 	/* Override the signal handler: shall not fail. */
-	sigaction(SIGINT, &handler, &_oldhandler);
+	if (sigaction(SIGINT, &handler, &_oldhandler[_handler_installed - 1]))
+		log_sys_error("sigaction", "SIGINT");
 
 	/* Unmask SIGINT.  Remember to mask it again on restore. */
-	sigprocmask(0, NULL, &sigs);
-	if ((_oldmasked = sigismember(&sigs, SIGINT))) {
+	if (sigprocmask(0, NULL, &sigs))
+		log_sys_error("sigprocmask", "");
+
+	if ((_oldmasked[_handler_installed] = sigismember(&sigs, SIGINT))) {
 		sigdelset(&sigs, SIGINT);
-		sigprocmask(SIG_SETMASK, &sigs, NULL);
+		if (sigprocmask(SIG_SETMASK, &sigs, NULL))
+			log_sys_error("sigprocmask", "SIG_SETMASK");
 	}
 }
 
 void sigint_restore(void)
 {
-	if (!_handler_installed)
-		return;
+	sigset_t sigs;
 
-	if (_handler_installed > 1) {
-		_handler_installed--;
+	if (!_handler_installed ||
+	    _handler_installed-- > MAX_SIGINTS)
 		return;
-	}
-
-	/* Nesting count went down to 0. */
-	_handler_installed = 0;
 
-	if (_oldmasked) {
-		sigset_t sigs;
+	/* Nesting count went bellow 2. */
+	if (_oldmasked[_handler_installed]) {
 		sigprocmask(0, NULL, &sigs);
 		sigaddset(&sigs, SIGINT);
-		sigprocmask(SIG_SETMASK, &sigs, NULL);
+		if (sigprocmask(SIG_SETMASK, &sigs, NULL))
+			log_sys_error("sigprocmask", "SIG_SETMASK");
 	}
 
-	sigaction(SIGINT, &_oldhandler, NULL);
+	if (sigaction(SIGINT, &_oldhandler[_handler_installed], NULL))
+		log_sys_error("sigaction", "SIGINT restore");
 }
 
 static void _block_signals(uint32_t flags __attribute__((unused)))
-- 
1.8.3.1

[PATCH 4/4] cleanup: add comment

[Zdenek Kabelac]

Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com>
---
 lib/display/display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/display/display.c b/lib/display/display.c
index e2b0f76..3f7f360 100644
--- a/lib/display/display.c
+++ b/lib/display/display.c
@@ -928,7 +928,7 @@ char yes_no_prompt(const char *prompt, ...)
 		}
 
 		if ((c = getchar()) == EOF) {
-			ret = 'n';
+			ret = 'n'; /* SIGINT */
 			break;
 		}
 
-- 
1.8.3.1