[PATCH 1/2] misc: Allow "-E" and "-O" options multiple times

[PATCH 1/2] misc: Allow "-E" and "-O" options multiple times

[Andreas Dilger]

Allow "-E" and "-O" options to be specified multiple times on the
command-line for mke2fs, tune2fs, and e2fsck, and parse them after
the config file options to more closely match user expectations.

It isn't correct to just parse these options as they are given,
since there may be a dependency on other options being specified
(e.g. blocksize), so the options are accumulated until all parsing
is finished, to avoid different behaviour if the other options are
given in a different order (e.g. passing "-E resize=40M -b 4096" might
behave differently from "-b 4096 -E resize=40M" on some filesystems).

Signed-off-by: Jim Garlick <garlick@llnl.gov>
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
---
 e2fsck/e2fsck.h         |    2 ++
 e2fsck/unix.c           |   28 +++++++++++++++++-----------
 e2fsck/util.c           |   26 ++++++++++++++++++++++++++
 misc/mke2fs.c           |   35 +++++++++++++++++++++++++----------
 misc/tune2fs.c          |   21 ++++++++++++++-------
 misc/util.c             |   26 ++++++++++++++++++++++++++
 misc/util.h             |    2 ++
 tests/m_raid_opt/script |    2 +-
 8 files changed, 113 insertions(+), 29 deletions(-)

diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
index b4a1a88..2574824 100644
--- a/e2fsck/e2fsck.h
+++ b/e2fsck/e2fsck.h
@@ -367,6 +367,8 @@ typedef struct region_struct *region_t;
 extern int e2fsck_strnlen(const char * s, int count);
 #endif
 
+extern errcode_t append_opts(char **old_opts, const char *added_opts);
+
 /*
  * Procedure declarations
  */
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 7e95ca8..0830175 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -618,14 +618,13 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)
 				continue;
 			}
 			ea_ver = strtoul(arg, &p, 0);
-			if (*p ||
-			    ((ea_ver != 1) && (ea_ver != 2))) {
-				fprintf(stderr,
-					_("Invalid EA version.\n"));
+			if (*p == '\0' && (ea_ver == 1 || ea_ver == 2)) {
+				ctx->ext_attr_ver = ea_ver;
+			} else {
+				fprintf(stderr, _("Invalid EA version.\n"));
 				extended_usage++;
 				continue;
 			}
-			ctx->ext_attr_ver = ea_ver;
 		} else if (strcmp(token, "fragcheck") == 0) {
 			ctx->options |= E2F_OPT_FRAGCHECK;
 			continue;
@@ -686,11 +685,11 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
 #ifdef HAVE_SIGNAL_H
 	struct sigaction	sa;
 #endif
-	char		*extended_opts = 0;
+	char		*extended_opts = NULL;
 	char		*cp;
-	int 		res;		/* result of sscanf */
+	int		res;		/* result of sscanf */
 #ifdef CONFIG_JBD_DEBUG
-	char 		*jbd_debug;
+	char		*jbd_debug;
 #endif
 
 	retval = e2fsck_allocate_context(&ctx);
@@ -748,7 +747,13 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
 			ctx->options |= E2F_OPT_COMPRESS_DIRS;
 			break;
 		case 'E':
-			extended_opts = optarg;
+			retval = append_opts(&extended_opts, optarg);
+			if (retval) {
+				com_err(ctx->program_name, retval,
+					_("reallocating for options '%s'"),
+					optarg);
+				fatal_error(ctx, NULL);
+			}
 			break;
 		case 'p':
 		case 'a':
@@ -883,14 +888,15 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
 			argv[optind]);
 		fatal_error(ctx, 0);
 	}
-	if (extended_opts)
-		parse_extended_opts(ctx, extended_opts);
 
 	if ((cp = getenv("E2FSCK_CONFIG")) != NULL)
 		config_fn[0] = cp;
 	profile_set_syntax_err_cb(syntax_err_report);
 	profile_init(config_fn, &ctx->profile);
 
+	if (extended_opts)
+		parse_extended_opts(ctx, extended_opts);
+
 	if (flush) {
 		fd = open(ctx->filesystem_name, O_RDONLY, 0);
 		if (fd < 0) {
diff --git a/e2fsck/util.c b/e2fsck/util.c
index fb9a87a..cb6c569 100644
--- a/e2fsck/util.c
+++ b/e2fsck/util.c
@@ -109,6 +109,32 @@ int e2fsck_strnlen(const char * s, int count)
 }
 #endif
 
+errcode_t append_opts(char **old_opts, const char *added_opts)
+{
+	/* First string adds NUL, others add ',' */
+	int newlen, oldlen = 0;
+	errcode_t retval;
+
+	if (*old_opts != NULL)
+		oldlen = strlen(*old_opts);
+
+	newlen = oldlen + strlen(added_opts) + 1;
+
+	retval = ext2fs_resize_mem(oldlen, newlen, old_opts);
+	if (retval != 0)
+		return retval;
+
+	if (oldlen != 0) {
+		if ((*old_opts)[oldlen - 1] !=',')
+			strcat(*old_opts, ",");
+		strcat(*old_opts, added_opts);
+	} else {
+		strcpy(*old_opts, added_opts);
+	}
+
+	return 0;
+}
+
 #ifndef HAVE_CONIO_H
 static int read_a_char(void)
 {
diff --git a/misc/mke2fs.c b/misc/mke2fs.c
index e062bda..e9711f0 100644
--- a/misc/mke2fs.c
+++ b/misc/mke2fs.c
@@ -1178,7 +1178,7 @@ static void PRS(int argc, char *argv[])
 {
 	int		b, c;
 	int		cluster_size = 0;
-	char 		*tmp, **cpp;
+	char		*tmp, **cpp;
 	int		blocksize = 0;
 	int		inode_ratio = 0;
 	int		inode_size = 0;
@@ -1188,18 +1188,18 @@ static void PRS(int argc, char *argv[])
 	int		show_version_only = 0;
 	unsigned long long num_inodes = 0; /* unsigned long long to catch too-large input */
 	errcode_t	retval;
-	char *		oldpath = getenv("PATH");
-	char *		extended_opts = 0;
-	char *		fs_type = 0;
-	char *		usage_types = 0;
+	char		*oldpath = getenv("PATH");
+	char		*extended_opts = NULL;
+	char		*fs_type = NULL;
+	char		*usage_types = NULL;
 	blk64_t		dev_size;
 	blk64_t		fs_blocks_count = 0;
 #ifdef __linux__
-	struct 		utsname ut;
+	struct		utsname ut;
 #endif
 	long		sysval;
 	int		s_opt = -1, r_opt = -1;
-	char		*fs_features = 0;
+	char		*fs_features = NULL;
 	int		use_bsize;
 	char		*newpath;
 	int		pathlen = sizeof(PATH_SET) + 1;
@@ -1436,11 +1436,26 @@ profile_error:
 			}
 			break;
 		case 'O':
-			fs_features = optarg;
+			retval = append_opts(&fs_features, optarg);
+			if (retval) {
+				com_err(program_name, retval,
+					_("reallocating for options '%s'"),
+					optarg);
+				exit(1);
+			}
 			break;
-		case 'E':
 		case 'R':
-			extended_opts = optarg;
+			fprintf(stderr, _("Warning: '-R option' is deprecated "
+					  "and should not be used anymore. "
+					  "Use '-E option' instead!\n"));
+		case 'E':
+			retval = append_opts(&extended_opts, optarg);
+			if (retval) {
+				com_err(program_name, retval,
+					_("reallocating for options '%s'"),
+					optarg);
+				exit(1);
+			}
 			break;
 		case 'S':
 			super_only = 1;
diff --git a/misc/tune2fs.c b/misc/tune2fs.c
index 5bf5187..672fe20 100644
--- a/misc/tune2fs.c
+++ b/misc/tune2fs.c
@@ -637,6 +637,7 @@ static void parse_tune2fs_options(int argc, char **argv)
 	char *tmp;
 	struct group *gr;
 	struct passwd *pw;
+	errcode_t retval;
 
 	open_flag = 0;
 
@@ -684,7 +685,13 @@ static void parse_tune2fs_options(int argc, char **argv)
 			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'E':
-			extended_cmd = optarg;
+			retval = append_opts(&extended_cmd, optarg);
+			if (retval) {
+				com_err(program_name, retval,
+					_("reallocating for options '%s'"),
+					optarg);
+				exit(1);
+			}
 			open_flag |= EXT2_FLAG_RW;
 			break;
 		case 'f': /* Force */
@@ -786,14 +793,14 @@ static void parse_tune2fs_options(int argc, char **argv)
 			mntopts_cmd = optarg;
 			open_flag = EXT2_FLAG_RW;
 			break;
-			
 		case 'O':
-			if (features_cmd) {
-				com_err(program_name, 0,
-					_("-O may only be specified once"));
-				usage();
+			retval = append_opts(&features_cmd, optarg);
+			if (retval) {
+				com_err(program_name, retval,
+					_("reallocating for options '%s'"),
+					optarg);
+				exit(1);
 			}
-			features_cmd = optarg;
 			open_flag = EXT2_FLAG_RW;
 			break;
 		case 'r':
diff --git a/misc/util.c b/misc/util.c
index 51bdb60..5ec479d 100644
--- a/misc/util.c
+++ b/misc/util.c
@@ -174,6 +174,32 @@ void check_mount(const char *device, int force, const char *type)
 	}
 }
 
+errcode_t append_opts(char **old_opts, const char *added_opts)
+{
+	/* First string adds NUL, others add ',' */
+	int newlen, oldlen = 0;
+	errcode_t retval;
+
+	if (*old_opts != NULL)
+		oldlen = strlen(*old_opts);
+
+	newlen = oldlen + strlen(added_opts) + 1;
+
+	retval = ext2fs_resize_mem(oldlen, newlen, old_opts);
+	if (retval)
+		return retval;
+
+	if (oldlen != 0) {
+		if ((*old_opts)[oldlen - 1] !=',')
+			strcat(*old_opts, ",");
+		strcat(*old_opts, added_opts);
+	} else {
+		strcpy(*old_opts, added_opts);
+	}
+
+	return 0;
+}
+
 void parse_journal_opts(const char *opts)
 {
 	char	*buf, *token, *next, *p, *arg;
diff --git a/misc/util.h b/misc/util.h
index e0c99f6..afbb700 100644
--- a/misc/util.h
+++ b/misc/util.h
@@ -24,3 +24,5 @@ extern void parse_journal_opts(const char *opts);
 extern void check_mount(const char *device, int force, const char *type);
 extern unsigned int figure_journal_size(int size, ext2_filsys fs);
 extern void print_check_message(ext2_filsys fs);
+extern errcode_t append_opts(char **old_opts, const char *added_opts);
+
diff --git a/tests/m_raid_opt/script b/tests/m_raid_opt/script
index 1e47cc1..296fe94 100644
--- a/tests/m_raid_opt/script
+++ b/tests/m_raid_opt/script
@@ -1,4 +1,4 @@
 DESCRIPTION="raid options"
 FS_SIZE=131072
-MKE2FS_OPTS="-R stride=13 -O sparse_super -g 1024"
+MKE2FS_OPTS="-E stride=13 -O sparse_super -g 1024"
 . $cmd_dir/run_mke2fs
-- 
1.7.3.4

[PATCH 2/2] e2fsck: allow shared blocks to be handled safely

[Andreas Dilger]

E2fsck fixes files that are found to be sharing blocks by cloning
the shared blocks and giving each file a private copy in pass 1D.

Allowing all files claiming the shared blocks to have copies can
inadvertantly bypass access restrictions.  Deleting all the files,
zeroing the cloned blocks, or placing the files in the /lost+found
directory after cloning may be preferable in some secure environments.
Handle all hard links to inodes with shared blocks in a similar way.

Add support for config file and command line options to e2fsck that
allow pass 1D behavior to be tuned according to site policy.  It adds
two extended options and config file counterparts.  On the command line:

 -E clone=dup|zero

    Select the block cloning method.  "dup" is old behavior which remains
    the default.  "zero" is a new method that substitutes zero-filled
    blocks for the shared blocks in all the files that claim them.

 -E shared=preserve|lost+found|delete

    Select the disposition of files containing shared blocks.  "preserve"
    is the old behavior which remains the default.  "lost+found" causes
    files to be unlinked after cloning so they will be reconnected to
    /lost+found in pass 3.   "delete" skips cloning entirely and simply
    deletes the files.

In the config file:
  [options]
      clone=dup|zero
      shared=preserve|lost+found|delete

Signed-off-by: Jim Garlick <garlick@llnl.gov>
Signed-off-by: Andreas Dilger <adilger@whamcloud.com>
---
 e2fsck/e2fsck.8.in      |   13 ++++++
 e2fsck/e2fsck.conf.5.in |   13 ++++++
 e2fsck/e2fsck.h         |   13 ++++++
 e2fsck/pass1b.c         |  102 ++++++++++++++++++++++++++++++++++++++---------
 e2fsck/problem.c        |    8 ++++
 e2fsck/problem.h        |    7 +++
 e2fsck/unix.c           |   77 +++++++++++++++++++++++++++++++++++
 7 files changed, 214 insertions(+), 19 deletions(-)

diff --git a/e2fsck/e2fsck.8.in b/e2fsck/e2fsck.8.in
index 178eecf..8db4173 100644
--- a/e2fsck/e2fsck.8.in
+++ b/e2fsck/e2fsck.8.in
@@ -180,6 +180,19 @@ separated, and may take an argument using the equals ('=') sign.  The
 following options are supported:
 .RS 1.2i
 .TP
+.BI clone= dup|zero
+Resolve files with shared blocks in pass 1D by giving each file a private
+copy of the blocks (dup);
+or replacing the shared blocks with private, zero-filled blocks (zero).  
+The default is dup.
+.TP
+.BI shared= preserve|lost+found|delete
+Files with shared blocks discovered in pass 1D are cloned and then left 
+in place (preserve); 
+cloned and then disconnected from their parent directory,
+then reconnected to /lost+found in pass 3 (lost+found); 
+or simply deleted (delete).  The default is preserve.
+.TP
 .BI ea_ver= extended_attribute_version
 Set the version of the extended attribute blocks which
 .B e2fsck
diff --git a/e2fsck/e2fsck.conf.5.in b/e2fsck/e2fsck.conf.5.in
index 2a600d0..83d2980 100644
--- a/e2fsck/e2fsck.conf.5.in
+++ b/e2fsck/e2fsck.conf.5.in
@@ -129,6 +129,19 @@ This boolean relation controls whether or not
 will offer to clear
 the test_fs flag if the ext4 filesystem is available on the system.  It
 defaults to true.
+.TP
+.I clone
+This string relation controls the default handling of shared blocks in pass 1D.
+It can be set to dup or zero.  See the
+.I "-E clone" 
+option description in e2fsck(8).
+.TP
+.I shared
+This string relation controls the default disposition of files discovered to 
+have shared blocks in pass 1D.  It can be set to preserve, lost+found, 
+or delete.  See the
+.I "-E shared" 
+option description in e2fsck(8).
 .TP 
 .I defer_check_on_battery
 This boolean relation controls whether or not the interval between 
diff --git a/e2fsck/e2fsck.h b/e2fsck/e2fsck.h
index 2574824..2b46f46 100644
--- a/e2fsck/e2fsck.h
+++ b/e2fsck/e2fsck.h
@@ -192,6 +192,17 @@ struct resource_track {
 #define E2F_PASS_5	5
 #define E2F_PASS_1B	6
 
+typedef	enum {
+	E2F_SHARED_PRESERVE = 0,
+	E2F_SHARED_DELETE,
+	E2F_SHARED_LPF
+} shared_opt_t;
+
+typedef enum {
+	E2F_CLONE_DUP = 0,
+	E2F_CLONE_ZERO
+} clone_opt_t;
+
 /*
  * Define the extended attribute refcount structure
  */
@@ -350,6 +361,8 @@ struct e2fsck_struct {
 	int ext_attr_ver;
 	profile_t	profile;
 	int blocks_per_page;
+	shared_opt_t shared;
+	clone_opt_t clone;
 
 	/*
 	 * For the use of callers of the e2fsck functions; not used by
diff --git a/e2fsck/pass1b.c b/e2fsck/pass1b.c
index 9bef368..3c651a4 100644
--- a/e2fsck/pass1b.c
+++ b/e2fsck/pass1b.c
@@ -63,6 +63,11 @@ struct inode_el {
 	struct inode_el *next;
 };
 
+struct dir_el {
+	ext2_ino_t	dir;
+	struct dir_el	*next;
+};
+
 struct dup_block {
 	int		num_bad;
 	struct inode_el *inode_list;
@@ -76,10 +81,10 @@ struct dup_block {
  * of multiply-claimed blocks.
  */
 struct dup_inode {
-	ext2_ino_t		dir;
 	int			num_dupblocks;
 	struct ext2_inode	inode;
 	struct block_el		*block_list;
+	struct dir_el		*dir_list;
 };
 
 static int process_pass1b_block(ext2_filsys fs, blk64_t	*blocknr,
@@ -123,6 +128,7 @@ static void add_dupe(e2fsck_t ctx, ext2_ino_t ino, blk64_t blk,
 	struct dup_inode	*di;
 	struct block_el		*blk_el;
 	struct inode_el 	*ino_el;
+	struct dir_el 		*dir_el;
 
 	n = dict_lookup(&blk_dict, INT_TO_VOIDPTR(blk));
 	if (n)
@@ -147,11 +153,17 @@ static void add_dupe(e2fsck_t ctx, ext2_ino_t ino, blk64_t blk,
 	else {
 		di = (struct dup_inode *) e2fsck_allocate_memory(ctx,
 			 sizeof(struct dup_inode), "duplicate inode header");
+		di->dir_list = NULL;
 		if (ino == EXT2_ROOT_INO) {
-			di->dir = EXT2_ROOT_INO;
+			dir_el = (struct dir_el *) e2fsck_allocate_memory(ctx,
+					sizeof(struct dir_el),
+					"duplicate inode element");
+			dir_el->dir = EXT2_ROOT_INO;
+			dir_el->next = di->dir_list;
+			di->dir_list = dir_el;
 			dup_inode_founddir++;
 		} else
-			di->dir = 0;
+			di->dir_list = NULL;
 
 		di->num_dupblocks = 0;
 		di->block_list = 0;
@@ -174,12 +186,17 @@ static void inode_dnode_free(dnode_t *node,
 {
 	struct dup_inode	*di;
 	struct block_el		*p, *next;
+	struct dir_el           *dp, *dnext;
 
 	di = (struct dup_inode *) dnode_get(node);
 	for (p = di->block_list; p; p = next) {
 		next = p->next;
 		free(p);
 	}
+	for (dp = di->dir_list; dp; dp = dnext) {
+		dnext = dp->next;
+		free(dp);
+	}
 	free(di);
 	free(node);
 }
@@ -371,6 +388,8 @@ struct search_dir_struct {
 	int		count;
 	ext2_ino_t	first_inode;
 	ext2_ino_t	max_inode;
+	e2fsck_t	ctx;
+	int		allflag;
 };
 
 static int search_dirent_proc(ext2_ino_t dir, int entry,
@@ -382,6 +401,7 @@ static int search_dirent_proc(ext2_ino_t dir, int entry,
 {
 	struct search_dir_struct *sd;
 	struct dup_inode	*p;
+	struct dir_el		*dd_el;
 	dnode_t			*n;
 
 	sd = (struct search_dir_struct *) priv_data;
@@ -398,12 +418,16 @@ static int search_dirent_proc(ext2_ino_t dir, int entry,
 	if (!n)
 		return 0;
 	p = (struct dup_inode *) dnode_get(n);
-	if (!p->dir) {
-		p->dir = dir;
+	if (sd->allflag || !p->dir_list) {
+		dd_el = (struct dir_el *) e2fsck_allocate_memory(sd->ctx,
+			sizeof(struct dir_el), "duplicate directory element");
+		dd_el->dir = dir;
+		dd_el->next = p->dir_list;
+		p->dir_list = dd_el;
 		sd->count--;
 	}
 
-	return(sd->count ? 0 : DIRENT_ABORT);
+	return((sd->allflag || sd->count) ? 0 : DIRENT_ABORT);
 }
 
 
@@ -420,15 +444,30 @@ static void pass1c(e2fsck_t ctx, char *block_buf)
 
 	/*
 	 * Search through all directories to translate inodes to names
-	 * (by searching for the containing directory for that inode.)
+	 * (by searching for the containing directories for that inode.)
 	 */
 	sd.count = dup_inode_count - dup_inode_founddir;
 	sd.first_inode = EXT2_FIRST_INODE(fs->super);
 	sd.max_inode = fs->super->s_inodes_count;
+	sd.ctx = ctx;
+	sd.allflag = (ctx->shared == E2F_SHARED_LPF);
 	ext2fs_dblist_dir_iterate(fs->dblist, 0, block_buf,
 				  search_dirent_proc, &sd);
 }
 
+static errcode_t unlink_all(e2fsck_t ctx, struct dup_inode *dup, ext2_ino_t ino)
+{
+	struct dir_el *dp;
+	errcode_t err, result = 0;
+
+	for (dp = dup->dir_list; dp; dp = dp->next) {
+		err = ext2fs_unlink(ctx->fs, dp->dir, NULL, ino, 0);
+			if (err)
+				result = err;
+	}
+	return result;
+}
+
 static void pass1d(e2fsck_t ctx, char *block_buf)
 {
 	ext2_filsys fs = ctx->fs;
@@ -476,6 +515,9 @@ static void pass1d(e2fsck_t ctx, char *block_buf)
 			q = (struct dup_block *) dnode_get(m);
 			if (q->num_bad > 1)
 				file_ok = 0;
+			if (q->num_bad == 1 && (ctx->clone == E2F_CLONE_ZERO ||
+			    ctx->shared != E2F_SHARED_PRESERVE))
+				file_ok = 0;
 			if (check_if_fs_block(ctx, s->block)) {
 				file_ok = 0;
 				meta_data = 1;
@@ -504,7 +546,7 @@ static void pass1d(e2fsck_t ctx, char *block_buf)
 		 */
 		pctx.inode = &p->inode;
 		pctx.ino = ino;
-		pctx.dir = p->dir;
+		pctx.dir = p->dir_list ? p->dir_list->dir : 0;
 		pctx.blkcount = p->num_dupblocks;
 		pctx.num = meta_data ? shared_len+1 : shared_len;
 		fix_problem(ctx, PR_1D_DUP_FILE, &pctx);
@@ -524,20 +566,32 @@ static void pass1d(e2fsck_t ctx, char *block_buf)
 			 */
 			pctx.inode = &t->inode;
 			pctx.ino = shared[i];
-			pctx.dir = t->dir;
+			pctx.dir = t->dir_list ? t->dir_list->dir : 0;
 			fix_problem(ctx, PR_1D_DUP_FILE_LIST, &pctx);
 		}
 		if (file_ok) {
 			fix_problem(ctx, PR_1D_DUP_BLOCKS_DEALT, &pctx);
 			continue;
 		}
-		if (fix_problem(ctx, PR_1D_CLONE_QUESTION, &pctx)) {
+		if (ctx->shared != E2F_SHARED_DELETE &&
+		    fix_problem(ctx, PR_1D_CLONE_QUESTION, &pctx)) {
 			pctx.errcode = clone_file(ctx, ino, p, block_buf);
-			if (pctx.errcode)
+			if (pctx.errcode) {
 				fix_problem(ctx, PR_1D_CLONE_ERROR, &pctx);
-			else
-				continue;
+				goto delete;
+			}
+			if (ctx->shared == E2F_SHARED_LPF &&
+			    fix_problem(ctx, PR_1D_DISCONNECT_QUESTION, &pctx)) {
+				pctx.errcode = unlink_all(ctx, p, ino);
+				if (pctx.errcode) {
+					fix_problem(ctx, PR_1D_DISCONNECT_ERROR,
+						    &pctx);
+					goto delete;
+				}
+			}
+			continue;
 		}
+delete:
 		if (fix_problem(ctx, PR_1D_DELETE_QUESTION, &pctx))
 			delete_file(ctx, ino, p, block_buf);
 		else
@@ -554,7 +608,8 @@ static void decrement_badcount(e2fsck_t ctx, blk64_t block, struct dup_block *p)
 {
 	p->num_bad--;
 	if (p->num_bad <= 0 ||
-	    (p->num_bad == 1 && !check_if_fs_block(ctx, block)))
+	    (p->num_bad == 1 && !check_if_fs_block(ctx, block) &&
+	    ctx->clone == E2F_CLONE_DUP))
 		ext2fs_unmark_block_bitmap2(ctx->block_dup_map, block);
 }
 
@@ -700,11 +755,15 @@ static int clone_file_block(ext2_filsys fs,
 			printf("Cloning block %u to %u\n", *block_nr,
 			       new_block);
 #endif
-			retval = io_channel_read_blk64(fs->io, *block_nr, 1,
-						     cs->buf);
-			if (retval) {
-				cs->errcode = retval;
-				return BLOCK_ABORT;
+			if (ctx->clone == E2F_CLONE_ZERO) {
+				memset(cs->buf, 0, fs->blocksize);
+			} else {
+				retval = io_channel_read_blk(fs->io, *block_nr,
+							1, cs->buf);
+				if (retval) {
+					cs->errcode = retval;
+					return BLOCK_ABORT;
+				}
 			}
 			retval = io_channel_write_blk64(fs->io, new_block, 1,
 						      cs->buf);
@@ -713,6 +772,11 @@ static int clone_file_block(ext2_filsys fs,
 				return BLOCK_ABORT;
 			}
 			decrement_badcount(ctx, *block_nr, p);
+			if (ctx->clone == E2F_CLONE_ZERO && p->num_bad == 0) {
+				ext2fs_unmark_block_bitmap2(ctx->block_found_map,
+							   *block_nr);
+				ext2fs_block_alloc_stats(fs, *block_nr, -1);
+			}
 			*block_nr = new_block;
 			ext2fs_mark_block_bitmap2(ctx->block_found_map,
 						 new_block);
diff --git a/e2fsck/problem.c b/e2fsck/problem.c
index c5bebf8..286dd9d 100644
--- a/e2fsck/problem.c
+++ b/e2fsck/problem.c
@@ -998,6 +998,14 @@ static struct e2fsck_problem problem_table[] = {
 	{ PR_1D_CLONE_ERROR,
 	  N_("Couldn't clone file: %m\n"), PROMPT_NONE, 0 },
 
+	/* File with shared blocks found */
+	{ PR_1D_DISCONNECT_QUESTION,
+	  N_("File with shared blocks found\n"), PROMPT_CONNECT, 0 },
+
+	/* Couldn't unlink file (error) */
+	{ PR_1D_DISCONNECT_ERROR,
+	  N_("Couldn't unlink file: %m\n"), PROMPT_NONE, 0 },
+
 	/* Pass 2 errors */
 
 	/* Pass 2: Checking directory structure */
diff --git a/e2fsck/problem.h b/e2fsck/problem.h
index 8379e0c..e8ab738 100644
--- a/e2fsck/problem.h
+++ b/e2fsck/problem.h
@@ -589,6 +589,13 @@ struct problem_context {
 /* Couldn't clone file (error) */
 #define PR_1D_CLONE_ERROR	0x013008
 
+/* File with shared blocks found */
+#define PR_1D_DISCONNECT_QUESTION 0x013009
+
+/* Couldn't unlink file (error) */
+#define PR_1D_DISCONNECT_ERROR	0x01300A
+
+
 /*
  * Pass 2 errors
  */
diff --git a/e2fsck/unix.c b/e2fsck/unix.c
index 0830175..825243c 100644
--- a/e2fsck/unix.c
+++ b/e2fsck/unix.c
@@ -593,6 +593,49 @@ static void signal_cancel(int sig EXT2FS_ATTR((unused)))
 }
 #endif
 
+static void initialize_profile_options(e2fsck_t ctx)
+{
+	char *tmp = NULL;
+
+	/* [options] shared=preserve|lost+found|delete */
+	ctx->shared = E2F_SHARED_PRESERVE;
+	profile_get_string(ctx->profile, "options", "shared", 0, "preserve",
+			   &tmp);
+	if (tmp) {
+		if (strcmp(tmp, "preserve") == 0) {
+			ctx->shared = E2F_SHARED_PRESERVE;
+		} else if (strcmp(tmp, "delete") == 0) {
+			ctx->shared = E2F_SHARED_DELETE;
+		} else if (strcmp(tmp, "lost+found") == 0) {
+			ctx->shared = E2F_SHARED_LPF;
+		} else {
+			com_err(ctx->program_name, 0,
+				_("unknown configuration option: 'shared=%s'"),
+				tmp);
+			fatal_error(ctx, 0);
+		}
+		free(tmp);
+	}
+
+	/* [options] clone=dup|zero */
+	tmp = NULL;
+	ctx->clone = E2F_CLONE_DUP;
+	profile_get_string(ctx->profile, "options", "clone", 0, "dup", &tmp);
+	if (tmp) {
+		if (strcmp(tmp, "dup") == 0){
+			ctx->clone = E2F_CLONE_DUP;
+		} else if (strcmp(tmp, "zero") == 0) {
+			ctx->clone = E2F_CLONE_ZERO;
+		} else {
+			com_err(ctx->program_name, 0,
+				_("unknown configuration option: 'clone=%s'"),
+				tmp);
+			fatal_error(ctx, 0);
+		}
+		free(tmp);
+	}
+}
+
 static void parse_extended_opts(e2fsck_t ctx, const char *opts)
 {
 	char	*buf, *token, *next, *p, *arg;
@@ -628,6 +671,36 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)
 		} else if (strcmp(token, "fragcheck") == 0) {
 			ctx->options |= E2F_OPT_FRAGCHECK;
 			continue;
+		/* -E shared=preserve|lost+found|delete */
+		} else if (strcmp(token, "shared") == 0) {
+			if (!arg) {
+				extended_usage++;
+				continue;
+			}
+			if (strcmp(arg, "preserve") == 0) {
+				ctx->shared = E2F_SHARED_PRESERVE;
+			} else if (strcmp(arg, "lost+found") == 0) {
+				ctx->shared = E2F_SHARED_LPF;
+			} else if (strcmp(arg, "delete") == 0) {
+				ctx->shared = E2F_SHARED_DELETE;
+			} else {
+				extended_usage++;
+				continue;
+			}
+		/* -E clone=dup|zero */
+		} else if (strcmp(token, "clone") == 0) {
+			if (!arg) {
+				extended_usage++;
+				continue;
+			}
+			if (strcmp(arg, "dup") == 0) {
+				ctx->clone = E2F_CLONE_DUP;
+			} else if (strcmp(arg, "zero") == 0) {
+				ctx->clone = E2F_CLONE_ZERO;
+			} else {
+				extended_usage++;
+				continue;
+			}
 		} else if (strcmp(token, "journal_only") == 0) {
 			if (arg) {
 				extended_usage++;
@@ -658,6 +731,8 @@ static void parse_extended_opts(e2fsck_t ctx, const char *opts)
 		fputs(("\tjournal_only\n"), stderr);
 		fputs(("\tdiscard\n"), stderr);
 		fputs(("\tnodiscard\n"), stderr);
+		fputs(("\tshared=<preserve|lost+found|delete>\n"), stderr);
+		fputs(("\tclone=<dup|zero>\n"), stderr);
 		fputc('\n', stderr);
 		exit(1);
 	}
@@ -717,6 +792,8 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
 	else
 		ctx->program_name = "e2fsck";
 
+	initialize_profile_options(ctx);
+
 	while ((c = getopt (argc, argv, "panyrcC:B:dE:fvtFVM:b:I:j:P:l:L:N:SsDk")) != EOF)
 		switch (c) {
 		case 'C':
-- 
1.7.3.4

Re: [PATCH 1/2] misc: Allow "-E" and "-O" options multiple times

[Ted Ts'o]

On Fri, Jul 08, 2011 at 06:49:03PM -0600, Andreas Dilger wrote:
> Allow "-E" and "-O" options to be specified multiple times on the
> command-line for mke2fs, tune2fs, and e2fsck, and parse them after
> the config file options to more closely match user expectations.

Well, we're doing that already for mke2fs.  As far as e2fsck is
concerned....

> @@ -883,14 +888,15 @@ static errcode_t PRS(int argc, char *argv[], e2fsck_t *ret_ctx)
>  			argv[optind]);
>  		fatal_error(ctx, 0);
>  	}
> -	if (extended_opts)
> -		parse_extended_opts(ctx, extended_opts);
>  
>  	if ((cp = getenv("E2FSCK_CONFIG")) != NULL)
>  		config_fn[0] = cp;
>  	profile_set_syntax_err_cb(syntax_err_report);
>  	profile_init(config_fn, &ctx->profile);
>  
> +	if (extended_opts)
> +		parse_extended_opts(ctx, extended_opts);
> +
>  	if (flush) {
>  		fd = open(ctx->filesystem_name, O_RDONLY, 0);
>  		if (fd < 0) {

Merely moving parse_extended_opts to after profile_init() doesn't
actually do anything.  All profile_init() does is to parse the
configuration file and load it into memory in a format where it can be
quickly and easily fetched via profile_get_{bool,string,int}().

If you want the command line options to override what is specified in
the config file, you have one of two options:

1)  Call profile_get_{bool,string,int}() and stash the result in a
variable, and then call parse_extended_opts(), and have that function
modify the configuration variable.

2) Have parse_extended_opts() modify a configuration variable, and
then if the configuration variable is set, then at the place in the
code where profile_get_{bool,string,int}() is called, don't call the
profile_get_* function.

In general, #1, is the better choice, but moving parse_extended_opts()
above is pointless.

							- Ted

Re: [PATCH 1/2] misc: Allow "-E" and "-O" options multiple times

[Ted Ts'o]

On Fri, Jul 08, 2011 at 06:49:03PM -0600, Andreas Dilger wrote:
> +errcode_t append_opts(char **old_opts, const char *added_opts)
> +{
> +	/* First string adds NUL, others add ',' */
> +	int newlen, oldlen = 0;
> +	errcode_t retval;
> +
> +	if (*old_opts != NULL)
> +		oldlen = strlen(*old_opts);
> +
> +	newlen = oldlen + strlen(added_opts) + 1;

Shouldn't this be "... + 2" instead?  Suppose old_opts is "foo", and
new_opts is "bar".  We need enough space for 8 characters: "foo,bar\0"

Also, can you do me a favor and rebase this patch series on top of the
most recent next branch of e2fsprogs.  The pass1b changes are going to
be impacted by the changes to support bigalloc which showed up in next
over the weekend.

Thanks!!

						- Ted