mkfs.gfs2: Allow longer cluster names
PATCH 1/1 mkfs.gfs2: Allow longer cluster names
Increase the enforced limit for cluster name to 32 bytes and file system name to 30 bytes for mkfs.gfs2 (was previously 16 + 16 bytes).
Also increased this limit in tunegfs2 when relabling gfs2 file sytems.
Updated the formation in the man pages along with adding a new test case for mkfs.gfs2 to validate the increased cluster/file system name support.
--- gfs2/man/mkfs.gfs2.8 | 2 +- gfs2/mkfs/main_mkfs.c | 4 ++-- gfs2/tune/super.c | 2 +- tests/mkfs.at | 6 ++++++ 4 files changed, 10 insertions(+), 4 deletions(-)
gfs2/man/mkfs.gfs2.8 +1 -1
@@ -103,7 +103,7 @@ It is \fIclustername:fsname\fR. Clustername must match that in cluster.conf; only members of this cluster are permitted to use this file system. Fsname is a unique file system name used to distinguish this GFS2 file -system from others created (1 to 16 characters). Lock_nolock doesn't +system from others created (1 to 30 characters). Lock_nolock doesn't use this field. Valid \fIclustername\fRs and \fIfsname\fRs may only contain alphanumeric characters, hyphens (-) and underscores (_). .TP
gfs2/mkfs/main_mkfs.c +2 -2
@@ -398,7 +398,7 @@ static void test_locking(const char *lockproto, const char *locktable)if (c == locktable)die ("%s %s\n" , errprefix,_ ("cluster name is missing" )); -if (c - locktable >16 ) +if (c - locktable >32 )die ("%s %s\n" , errprefix,_ ("cluster name is too long" )); c++;
@@ -406,7 +406,7 @@ static void test_locking(const char *lockproto, const char *locktable)die ("%s %s\n" , errprefix,_ ("contains more than one colon" ));if (!strlen (c))die ("%s %s\n" , errprefix,_ ("file system name is missing" )); -if (strlen (c) >16 ) +if (strlen (c) >30 )die ("%s %s\n" , errprefix,_ ("file system name is too long" )); }else {die (_ ("Invalid lock protocol: %s\n" ), lockproto);
gfs2/tune/super.c +1 -1
@@ -194,7 +194,7 @@ int change_locktable(struct tunegfs2 *tfs, const char *locktable)fprintf (stderr,"%s %s\n" , errpre,_ ("missing colon" ));return EX_DATAERR ; } -if (strlen (++fsname) >16 ) { +if (strlen (++fsname) >30 ) {fprintf (stderr,"%s %s\n" , errpre,_ ("file system name is too long" ));return EX_DATAERR ; }
tests/mkfs.at +6 -0
@@ -89,3 +89,9 @@ AT_SETUP([Min. quota change file size]) AT_KEYWORDS(mkfs.gfs2 mkfs) GFS_FSCK_CHECK([$GFS_MKFS -p lock_nolock -c 1 $GFS_TGT]) AT_CLEANUP + +AT_SETUP([Incr. cluster/file system name support]) +AT_KEYWORDS(mkfs.gfs2 mkfs) +GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:intec34p" $GFS_TGT]) +GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:financial_volume-001" $GFS_TGT]) +AT_CLEANUP
Signature · 1 line
1.9.3
PATCH 1/1 mkfs.gfs2: Allow longer cluster names
Hi Paul,
Quoted 10 lines · Paul Evans
Increase the enforced limit for cluster name to 32 bytes and file system name to 30 bytes for mkfs.gfs2 (was previously 16 + 16 bytes).
Also increased this limit in tunegfs2 when relabling gfs2 file sytems.
Updated the formation in the man pages along with adding a new test case for mkfs.gfs2 to validate the increased cluster/file system name support.
Could you add a signed-off-by? git commit --amend -s will do that quickly.
Quoted 12 lines
--- gfs2/man/mkfs.gfs2.8 | 2 +- gfs2/mkfs/main_mkfs.c | 4 ++-- gfs2/tune/super.c | 2 +- tests/mkfs.at | 6 ++++++ 4 files changed, 10 insertions(+), 4 deletions(-)gfs2/man/mkfs.gfs2.8 +0
-0
gfs2/man/mkfs.gfs2.8 It is \fIclustername:fsname\fR. · line 108
Quoted 7 lines
gfs2/man/mkfs.gfs2.8 +1
-1@@ -103,7 +103,7 @@ It is \fIclustername:fsname\fR. Clustername must match that in cluster.conf; only members of this cluster are permitted to use this file system. Fsname is a unique file system name used to distinguish this GFS2 file -system from others created (1 to 16 characters). Lock_nolock doesn't +system from others created (1 to 30 characters). Lock_nolock doesn't use this field. Valid \fIclustername\fRs and \fIfsname\fRs may only contain alphanumeric characters, hyphens (-) and underscores (_).
Hm, the man page doesn't seem to mention the cluster name length limit, maybe we should add that.
Quoted 41 lines
gfs2/man/mkfs.gfs2.8 +0
-0.TPgfs2/mkfs/main_mkfs.c +2
-2@@ -398,7 +398,7 @@ static void test_locking(const char *lockproto, const char *locktable)if (c == locktable)die ("%s %s\n" , errprefix,_ ("cluster name is missing" )); -if (c - locktable >16 ) +if (c - locktable >32 )die ("%s %s\n" , errprefix,_ ("cluster name is too long" )); c++;@@ -406,7 +406,7 @@ static void test_locking(const char *lockproto, const char *locktable)die ("%s %s\n" , errprefix,_ ("contains more than one colon" ));if (!strlen (c))die ("%s %s\n" , errprefix,_ ("file system name is missing" )); -if (strlen (c) >16 ) +if (strlen (c) >30 )die ("%s %s\n" , errprefix,_ ("file system name is too long" )); }else {die (_ ("Invalid lock protocol: %s\n" ), lockproto);gfs2/tune/super.c +1
-1@@ -194,7 +194,7 @@ int change_locktable(struct tunegfs2 *tfs, const char *locktable)fprintf (stderr,"%s %s\n" , errpre,_ ("missing colon" ));return EX_DATAERR ; } -if (strlen (++fsname) >16 ) { +if (strlen (++fsname) >30 ) {fprintf (stderr,"%s %s\n" , errpre,_ ("file system name is too long" ));return EX_DATAERR ; }tests/mkfs.at +0
-0
tests/mkfs.at AT_SETUP([Min. quota change file size]) · line 97
Quoted 9 lines
tests/mkfs.at +6
-0@@ -89,3 +89,9 @@ AT_SETUP([Min. quota change file size]) AT_KEYWORDS(mkfs.gfs2 mkfs) GFS_FSCK_CHECK([$GFS_MKFS -p lock_nolock -c 1 $GFS_TGT]) AT_CLEANUP + +AT_SETUP([Incr. cluster/file system name support]) +AT_KEYWORDS(mkfs.gfs2 mkfs) +GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:intec34p" $GFS_TGT]) +GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:financial_volume-001" $GFS_TGT]) +AT_CLEANUP
Thanks for adding tests! Ideally they should exercise the validation code that you've changed, so boundary and invalid values would be more effective. Could you change these into tests for, e.g.:
Zero-length locktable -> expect mkfs failure (no need to fsck) Max fsname length, clustername 1 char too long -> ditto
Max clustername length, fsname 1 char too long -> ditto
Max fsname and clustername length -> expect success (and fsck success)
These kind of tests seem too trivial to be useful when you're writing them but I assure you they will catch silly regressions next time somebody decides to rework that code :)
Cheers,
Andy