* [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names
@ 2015-04-22 16:44 Paul Evans
2015-04-22 18:01 ` Andrew Price
2015-04-22 18:09 ` Bob Peterson
0 siblings, 2 replies; 5+ messages in thread
From: Paul Evans @ 2015-04-22 16:44 UTC (permalink / raw)
To: cluster-devel.redhat.com
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 labelling gfs2 file
systems.
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.
Signed-off-by: Paul Evans <pevans@redhat.com>
---
gfs2/man/mkfs.gfs2.8 | 4 ++--
gfs2/mkfs/main_mkfs.c | 4 ++--
gfs2/tune/super.c | 2 +-
tests/mkfs.at | 8 ++++++++
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/gfs2/man/mkfs.gfs2.8 b/gfs2/man/mkfs.gfs2.8
index ceb6f38..f480082 100644
--- a/gfs2/man/mkfs.gfs2.8
+++ b/gfs2/man/mkfs.gfs2.8
@@ -101,9 +101,9 @@ bigger file systems will have bigger RGs for better performance.
The lock table field appropriate to the lock module you're using.
It is \fIclustername:fsname\fR.
Clustername must match that in cluster.conf; only members of this
-cluster are permitted to use this file system.
+cluster are permitted to use this file system (1 to 32 characters).
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
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..e25b6dc 100644
--- a/tests/mkfs.at
+++ b/tests/mkfs.at
@@ -89,3 +89,11 @@ 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 validation])
+AT_KEYWORDS(mkfs.gfs2 mkfs)
+AT_CHECK([$GFS_MKFS -p lock_dlm -t "" $GFS_TGT], 255, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_dlm -t "quite_long_cluster_name_test_here:intec34p" $GFS_TGT], 255, [ignore], [ignore])
+AT_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:this_time_we_test_fs_naming_len" $GFS_TGT], 255, [ignore], [ignore])
+GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "a_really_long_named_cluster_here:concurrently_lets_check_fs_len" $GFS_TGT])
+AT_CLEANUP
--
1.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names
2015-04-22 16:44 [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names Paul Evans
@ 2015-04-22 18:01 ` Andrew Price
2015-04-22 18:09 ` Bob Peterson
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Price @ 2015-04-22 18:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
On 22/04/15 17:44, 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 labelling gfs2 file
> systems.
>
> 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.
>
> Signed-off-by: Paul Evans <pevans@redhat.com>
Looks good to me.
Thanks,
Andy
> ---
> gfs2/man/mkfs.gfs2.8 | 4 ++--
> gfs2/mkfs/main_mkfs.c | 4 ++--
> gfs2/tune/super.c | 2 +-
> tests/mkfs.at | 8 ++++++++
> 4 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/gfs2/man/mkfs.gfs2.8 b/gfs2/man/mkfs.gfs2.8
> index ceb6f38..f480082 100644
> --- a/gfs2/man/mkfs.gfs2.8
> +++ b/gfs2/man/mkfs.gfs2.8
> @@ -101,9 +101,9 @@ bigger file systems will have bigger RGs for better performance.
> The lock table field appropriate to the lock module you're using.
> It is \fIclustername:fsname\fR.
> Clustername must match that in cluster.conf; only members of this
> -cluster are permitted to use this file system.
> +cluster are permitted to use this file system (1 to 32 characters).
> 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
> 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..e25b6dc 100644
> --- a/tests/mkfs.at
> +++ b/tests/mkfs.at
> @@ -89,3 +89,11 @@ 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 validation])
> +AT_KEYWORDS(mkfs.gfs2 mkfs)
> +AT_CHECK([$GFS_MKFS -p lock_dlm -t "" $GFS_TGT], 255, [ignore], [ignore])
> +AT_CHECK([$GFS_MKFS -p lock_dlm -t "quite_long_cluster_name_test_here:intec34p" $GFS_TGT], 255, [ignore], [ignore])
> +AT_CHECK([$GFS_MKFS -p lock_dlm -t "financial_cluster:this_time_we_test_fs_naming_len" $GFS_TGT], 255, [ignore], [ignore])
> +GFS_FSCK_CHECK([$GFS_MKFS -p lock_dlm -t "a_really_long_named_cluster_here:concurrently_lets_check_fs_len" $GFS_TGT])
> +AT_CLEANUP
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names
2015-04-22 16:44 [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names Paul Evans
2015-04-22 18:01 ` Andrew Price
@ 2015-04-22 18:09 ` Bob Peterson
1 sibling, 0 replies; 5+ messages in thread
From: Bob Peterson @ 2015-04-22 18:09 UTC (permalink / raw)
To: cluster-devel.redhat.com
----- Original Message -----
> 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 labelling gfs2 file
> systems.
>
> 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.
>
> Signed-off-by: Paul Evans <pevans@redhat.com>
> ---
> gfs2/man/mkfs.gfs2.8 | 4 ++--
> gfs2/mkfs/main_mkfs.c | 4 ++--
> gfs2/tune/super.c | 2 +-
> tests/mkfs.at | 8 ++++++++
> 4 files changed, 13 insertions(+), 5 deletions(-)
Hi,
ACK
Bob Peterson
Red Hat File Systems
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names
@ 2015-04-21 15:52 Paul Evans
2015-04-21 17:01 ` Andrew Price
0 siblings, 1 reply; 5+ messages in thread
From: Paul Evans @ 2015-04-21 15:52 UTC (permalink / raw)
To: cluster-devel.redhat.com
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(-)
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 (_).
.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
--
1.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names
2015-04-21 15:52 Paul Evans
@ 2015-04-21 17:01 ` Andrew Price
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Price @ 2015-04-21 17:01 UTC (permalink / raw)
To: cluster-devel.redhat.com
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-04-22 18:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-22 16:44 [Cluster-devel] [PATCH] mkfs.gfs2: Allow longer cluster names Paul Evans
2015-04-22 18:01 ` Andrew Price
2015-04-22 18:09 ` Bob Peterson
-- strict thread matches above, loose matches on Subject: below --
2015-04-21 15:52 Paul Evans
2015-04-21 17:01 ` Andrew Price
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).