Hi Paul,
On 21/04/15 16:52, Paul Evans wrote:
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.
---
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(-)
diff --git a/gfs2/man/mkfs.gfs2.8 b/gfs2/man/mkfs.gfs2.8
index ceb6f38..44483f1 100644
--- a/gfs2/man/mkfs.gfs2.8
+++ b/gfs2/man/mkfs.gfs2.8
@@ -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.
.TP
diff --git a/gfs2/mkfs/main_mkfs.c b/gfs2/mkfs/main_mkfs.c
index 0636f0b..3fab08c 100644
--- a/gfs2/mkfs/main_mkfs.c
+++ b/gfs2/mkfs/main_mkfs.c
@@ -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);
diff --git a/gfs2/tune/super.c b/gfs2/tune/super.c
index cbd0026..560ce68 100644
--- a/gfs2/tune/super.c
+++ b/gfs2/tune/super.c
@@ -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;
}
diff --git a/tests/mkfs.at b/tests/mkfs.at
index 438184c..786f30c 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -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
