[Cluster-devel] [PATCH] gfs2 userland: BZ 240545: gfs2_fsck should behave more like the other fscks.

[Bob Peterson <rpeterso@redhat.com>]

Since this is a fairly big/ugly patch, I thought I'd toss it out
here for people to review before I put it into cvs.

Basically, these are changes to make gfs2_fsck behave more
like the other fscks, e.g., fsck.ext3.  It's for bugzilla 240545,
(which may be marked private, but the bug doesn't say anything
that's not in this email.)

The changes are basically this:

1. When gfs2_fsck asks questions (especially y/n questions), it
   makes you hit enter after every y or n.  The other fscks don't;
   they respond as soon as you hit the y or n key.
   To accomplish that, I added a new gfs2_getch() function to
   libgfs2.  I chose not to use the other getch() functions because
   they force a dependency on curses or ncurses and it caused me
   some weird problems.

2. Recent versions of gfs2_fsck trap interrupt signals, as when
   the user hits <ctrl-c>, and give you the option to continue,
   skip or abort.  However, if you're being asked a y/n question,
   it doesn't let you interrupt, which is very annoying if you
   just want to break out.  To fix this, I broke the interrupt()
   function up and added some new helper functions to libgfs2,
   so now it will let you interrupt a question and handle it
   reasonably (also asking if you want to abort or continue).
   I did it this way in order to keep the question-asking
   functions in libgfs2 generic (not specific to gfs2_fsck) and
   still be able to re-ask an interrupted question.

3. The other fsck's are aptly named fsck.xxx (e.g. fsck.ext3).
   Recent versions of gfs2 compile the tool as gfs2_fsck, but
   make a symlink to it called fsck.gfs2.  I'm changing it to
   compile to fsck.gfs2, and creating the symlink gfs2_fsck
   to point to it instead.

The changes are more ugly than I had hoped, but it seems to work well.
Let me know what you think.

Regards,

Bob Peterson
Red Hat Cluster Suite
--
Index: convert/gfs2_convert.c
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/convert/gfs2_convert.c,v
retrieving revision 1.10
diff -w -u -p -p -u -r1.10 gfs2_convert.c
--- convert/gfs2_convert.c	6 Jun 2007 15:19:55 -0000	1.10
+++ convert/gfs2_convert.c	7 Aug 2007 14:47:03 -0000
@@ -1088,8 +1088,12 @@ int main(int argc, char **argv)
 	/* Make them seal their fate.                     */
 	/* ---------------------------------------------- */
 	if (!error) {
+		int abort;
+
 		give_warning();
-		if (!query(&opts, "Convert %s from GFS1 to GFS2? (y/n)", device)) {
+		if (!gfs2_query(&abort, &opts,
+				"Convert %s from GFS1 to GFS2? (y/n)",
+				device)) {
 			log_crit("%s not converted.\n", device);
 			close(sb2.device_fd);
 			exit(0);
Index: fsck/Makefile
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/fsck/Makefile,v
retrieving revision 1.12
diff -w -u -p -p -u -r1.12 Makefile
--- fsck/Makefile	1 Jun 2007 09:45:34 -0000	1.12
+++ fsck/Makefile	7 Aug 2007 14:47:03 -0000
@@ -12,8 +12,8 @@
 
 include ../../make/defines.mk
 
-TARGET1= gfs2_fsck
-TARGET2= fsck.gfs2
+TARGET1= fsck.gfs2
+TARGET2= gfs2_fsck
 
 OBJS=	eattr.o \
 	fs_recovery.o \
Index: fsck/fsck.h
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/fsck/fsck.h,v
retrieving revision 1.5
diff -w -u -p -p -u -r1.5 fsck.h
--- fsck/fsck.h	1 May 2007 16:43:38 -0000	1.5
+++ fsck/fsck.h	7 Aug 2007 14:47:03 -0000
@@ -19,6 +19,8 @@
 #define FSCK_HASH_SIZE          (1 << FSCK_HASH_SHIFT)
 #define FSCK_HASH_MASK          (FSCK_HASH_SIZE - 1)
 
+#define query(opts, fmt, args...) gfs2_query(&fsck_abort, opts, fmt,
##args)
+
 struct inode_info
 {
         osi_list_t list;
Index: fsck/main.c
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/fsck/main.c,v
retrieving revision 1.7
diff -w -u -p -p -u -r1.7 main.c
--- fsck/main.c	1 May 2007 16:43:38 -0000	1.7
+++ fsck/main.c	7 Aug 2007 14:47:03 -0000
@@ -139,40 +139,19 @@ int read_cmdline(int argc, char **argv, 
 
 void interrupt(int sig)
 {
-	fd_set rfds;
-	struct timeval tv;
 	char response;
-	int err;
+	char progress[PATH_MAX];
 
-	if (opts.query) /* if we're asking them a question */
-		return;     /* ignore the interrupt signal */
-	FD_ZERO(&rfds);
-	FD_SET(STDIN_FILENO, &rfds);
-
-	tv.tv_sec = 0;
-	tv.tv_usec = 0;
-	/* Make sure there isn't extraneous input before asking the
-	 * user the question */
-	while((err = select(STDIN_FILENO + 1, &rfds, NULL, NULL, &tv))) {
-		if(err < 0) {
-			log_debug("Error in select() on stdin\n");
-			break;
-		}
-		read(STDIN_FILENO, &response, sizeof(char));
-	}
-	while (TRUE) {
-		printf("\ngfs_fsck interrupted in %s:  ", pass);
 		if (!last_reported_block || last_reported_block == last_fs_block)
-			printf("progress unknown.\n");
+		sprintf(progress, "progress unknown.\n");
 		else
-			printf("processing block %" PRIu64 " out of %" PRIu64 "\n",
-				   last_reported_block, last_fs_block);
-		printf("Do you want to abort gfs_fsck, skip the rest of %s or
continue (a/s/c)?", pass);
-
-		/* Make sure query is printed out */
-		fflush(stdout);
-		read(STDIN_FILENO, &response, sizeof(char));
+		sprintf(progress, "processing block %" PRIu64 " out of %"
+			PRIu64 "\n", last_reported_block, last_fs_block);
 
+	response = generic_interrupt("gfs2_fsck", pass, progress,
+				     "Do you want to abort gfs_fsck, skip " \
+				     "the rest of this pass or continue " \
+				     "(a/s/c)?", "asc");
 		if(tolower(response) == 's') {
 			skip_this_pass = TRUE;
 			return;
@@ -181,15 +160,6 @@ void interrupt(int sig)
 			fsck_abort = TRUE;
 			return;
 		}
-		else if (tolower(response) == 'c')
-			return;
-        else {
-			while(response != '\n')
-				read(STDIN_FILENO, &response, sizeof(char));
-			printf("Bad response, please type 'c', 'a' or 's'.\n");
-			continue;
-        }
-	}
 }
 
 /* Check system inode and verify it's marked "in use" in the bitmap:
*/
Index: fsck/pass1.c
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/fsck/pass1.c,v
retrieving revision 1.9
diff -w -u -p -p -u -r1.9 pass1.c
--- fsck/pass1.c	28 Jun 2007 23:41:37 -0000	1.9
+++ fsck/pass1.c	7 Aug 2007 14:47:03 -0000
@@ -766,8 +766,10 @@ int pass1(struct gfs2_sbd *sbp)
 			if (gfs2_next_rg_meta(rgd, &block, first))
 				break;
 			warm_fuzzy_stuff(block);
-			if (fsck_abort) /* if asked to abort */
+			if (fsck_abort) { /* if asked to abort */
+				gfs2_rgrp_relse(rgd, not_updated);
 				return 0;
+			}
 			if (skip_this_pass) {
 				printf("Skipping pass 1 is not a good idea.\n");
 				skip_this_pass = FALSE;
Index: libgfs2/gfs2_log.c
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/libgfs2/gfs2_log.c,v
retrieving revision 1.3
diff -w -u -p -p -u -r1.3 gfs2_log.c
--- libgfs2/gfs2_log.c	23 Jan 2007 19:23:06 -0000	1.3
+++ libgfs2/gfs2_log.c	7 Aug 2007 14:47:03 -0000
@@ -14,8 +14,10 @@
 #include <stdarg.h>
 #include <ctype.h>
 #include <libintl.h>
-
 #include <sys/select.h>
+#include <signal.h>
+#include <string.h>
+#include <termios.h>
 #include <unistd.h>
 
 #include "libgfs2.h"
@@ -45,13 +47,13 @@ void print_msg(int priority, char *file,
 	case MSG_DEBUG:
 		printf("(%s:%d)\t", file, line);
 		vprintf(format, args);
-		fflush(stdout);
+		fflush(NULL);
 		break;
 	case MSG_INFO:
 	case MSG_NOTICE:
 	case MSG_WARN:
 		vprintf(format, args);
-		fflush(stdout);
+		fflush(NULL);
 		break;
 	case MSG_ERROR:
 	case MSG_CRITICAL:
@@ -81,30 +83,43 @@ void print_fsck_log(int iif, int priorit
 	va_end(args);
 }
 
+char gfs2_getch(void)
+{
+	struct termios termattr, savetermattr;
+	char ch;
+	ssize_t size;
+
+	tcgetattr (STDIN_FILENO, &termattr);
+	savetermattr = termattr;
+	termattr.c_lflag &= ~(ICANON | IEXTEN | ISIG);
+	termattr.c_iflag &= ~(BRKINT | ICRNL | INPCK | ISTRIP | IXON);
+	termattr.c_cflag &= ~(CSIZE | PARENB);
+	termattr.c_cflag |= CS8;
+	termattr.c_oflag &= ~(OPOST);
+   	termattr.c_cc[VMIN] = 0;
+	termattr.c_cc[VTIME] = 0;
+
+	tcsetattr (STDIN_FILENO, TCSANOW, &termattr);
+	do {
+		size = read(STDIN_FILENO, &ch, 1);
+		if (size)
+			break;
+		usleep(50000);
+	} while (!size);
 
+	tcsetattr (STDIN_FILENO, TCSANOW, &savetermattr);
+	return ch;
+}
 
-int query(struct gfs2_options *opts, const char *format, ...)
+char generic_interrupt(const char *caller, const char *where,
+		       const char *progress, const char *question,
+		       const char *answers)
 {
-
-	va_list args;
-	const char *transform;
-	char response;
 	fd_set rfds;
 	struct timeval tv;
-	int err = 0;
-	int ret = 0;
-
-	va_start(args, format);
-
-	transform = _(format);
-
-	if(opts->yes)
-		return 1;
-	if(opts->no)
-		return 0;
+	char response;
+	int err, i;
 
-	opts->query = TRUE;
-	/* Watch stdin (fd 0) to see when it has input. */
 	FD_ZERO(&rfds);
 	FD_SET(STDIN_FILENO, &rfds);
 
@@ -118,37 +133,79 @@ int query(struct gfs2_options *opts, con
 			break;
 		}
 		read(STDIN_FILENO, &response, sizeof(char));
+	}
+	while (TRUE) {
+		printf("\n%s interrupted during %s:  ", caller, where);
+		if (progress)
+			printf("%s.\n", progress);
+		printf("%s", question);
 
+		/* Make sure query is printed out */
+		fflush(NULL);
+		response = gfs2_getch();
+		printf("\n");
+		fflush(NULL);
+		if (strchr(answers, response))
+			break;
+		printf("Bad response, please type ");
+		for (i = 0; i < strlen(answers) - 1; i++)
+			printf("'%c', ", answers[i]);
+		printf(" or '%c'.\n", answers[i]);
+	}
+	return response;
 	}
- query:
+
+int gfs2_query(int *setonabort, struct gfs2_options *opts,
+	       const char *format, ...)
+{
+
+	va_list args;
+	const char *transform;
+	char response;
+	int ret = 0;
+
+	*setonabort = 0;
+	va_start(args, format);
+
+	transform = _(format);
+
+	if(opts->yes)
+		return 1;
+	if(opts->no)
+		return 0;
+
+	opts->query = TRUE;
+	while (1) {
 	vprintf(transform, args);
 
 	/* Make sure query is printed out */
 	fflush(NULL);
+		response = gfs2_getch();
 
- rescan:
-	read(STDIN_FILENO, &response, sizeof(char));
-
-	if(tolower(response) == 'y') {
+		printf("\n");
+		fflush(NULL);
+		if (response == 0x3) { /* if interrupted, by ctrl-c */
+			response = generic_interrupt("Question", "response",
+						     NULL,
+						     "Do you want to abort " \
+						     "or continue (a/c)?",
+						     "ac");
+			if (response == 'a') {
+				ret = 0;
+				*setonabort = 1;
+				break;
+			}
+			printf("Continuing.\n");
+		} else if(tolower(response) == 'y') {
 		ret = 1;
+                        break;
 	} else if (tolower(response) == 'n') {
 		ret = 0;
-	} else if ((response == ' ') || (response == '\t')) {
-		goto rescan;
-	} else {
-		while(response != '\n')
-			read(STDIN_FILENO, &response, sizeof(char));
-		printf("Bad response, please type 'y' or 'n'.\n");
-		goto query;
-	}
-
-	/* Clip the input */
-	while((err = select(STDIN_FILENO + 1, &rfds, NULL, NULL, &tv))) {
-		if(err < 0) {
-			log_debug("Error in select() on stdin\n");
 			break;
+		} else {
+			printf("Bad response %d, please type 'y' or 'n'.\n",
+			       response);
 		}
-		read(STDIN_FILENO, &response, sizeof(char));
 	}
 
 	opts->query = FALSE;
Index: libgfs2/libgfs2.h
===================================================================
RCS file: /cvs/cluster/cluster/gfs2/libgfs2/libgfs2.h,v
retrieving revision 1.16
diff -w -u -p -p -u -r1.16 libgfs2.h
--- libgfs2/libgfs2.h	10 Jul 2007 18:21:07 -0000	1.16
+++ libgfs2/libgfs2.h	7 Aug 2007 14:47:03 -0000
@@ -496,7 +496,13 @@ void increase_verbosity(void);
 void decrease_verbosity(void);
 void print_fsck_log(int iif, int priority, char *file, int line,
 					const char *format, ...);
-int query(struct gfs2_options *opts, const char *format, ...);
+char gfs2_getch(void);
+
+char generic_interrupt(const char *caller, const char *where,
+		       const char *progress, const char *question,
+		       const char *answers);
+int gfs2_query(int *setonabort, struct gfs2_options *opts,
+	       const char *format, ...);
 
 /* misc.c */
 void compute_constants(struct gfs2_sbd *sdp);