Skip to contentSkip to conversation

resize2fs: add debug switch to use old online interface

PATCH In review Eric Sandeen 3 participants 7 messages Jan 11, 2013 – Jan 14, 2013

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 resize2fs: add debug switch to use old online interface

The old online resize ioctl interfaces are still present in the kernel; add a debug switch to resize2fs to be able to test them.

---

resize/online.c +8 -4
index d3d3546..9b38ac8 100644
@@ -22,7 +22,7 @@ extern char *program_name;
 #define MAX_32_NUM ((((unsigned long long) 1) << 32) - 1)
 
 errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
-			   blk64_t *new_size, int flags EXT2FS_ATTR((unused)))
+			   blk64_t *new_size, int flags)
 {
 #ifdef __linux__
 	struct ext2_new_group_input input;
@@ -94,9 +94,11 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 		exit(1);
 	}
 
-	if (ioctl(fd, EXT4_IOC_RESIZE_FS, new_size)) {
+	if (flags & RESIZE_DEBUG_OLD ||
+	    ioctl(fd, EXT4_IOC_RESIZE_FS, new_size)) {
 		/*
-		 * If kernel does not support EXT4_IOC_RESIZE_FS, use the
+		 * If kernel does not support EXT4_IOC_RESIZE_FS, or -d 16
+		 * is specified on the resize2fs command line, use the
 		 * old online resize. Note that the old approach does not
 		 * handle >32 bit file systems
 		 *
@@ -109,7 +111,9 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 		 * in the kernel sources.  This is probably a kernel
 		 * bug, but work around it here.
 		 */
-		if ((errno != ENOTTY) && (errno != EINVAL)) {
+		if (flags & RESIZE_DEBUG_OLD) {
+			printf("Old resize interface requested.\n");
+		} else if ((errno != ENOTTY) && (errno != EINVAL)) {
 			if (errno == EPERM)
 				com_err(program_name, 0,
 				_("Permission denied to resize filesystem"));
resize/resize2fs.8.in +2 -0
index 9ff6e0a..766c4d8 100644
@@ -99,6 +99,8 @@ from the following list:
 	4	\-\ Debug inode relocations
 .br
 	8	\-\ Debug moving the inode table
+.br
+	16	\-\ Debug old online resize interface
 .TP 
 .B \-f
 Forces resize2fs to proceed with the filesystem resize operation, overriding
resize/resize2fs.h +2 -0
index 2184759..58fc04d 100644
@@ -76,6 +76,8 @@ typedef struct ext2_sim_progress *ext2_sim_progmeter;
 #define RESIZE_DEBUG_BMOVE		0x0002
 #define RESIZE_DEBUG_INODEMAP		0x0004
 #define RESIZE_DEBUG_ITABLEMOVE		0x0008
+/* old online resize interface */
+#define RESIZE_DEBUG_OLD		0x0010
 
 #define RESIZE_PERCENT_COMPLETE		0x0100
 #define RESIZE_VERBOSE			0x0200
#1 lore raw

resize/resize2fs.h typedef struct ext2_sim_progress *ext2_sim_progmeter; · line 80

Quoted 8 lines · Eric Sandeen
resize/resize2fs.h +2 -0
@@ -76,6 +76,8 @@ typedef struct ext2_sim_progress *ext2_sim_progmeter;
 #define RESIZE_DEBUG_BMOVE		0x0002
 #define RESIZE_DEBUG_INODEMAP		0x0004
 #define RESIZE_DEBUG_ITABLEMOVE		0x0008
+/* old online resize interface */
+#define RESIZE_DEBUG_OLD		0x0010

0x0010 is already used for

#define RESIZE_DEBUG_RTRACK 0x0010

(which is very useful for performance tuning resize2fs).

I wonder if we would be better off using a environment variable for this? Mainly because currently, the current resize flags don't actually change the behavior of resize2fs, but just add different levels of debugging messages.

- Ted

#2 lore raw
Quoted 12 lines · Theodore Ts'o

resize/resize2fs.h typedef struct ext2_sim_progress *ext2_sim_progmeter; · line 80

Quoted 8 lines · Eric Sandeen
resize/resize2fs.h +2 -0
@@ -76,6 +76,8 @@ typedef struct ext2_sim_progress *ext2_sim_progmeter;
 #define RESIZE_DEBUG_BMOVE		0x0002
 #define RESIZE_DEBUG_INODEMAP		0x0004
 #define RESIZE_DEBUG_ITABLEMOVE		0x0008
+/* old online resize interface */
+#define RESIZE_DEBUG_OLD		0x0010

0x0010 is already used for

Quoted 3 lines

#define RESIZE_DEBUG_RTRACK 0x0010

(which is very useful for performance tuning resize2fs).

Ugh, how did I miss that. Sorry.

Quoted 4 lines

I wonder if we would be better off using a environment variable for this? Mainly because currently, the current resize flags don't actually change the behavior of resize2fs, but just add different levels of debugging messages.

The mechanism doesn't matter to me, we just need to be able to test it if the interface is in the kernel. I had originally had an "-o" option for "old interface" but that didn't seem great.

-Eric

Quoted 3 lines

- Ted

#3 lore raw
Quoted 4 lines · Eric Sandeen

The mechanism doesn't matter to me, we just need to be able to test it if the interface is in the kernel. I had originally had an "-o" option for "old interface" but that didn't seem great.

How about this instead?

- Ted

Quoted 1 line

From d1d0b92a2e031b8962f7c3f4f97983db88c82969 Mon Sep 17 00:00:00 2001

From: Theodore Ts'o <[email protected]>
Date: Sat, 12 Jan 2013 17:33:44 -0500
Subject: [PATCH] resize2fs: add debugging code to test the old online
resizing interfaces

The old online resize ioctl interfaces are still present in the kernel, and we want to make it easy to test both the kernel code for those older interfaces, and resize2fs's use of those interfaces in a relatively easy manner.

To do this, resize2fs will now check the environment variable RESIZE2FS_KERNEL_VERSION. If the version given is less than 3.3, then do not try using the new resizing ioctl, but instead use the resizing ioctls that were used before Linux version 3.3.

If the version given is less than 3.7, then emulate sanity checks which get done to protect against the fact that the new resizing ioctl prior to 3.7 did not handle meta_bg resizing. (This was previously tested via the presence of the RESIZE2FS_NO_META_BG_RESIZE environment variable. But the new environment variable, RESIZE2FS_KERNEL_VERISON, is more general.)

---
 resize/online.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)
resize/online.c +43 -2
index d3d3546..2d34640 100644
@@ -21,6 +21,33 @@ extern char *program_name;
 
 #define MAX_32_NUM ((((unsigned long long) 1) << 32) - 1)
 
+#ifdef __linux__
+static int parse_version_number(const char *s)
+{
+	int	major, minor, rev;
+	char	*endptr;
+	const char *cp = s;
+
+	if (!s)
+		return 0;
+	major = strtol(cp, &endptr, 10);
+	if (cp == endptr || *endptr != '.')
+		return 0;
+	cp = endptr + 1;
+	minor = strtol(cp, &endptr, 10);
+	if (cp == endptr || *endptr != '.')
+		return 0;
+	cp = endptr + 1;
+	rev = strtol(cp, &endptr, 10);
+	if (cp == endptr)
+		return 0;
+	return ((((major * 256) + minor) * 256) + rev);
+}
+
+#define VERSION_CODE(a,b,c) (((a) << 16) + ((b) << 8) + (c))
+
+#endif
+
 errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 			   blk64_t *new_size, int flags EXT2FS_ATTR((unused)))
 {
@@ -36,6 +63,18 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 	blk_t			size;
 	int			fd, overhead;
 	int			use_old_ioctl = 1;
+	int			no_meta_bg_resize = 0;
+	int			no_resize_ioctl = 0;
+
+	if (getenv("RESIZE2FS_KERNEL_VERSION")) {
+		char *version_to_emulate = getenv("RESIZE2FS_KERNEL_VERSION");
+		int kvers = parse_version_number(version_to_emulate);
+
+		if (kvers < VERSION_CODE(3, 7, 0))
+			no_meta_bg_resize = 1;
+		if (kvers < VERSION_CODE(3, 3, 0))
+			no_resize_ioctl = 1;
+	}
 
 	printf(_("Filesystem at %s is mounted on %s; "
 		 "on-line resizing required\n"), fs->device_name, mtpt);
@@ -61,7 +100,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 	 * Do error checking to make sure the resize will be successful.
 	 */
 	if ((access("/sys/fs/ext4/features/meta_bg_resize", R_OK) != 0) ||
-	    getenv("RESIZE2FS_NO_META_BG_RESIZE")) {
+	    no_meta_bg_resize) {
 		if (!EXT2_HAS_COMPAT_FEATURE(fs->super,
 					EXT2_FEATURE_COMPAT_RESIZE_INODE) &&
 		    (new_desc_blocks != fs->desc_blocks)) {
@@ -94,7 +133,9 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 		exit(1);
 	}
 
-	if (ioctl(fd, EXT4_IOC_RESIZE_FS, new_size)) {
+	if (no_resize_ioctl) {
+		printf(_("Old resize interface requested.\n"));
+	} else if (ioctl(fd, EXT4_IOC_RESIZE_FS, new_size)) {
 		/*
 		 * If kernel does not support EXT4_IOC_RESIZE_FS, use the
 		 * old online resize. Note that the old approach does not
Signature · 1 line
1.7.12.rc0.22.gcdd159b
#4 lore raw
Quoted 10 lines · Theodore Ts'o
Quoted 4 lines · Eric Sandeen

The mechanism doesn't matter to me, we just need to be able to test it if the interface is in the kernel. I had originally had an "-o" option for "old interface" but that didn't seem great.

How about this instead?

- Ted

Hi,

the patch looks good, but I'm very concerned about how much problems this will cause to stable releases and enterprise products which use to work with older kernel versions but a lot of backports of new features/bug fixes. Probably upstream stable releases will not be too much affected giving these features (new resize interface) might never be ported to stable kernels, but how about the backporters? This mightl cause a lot of confusion when changing the major/minor versions needed here.

IMHO, the new resize interface looks much better than the old one, and just developers and users which do not have new interface should make use of the old one. For developers I believe a resize2fs option is good enough to be used as debug, and for users, well, if they do not have the new resize2fs, we should do noting, once they'll use the old one automatically :-)

Quoted 108 lines

From d1d0b92a2e031b8962f7c3f4f97983db88c82969 Mon Sep 17 00:00:00 2001
From: Theodore Ts'o <[email protected]>
Date: Sat, 12 Jan 2013 17:33:44 -0500
Subject: [PATCH] resize2fs: add debugging code to test the old online
resizing interfaces

The old online resize ioctl interfaces are still present in the kernel, and we want to make it easy to test both the kernel code for those older interfaces, and resize2fs's use of those interfaces in a relatively easy manner.

To do this, resize2fs will now check the environment variable RESIZE2FS_KERNEL_VERSION. If the version given is less than 3.3, then do not try using the new resizing ioctl, but instead use the resizing ioctls that were used before Linux version 3.3.

If the version given is less than 3.7, then emulate sanity checks which get done to protect against the fact that the new resizing ioctl prior to 3.7 did not handle meta_bg resizing. (This was previously tested via the presence of the RESIZE2FS_NO_META_BG_RESIZE environment variable. But the new environment variable, RESIZE2FS_KERNEL_VERISON, is more general.)

---
 resize/online.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)
resize/online.c +43 -2
index d3d3546..2d34640 100644
@@ -21,6 +21,33 @@ extern char *program_name;
 
 #define MAX_32_NUM ((((unsigned long long) 1) << 32) - 1)
 
+#ifdef __linux__
+static int parse_version_number(const char *s)
+{
+	int	major, minor, rev;
+	char	*endptr;
+	const char *cp = s;
+
+	if (!s)
+		return 0;
+	major = strtol(cp, &endptr, 10);
+	if (cp == endptr || *endptr != '.')
+		return 0;
+	cp = endptr + 1;
+	minor = strtol(cp, &endptr, 10);
+	if (cp == endptr || *endptr != '.')
+		return 0;
+	cp = endptr + 1;
+	rev = strtol(cp, &endptr, 10);
+	if (cp == endptr)
+		return 0;
+	return ((((major * 256) + minor) * 256) + rev);
+}
+
+#define VERSION_CODE(a,b,c) (((a) << 16) + ((b) << 8) + (c))
+
+#endif
+
 errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 			   blk64_t *new_size, int flags EXT2FS_ATTR((unused)))
 {
@@ -36,6 +63,18 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 	blk_t			size;
 	int			fd, overhead;
 	int			use_old_ioctl = 1;
+	int			no_meta_bg_resize = 0;
+	int			no_resize_ioctl = 0;
+
+	if (getenv("RESIZE2FS_KERNEL_VERSION")) {
+		char *version_to_emulate = getenv("RESIZE2FS_KERNEL_VERSION");
+		int kvers = parse_version_number(version_to_emulate);
+
+		if (kvers < VERSION_CODE(3, 7, 0))
+			no_meta_bg_resize = 1;
+		if (kvers < VERSION_CODE(3, 3, 0))
+			no_resize_ioctl = 1;
+	}
 
 	printf(_("Filesystem at %s is mounted on %s; "
 		 "on-line resizing required\n"), fs->device_name, mtpt);
@@ -61,7 +100,7 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 	 * Do error checking to make sure the resize will be successful.
 	 */
 	if ((access("/sys/fs/ext4/features/meta_bg_resize", R_OK) != 0) ||
-	    getenv("RESIZE2FS_NO_META_BG_RESIZE")) {
+	    no_meta_bg_resize) {
 		if (!EXT2_HAS_COMPAT_FEATURE(fs->super,
 					EXT2_FEATURE_COMPAT_RESIZE_INODE) &&
 		    (new_desc_blocks != fs->desc_blocks)) {
@@ -94,7 +133,9 @@ errcode_t online_resize_fs(ext2_filsys fs, const char *mtpt,
 		exit(1);
 	}
 
-	if (ioctl(fd, EXT4_IOC_RESIZE_FS, new_size)) {
+	if (no_resize_ioctl) {
+		printf(_("Old resize interface requested.\n"));
+	} else if (ioctl(fd, EXT4_IOC_RESIZE_FS, new_size)) {
 		/*
 		 * If kernel does not support EXT4_IOC_RESIZE_FS, use the
 		 * old online resize. Note that the old approach does not
Signature · 1 line
1.7.12.rc0.22.gcdd159b
Signature · 1 line
Carlos
#5 lore raw
Quoted 8 lines · Carlos Maiolino

the patch looks good, but I'm very concerned about how much problems this will cause to stable releases and enterprise products which use to work with older kernel versions but a lot of backports of new features/bug fixes. Probably upstream stable releases will not be too much affected giving these features (new resize interface) might never be ported to stable kernels, but how about the backporters? This mightl cause a lot of confusion when changing the major/minor versions needed here.

Quoted 5 lines

IMHO, the new resize interface looks much better than the old one, and just developers and users which do not have new interface should make use of the old one. For developers I believe a resize2fs option is good enough to be used as debug, and for users, well, if they do not have the new resize2fs, we should do noting, once they'll use the old one automatically :-)

Well, this environment variable was intended only for developers who would be doing testing. Note that I didn't bother to documenting both the older debugging environment variable, nor this new debugging environment variable. It would problably be a good idea for me to write up the changes in the resize2fs interface (i.e., the new resize ioctl in 3.3, meta_bg / 64-bit resizing in 3.7) in the ext4 wiki, if for no other reason so that less sophisticated distributions _know_ which commits they should back port so they can have a fully working resize2fs. :-)

- Ted

#6 lore raw
Quoted 25 lines · Theodore Ts'o
Quoted 8 lines · Carlos Maiolino

the patch looks good, but I'm very concerned about how much problems this will cause to stable releases and enterprise products which use to work with older kernel versions but a lot of backports of new features/bug fixes. Probably upstream stable releases will not be too much affected giving these features (new resize interface) might never be ported to stable kernels, but how about the backporters? This mightl cause a lot of confusion when changing the major/minor versions needed here.

Quoted 5 lines

IMHO, the new resize interface looks much better than the old one, and just developers and users which do not have new interface should make use of the old one. For developers I believe a resize2fs option is good enough to be used as debug, and for users, well, if they do not have the new resize2fs, we should do noting, once they'll use the old one automatically :-)

Well, this environment variable was intended only for developers who would be doing testing. Note that I didn't bother to documenting both the older debugging environment variable, nor this new debugging environment variable. It would problably be a good idea for me to write up the changes in the resize2fs interface (i.e., the new resize ioctl in 3.3, meta_bg / 64-bit resizing in 3.7) in the ext4 wiki, if for no other reason so that less sophisticated distributions _know_ which commits they should back port so they can have a fully working resize2fs. :-)

Quoted 1 line

- Ted

This looks good for me, thanks =]

Signature · 1 line
Carlos