Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=a523af5846…
Commit: a523af584645a01bb4900b10d0c9232d0ae50339
Parent: e394013ec812e7fa359344a3b901b8b15b15bc4a
Author: Paul Evans <pevans(a)redhat.com>
AuthorDate: Wed Apr 22 17:43:26 2015 +0100
Committer: Paul Evans <pevans(a)redhat.com>
CommitterDate: Fri Apr 24 17:09:45 2015 +0100
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 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.
Resolves: rhbz#1202831
Signed-off-by: Paul Evans <pevans(a)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
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=335552bcf1…
Commit: 335552bcf1345d9083690e1ef892fa856ac1e59e
Parent: f1028ec054f2d7f85a449b2bf0894e0435c01d6a
Author: Paul Evans <pevans(a)redhat.com>
AuthorDate: Wed Apr 22 17:43:26 2015 +0100
Committer: Paul Evans <pevans(a)redhat.com>
CommitterDate: Wed Apr 22 17:43:26 2015 +0100
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 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(a)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
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=e394013ec8…
Commit: e394013ec812e7fa359344a3b901b8b15b15bc4a
Parent: be3383e4f295c0d519c811ac9ace6bbbfa5f58e3
Author: Abhi Das <adas(a)redhat.com>
AuthorDate: Tue Apr 14 19:51:55 2015 -0500
Committer: Abhi Das <adas(a)redhat.com>
CommitterDate: Fri Apr 17 09:51:22 2015 -0500
fsck.gfs2: replace recent i_goal fixes with simple logic
This patch reverses the recent set of i_goal fixes for fsck.gfs2.
This is because of two problems.
1. It is not possible to determine if a valid block within the fs
is the correct goal block for a given inode.
2. Conversely, given an inode, it is also not possible to accurately
determine what its goal block should be.
The previous patches assumed that the last block of a file is its
goal block, but that is not true if the file is a directory or if
its blocks are not allocated sequentially. fsck.gfs2 would flag
these inodes incorrectly as having bad i_goal values.
This patch takes a simple approach. It checks if the i_goal of a
given inode is out of bounds of the fs. If so, we can be certain
that it is wrong and we set it to the inode metadata block. This
is a safe starting point for gfs2 to determine where to allocate
from next.
Resolves: rhbz#1186515
Signed-off-by: Abhi Das <adas(a)redhat.com>
---
gfs2/fsck/metawalk.c | 92 ++---------------------------------------------
gfs2/fsck/metawalk.h | 5 ---
gfs2/fsck/pass1.c | 35 +++++++++++++++---
gfs2/libgfs2/libgfs2.h | 1 -
4 files changed, 34 insertions(+), 99 deletions(-)
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index f05fb51..4d5a660 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1428,8 +1428,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
*/
static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
struct gfs2_buffer_head *bh, int head_size,
- uint64_t *last_block, uint64_t *blks_checked,
- uint64_t *error_blk)
+ uint64_t *blks_checked, uint64_t *error_blk)
{
int error = 0, rc = 0;
uint64_t block, *ptr;
@@ -1444,7 +1443,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
if (skip_this_pass || fsck_abort)
return error;
- *last_block = block = be64_to_cpu(*ptr);
+ block = be64_to_cpu(*ptr);
/* It's important that we don't call valid_block() and
bypass calling check_data on invalid blocks because that
would defeat the rangecheck_block related functions in
@@ -1548,15 +1547,12 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
struct gfs2_buffer_head *bh;
uint32_t height = ip->i_di.di_height;
int i, head_size;
- uint64_t blks_checked = 0, last_block = 0;
+ uint64_t blks_checked = 0;
int error, rc;
int metadata_clean = 0;
uint64_t error_blk = 0;
int hit_error_blk = 0;
- if (!height && pass->check_i_goal)
- pass->check_i_goal(ip, ip->i_di.di_num.no_addr,
- pass->private);
if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1))
return 0;
@@ -1575,9 +1571,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
* comprise the directory hash table, so we perform the directory
* checks and exit. */
if (is_dir(&ip->i_di, ip->i_sbd->gfs1)) {
- last_block = ip->i_di.di_num.no_addr;
- if (pass->check_i_goal)
- pass->check_i_goal(ip, last_block, pass->private);
if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH))
goto out;
/* check validity of leaf blocks and leaf chains */
@@ -1604,7 +1597,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
if (pass->check_data)
error = check_data(ip, pass, bh, head_size,
- &last_block, &blks_checked, &error_blk);
+ &blks_checked, &error_blk);
if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS)
pass->big_file_msg(ip, blks_checked);
}
@@ -1616,8 +1609,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
(unsigned long long)ip->i_di.di_num.no_addr);
fflush(stdout);
}
- if (!error && pass->check_i_goal)
- pass->check_i_goal(ip, last_block, pass->private);
undo_metalist:
if (!error)
goto out;
@@ -1958,80 +1949,6 @@ static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
return 0;
}
-/**
- * rgrp_contains_block - Check if the rgrp provided contains the
- * given block. Taken directly from the gfs2 kernel code
- * @rgd: The rgrp to search within
- * @block: The block to search for
- *
- * Returns: 1 if present, 0 if not.
- */
-static inline int rgrp_contains_block(struct rgrp_tree *rgd, uint64_t block)
-{
- uint64_t first = rgd->ri.ri_data0;
- uint64_t last = first + rgd->ri.ri_data;
- return first <= block && block < last;
-}
-
-/**
- * check_i_goal
- * @ip
- * @goal_blk: What the goal block should be for this inode
- *
- * The goal block for a regular file is typically the last
- * data block of the file. If we can't get the right value,
- * the inode metadata block is the next best thing.
- *
- * Returns: 0 if corrected, 1 if not corrected
- */
-int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
- void *private)
-{
- struct gfs2_sbd *sdp = ip->i_sbd;
- uint64_t i_block = ip->i_di.di_num.no_addr;
-
- /* Don't fix gfs1 inodes, system inodes or inodes whose goal blocks are
- * set to the inode blocks themselves. */
- if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM ||
- ip->i_di.di_goal_meta == i_block)
- return 0;
- /* Don't fix directory goal blocks unless we know they're wrong.
- * i.e. out of bounds of the fs. Directories can easily have blocks
- * outside of the dinode's rgrp and thus we have no way of knowing
- * if the goal block is bogus or not. */
- if (is_dir(&ip->i_di, ip->i_sbd->gfs1) &&
- (ip->i_di.di_goal_meta > sdp->sb_addr &&
- ip->i_di.di_goal_meta <= sdp->fssize))
- return 0;
- /* We default to the inode block */
- if (!goal_blk)
- goal_blk = i_block;
-
- if (ip->i_di.di_goal_meta != goal_blk) {
- /* If the existing goal block is in the same rgrp as the inode,
- * we give the benefit of doubt and assume the value is correct */
- if (ip->i_rgd &&
- rgrp_contains_block(ip->i_rgd, ip->i_di.di_goal_meta))
- goto skip;
- log_err( _("Error: inode %llu (0x%llx) has invalid "
- "allocation goal block %llu (0x%llx). Should"
- " be %llu (0x%llx)\n"),
- (unsigned long long)i_block, (unsigned long long)i_block,
- (unsigned long long)ip->i_di.di_goal_meta,
- (unsigned long long)ip->i_di.di_goal_meta,
- (unsigned long long)goal_blk, (unsigned long long)goal_blk);
- if (query( _("Fix the invalid goal block? (y/n) "))) {
- ip->i_di.di_goal_meta = ip->i_di.di_goal_data = goal_blk;
- bmodified(ip->i_bh);
- } else {
- log_err(_("Invalid goal block not fixed.\n"));
- return 1;
- }
- }
-skip:
- return 0;
-}
-
struct metawalk_fxns alloc_fxns = {
.private = NULL,
.check_leaf = alloc_leaf,
@@ -2042,7 +1959,6 @@ struct metawalk_fxns alloc_fxns = {
.check_dentry = NULL,
.check_eattr_entry = NULL,
.check_eattr_extentry = NULL,
- .check_i_goal = check_i_goal,
.finish_eattr_indir = NULL,
};
diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
index 779360e..fa4c850 100644
--- a/gfs2/fsck/metawalk.h
+++ b/gfs2/fsck/metawalk.h
@@ -50,8 +50,6 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock,
const char *caller, int line);
extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
int error_on_dinode, int new_blockmap_state);
-extern int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
- void *private);
extern void reprocess_inode(struct gfs2_inode *ip, const char *desc);
extern struct duptree *dupfind(uint64_t block);
extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp,
@@ -91,7 +89,6 @@ enum meta_check_rc {
* check_dentry:
* check_eattr_entry:
* check_eattr_extentry:
- * check_i_goal:
*/
struct metawalk_fxns {
void *private;
@@ -143,8 +140,6 @@ struct metawalk_fxns {
struct gfs2_ea_header *ea_hdr,
struct gfs2_ea_header *ea_hdr_prev,
void *private);
- int (*check_i_goal) (struct gfs2_inode *ip, uint64_t goal_blk,
- void *private);
int (*finish_eattr_indir) (struct gfs2_inode *ip, int leaf_pointers,
int leaf_pointer_errors, void *private);
void (*big_file_msg) (struct gfs2_inode *ip, uint64_t blks_checked);
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 69c88f4..0909873 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -100,7 +100,6 @@ struct metawalk_fxns pass1_fxns = {
.check_dentry = NULL,
.check_eattr_entry = check_eattr_entries,
.check_eattr_extentry = check_extended_leaf_eattr,
- .check_i_goal = check_i_goal,
.finish_eattr_indir = finish_eattr_indir,
.big_file_msg = big_file_comfort,
.repair_leaf = pass1_repair_leaf,
@@ -1205,12 +1204,37 @@ bad_dinode:
return -1;
}
+static void check_i_goal(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
+{
+ if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM)
+ return;
+
+ if (ip->i_di.di_goal_meta <= sdp->sb_addr ||
+ ip->i_di.di_goal_meta > sdp->fssize) {
+ log_err(_("Inode #%llu (0x%llx): Bad allocation goal block "
+ "found: %llu (0x%llx)\n"),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_goal_meta,
+ (unsigned long long)ip->i_di.di_goal_meta);
+ if (query( _("Fix goal block in inode #%llu (0x%llx)? (y/n) "),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_num.no_addr)) {
+ ip->i_di.di_goal_meta = ip->i_di.di_num.no_addr;
+ bmodified(ip->i_bh);
+ } else
+ log_err(_("Allocation goal block in inode #%lld "
+ "(0x%llx) not fixed\n"),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_num.no_addr);
+ }
+}
+
/*
* handle_di - This is now a wrapper function that takes a gfs2_buffer_head
* and calls handle_ip, which takes an in-code dinode structure.
*/
-static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
- struct rgrp_tree *rgd)
+static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
{
int error = 0;
uint64_t block = bh->b_blocknr;
@@ -1252,7 +1276,7 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
(unsigned long long)block,
(unsigned long long)block);
}
- ip->i_rgd = rgd;
+ check_i_goal(sdp, ip);
error = handle_ip(sdp, ip);
fsck_inode_put(&ip);
return error;
@@ -1378,6 +1402,7 @@ static int check_system_inode(struct gfs2_sbd *sdp,
"directory entries.\n"), filename);
}
}
+ check_i_goal(sdp, *sysinode);
error = handle_ip(sdp, *sysinode);
return error ? error : err;
}
@@ -1602,7 +1627,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
(unsigned long long)block,
(unsigned long long)block);
check_n_fix_bitmap(sdp, block, 0, GFS2_BLKST_FREE);
- } else if (handle_di(sdp, bh, rgd) < 0) {
+ } else if (handle_di(sdp, bh) < 0) {
stack;
brelse(bh);
gfs2_special_free(&gfs1_rindex_blks);
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index f1f81d3..ccae721 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -233,7 +233,6 @@ struct gfs2_inode {
struct gfs2_dinode i_di;
struct gfs2_buffer_head *i_bh;
struct gfs2_sbd *i_sbd;
- struct rgrp_tree *i_rgd; /* The rgrp this inode is in */
};
struct master_dir
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=f1028ec054…
Commit: f1028ec054f2d7f85a449b2bf0894e0435c01d6a
Parent: 68b368b8c3cc168926820be922016bf2e40d51f5
Author: Abhi Das <adas(a)redhat.com>
AuthorDate: Tue Apr 14 19:51:55 2015 -0500
Committer: Abhi Das <adas(a)redhat.com>
CommitterDate: Tue Apr 14 19:51:55 2015 -0500
fsck.gfs2: replace recent i_goal fixes with simple logic
This patch reverses the recent set of i_goal fixes for fsck.gfs2.
This is because of two problems.
1. It is not possible to determine if a valid block within the fs
is the correct goal block for a given inode.
2. Conversely, given an inode, it is also not possible to accurately
determine what its goal block should be.
The previous patches assumed that the last block of a file is its
goal block, but that is not true if the file is a directory or if
its blocks are not allocated sequentially. fsck.gfs2 would flag
these inodes incorrectly as having bad i_goal values.
This patch takes a simple approach. It checks if the i_goal of a
given inode is out of bounds of the fs. If so, we can be certain
that it is wrong and we set it to the inode metadata block. This
is a safe starting point for gfs2 to determine where to allocate
from next.
Resolves: rhbz#1186515
Signed-off-by: Abhi Das <adas(a)redhat.com>
---
gfs2/fsck/metawalk.c | 92 ++---------------------------------------------
gfs2/fsck/metawalk.h | 5 ---
gfs2/fsck/pass1.c | 35 +++++++++++++++---
gfs2/libgfs2/libgfs2.h | 1 -
4 files changed, 34 insertions(+), 99 deletions(-)
diff --git a/gfs2/fsck/metawalk.c b/gfs2/fsck/metawalk.c
index f05fb51..4d5a660 100644
--- a/gfs2/fsck/metawalk.c
+++ b/gfs2/fsck/metawalk.c
@@ -1428,8 +1428,7 @@ static int build_and_check_metalist(struct gfs2_inode *ip, osi_list_t *mlp,
*/
static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
struct gfs2_buffer_head *bh, int head_size,
- uint64_t *last_block, uint64_t *blks_checked,
- uint64_t *error_blk)
+ uint64_t *blks_checked, uint64_t *error_blk)
{
int error = 0, rc = 0;
uint64_t block, *ptr;
@@ -1444,7 +1443,7 @@ static int check_data(struct gfs2_inode *ip, struct metawalk_fxns *pass,
if (skip_this_pass || fsck_abort)
return error;
- *last_block = block = be64_to_cpu(*ptr);
+ block = be64_to_cpu(*ptr);
/* It's important that we don't call valid_block() and
bypass calling check_data on invalid blocks because that
would defeat the rangecheck_block related functions in
@@ -1548,15 +1547,12 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
struct gfs2_buffer_head *bh;
uint32_t height = ip->i_di.di_height;
int i, head_size;
- uint64_t blks_checked = 0, last_block = 0;
+ uint64_t blks_checked = 0;
int error, rc;
int metadata_clean = 0;
uint64_t error_blk = 0;
int hit_error_blk = 0;
- if (!height && pass->check_i_goal)
- pass->check_i_goal(ip, ip->i_di.di_num.no_addr,
- pass->private);
if (!height && !is_dir(&ip->i_di, ip->i_sbd->gfs1))
return 0;
@@ -1575,9 +1571,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
* comprise the directory hash table, so we perform the directory
* checks and exit. */
if (is_dir(&ip->i_di, ip->i_sbd->gfs1)) {
- last_block = ip->i_di.di_num.no_addr;
- if (pass->check_i_goal)
- pass->check_i_goal(ip, last_block, pass->private);
if (!(ip->i_di.di_flags & GFS2_DIF_EXHASH))
goto out;
/* check validity of leaf blocks and leaf chains */
@@ -1604,7 +1597,7 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
if (pass->check_data)
error = check_data(ip, pass, bh, head_size,
- &last_block, &blks_checked, &error_blk);
+ &blks_checked, &error_blk);
if (pass->big_file_msg && ip->i_di.di_blocks > COMFORTABLE_BLKS)
pass->big_file_msg(ip, blks_checked);
}
@@ -1616,8 +1609,6 @@ int check_metatree(struct gfs2_inode *ip, struct metawalk_fxns *pass)
(unsigned long long)ip->i_di.di_num.no_addr);
fflush(stdout);
}
- if (!error && pass->check_i_goal)
- pass->check_i_goal(ip, last_block, pass->private);
undo_metalist:
if (!error)
goto out;
@@ -1958,80 +1949,6 @@ static int alloc_leaf(struct gfs2_inode *ip, uint64_t block, void *private)
return 0;
}
-/**
- * rgrp_contains_block - Check if the rgrp provided contains the
- * given block. Taken directly from the gfs2 kernel code
- * @rgd: The rgrp to search within
- * @block: The block to search for
- *
- * Returns: 1 if present, 0 if not.
- */
-static inline int rgrp_contains_block(struct rgrp_tree *rgd, uint64_t block)
-{
- uint64_t first = rgd->ri.ri_data0;
- uint64_t last = first + rgd->ri.ri_data;
- return first <= block && block < last;
-}
-
-/**
- * check_i_goal
- * @ip
- * @goal_blk: What the goal block should be for this inode
- *
- * The goal block for a regular file is typically the last
- * data block of the file. If we can't get the right value,
- * the inode metadata block is the next best thing.
- *
- * Returns: 0 if corrected, 1 if not corrected
- */
-int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
- void *private)
-{
- struct gfs2_sbd *sdp = ip->i_sbd;
- uint64_t i_block = ip->i_di.di_num.no_addr;
-
- /* Don't fix gfs1 inodes, system inodes or inodes whose goal blocks are
- * set to the inode blocks themselves. */
- if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM ||
- ip->i_di.di_goal_meta == i_block)
- return 0;
- /* Don't fix directory goal blocks unless we know they're wrong.
- * i.e. out of bounds of the fs. Directories can easily have blocks
- * outside of the dinode's rgrp and thus we have no way of knowing
- * if the goal block is bogus or not. */
- if (is_dir(&ip->i_di, ip->i_sbd->gfs1) &&
- (ip->i_di.di_goal_meta > sdp->sb_addr &&
- ip->i_di.di_goal_meta <= sdp->fssize))
- return 0;
- /* We default to the inode block */
- if (!goal_blk)
- goal_blk = i_block;
-
- if (ip->i_di.di_goal_meta != goal_blk) {
- /* If the existing goal block is in the same rgrp as the inode,
- * we give the benefit of doubt and assume the value is correct */
- if (ip->i_rgd &&
- rgrp_contains_block(ip->i_rgd, ip->i_di.di_goal_meta))
- goto skip;
- log_err( _("Error: inode %llu (0x%llx) has invalid "
- "allocation goal block %llu (0x%llx). Should"
- " be %llu (0x%llx)\n"),
- (unsigned long long)i_block, (unsigned long long)i_block,
- (unsigned long long)ip->i_di.di_goal_meta,
- (unsigned long long)ip->i_di.di_goal_meta,
- (unsigned long long)goal_blk, (unsigned long long)goal_blk);
- if (query( _("Fix the invalid goal block? (y/n) "))) {
- ip->i_di.di_goal_meta = ip->i_di.di_goal_data = goal_blk;
- bmodified(ip->i_bh);
- } else {
- log_err(_("Invalid goal block not fixed.\n"));
- return 1;
- }
- }
-skip:
- return 0;
-}
-
struct metawalk_fxns alloc_fxns = {
.private = NULL,
.check_leaf = alloc_leaf,
@@ -2042,7 +1959,6 @@ struct metawalk_fxns alloc_fxns = {
.check_dentry = NULL,
.check_eattr_entry = NULL,
.check_eattr_extentry = NULL,
- .check_i_goal = check_i_goal,
.finish_eattr_indir = NULL,
};
diff --git a/gfs2/fsck/metawalk.h b/gfs2/fsck/metawalk.h
index 779360e..fa4c850 100644
--- a/gfs2/fsck/metawalk.h
+++ b/gfs2/fsck/metawalk.h
@@ -50,8 +50,6 @@ extern int _fsck_blockmap_set(struct gfs2_inode *ip, uint64_t bblock,
const char *caller, int line);
extern int check_n_fix_bitmap(struct gfs2_sbd *sdp, uint64_t blk,
int error_on_dinode, int new_blockmap_state);
-extern int check_i_goal(struct gfs2_inode *ip, uint64_t goal_blk,
- void *private);
extern void reprocess_inode(struct gfs2_inode *ip, const char *desc);
extern struct duptree *dupfind(uint64_t block);
extern struct gfs2_inode *fsck_system_inode(struct gfs2_sbd *sdp,
@@ -91,7 +89,6 @@ enum meta_check_rc {
* check_dentry:
* check_eattr_entry:
* check_eattr_extentry:
- * check_i_goal:
*/
struct metawalk_fxns {
void *private;
@@ -143,8 +140,6 @@ struct metawalk_fxns {
struct gfs2_ea_header *ea_hdr,
struct gfs2_ea_header *ea_hdr_prev,
void *private);
- int (*check_i_goal) (struct gfs2_inode *ip, uint64_t goal_blk,
- void *private);
int (*finish_eattr_indir) (struct gfs2_inode *ip, int leaf_pointers,
int leaf_pointer_errors, void *private);
void (*big_file_msg) (struct gfs2_inode *ip, uint64_t blks_checked);
diff --git a/gfs2/fsck/pass1.c b/gfs2/fsck/pass1.c
index 69c88f4..0909873 100644
--- a/gfs2/fsck/pass1.c
+++ b/gfs2/fsck/pass1.c
@@ -100,7 +100,6 @@ struct metawalk_fxns pass1_fxns = {
.check_dentry = NULL,
.check_eattr_entry = check_eattr_entries,
.check_eattr_extentry = check_extended_leaf_eattr,
- .check_i_goal = check_i_goal,
.finish_eattr_indir = finish_eattr_indir,
.big_file_msg = big_file_comfort,
.repair_leaf = pass1_repair_leaf,
@@ -1205,12 +1204,37 @@ bad_dinode:
return -1;
}
+static void check_i_goal(struct gfs2_sbd *sdp, struct gfs2_inode *ip)
+{
+ if (sdp->gfs1 || ip->i_di.di_flags & GFS2_DIF_SYSTEM)
+ return;
+
+ if (ip->i_di.di_goal_meta <= sdp->sb_addr ||
+ ip->i_di.di_goal_meta > sdp->fssize) {
+ log_err(_("Inode #%llu (0x%llx): Bad allocation goal block "
+ "found: %llu (0x%llx)\n"),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_goal_meta,
+ (unsigned long long)ip->i_di.di_goal_meta);
+ if (query( _("Fix goal block in inode #%llu (0x%llx)? (y/n) "),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_num.no_addr)) {
+ ip->i_di.di_goal_meta = ip->i_di.di_num.no_addr;
+ bmodified(ip->i_bh);
+ } else
+ log_err(_("Allocation goal block in inode #%lld "
+ "(0x%llx) not fixed\n"),
+ (unsigned long long)ip->i_di.di_num.no_addr,
+ (unsigned long long)ip->i_di.di_num.no_addr);
+ }
+}
+
/*
* handle_di - This is now a wrapper function that takes a gfs2_buffer_head
* and calls handle_ip, which takes an in-code dinode structure.
*/
-static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
- struct rgrp_tree *rgd)
+static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh)
{
int error = 0;
uint64_t block = bh->b_blocknr;
@@ -1252,7 +1276,7 @@ static int handle_di(struct gfs2_sbd *sdp, struct gfs2_buffer_head *bh,
(unsigned long long)block,
(unsigned long long)block);
}
- ip->i_rgd = rgd;
+ check_i_goal(sdp, ip);
error = handle_ip(sdp, ip);
fsck_inode_put(&ip);
return error;
@@ -1378,6 +1402,7 @@ static int check_system_inode(struct gfs2_sbd *sdp,
"directory entries.\n"), filename);
}
}
+ check_i_goal(sdp, *sysinode);
error = handle_ip(sdp, *sysinode);
return error ? error : err;
}
@@ -1602,7 +1627,7 @@ static int pass1_process_bitmap(struct gfs2_sbd *sdp, struct rgrp_tree *rgd, uin
(unsigned long long)block,
(unsigned long long)block);
check_n_fix_bitmap(sdp, block, 0, GFS2_BLKST_FREE);
- } else if (handle_di(sdp, bh, rgd) < 0) {
+ } else if (handle_di(sdp, bh) < 0) {
stack;
brelse(bh);
gfs2_special_free(&gfs1_rindex_blks);
diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h
index f1f81d3..ccae721 100644
--- a/gfs2/libgfs2/libgfs2.h
+++ b/gfs2/libgfs2/libgfs2.h
@@ -233,7 +233,6 @@ struct gfs2_inode {
struct gfs2_dinode i_di;
struct gfs2_buffer_head *i_bh;
struct gfs2_sbd *i_sbd;
- struct rgrp_tree *i_rgd; /* The rgrp this inode is in */
};
struct master_dir
Gitweb: http://git.fedorahosted.org/git/?p=gfs2-utils.git;a=commitdiff;h=be3383e4f2…
Commit: be3383e4f295c0d519c811ac9ace6bbbfa5f58e3
Parent: 34defaf57b621dbe4b0b6eb4940fd15ae524a845
Author: Andrew Price <anprice(a)redhat.com>
AuthorDate: Tue Apr 7 12:00:08 2015 +0100
Committer: Andrew Price <anprice(a)redhat.com>
CommitterDate: Tue Apr 7 19:36:30 2015 +0100
gfs2_grow: Put back the definition of FALLOC_FL_KEEP_SIZE
An #include <linux/falloc.h> was removed in commit 14193bb. Although
fallocate(2) support was added to glibc 2.10, the FALLOC_FL_*
definitions were not added until 2.18. This means that gfs2-utils failed
to build on RHEL7 which has glibc 2.17. Add back the FALLOC_FL_KEEP_SIZE
definition in main_grow.c to fix that.
Signed-off-by: Andrew Price <anprice(a)redhat.com>
---
gfs2/mkfs/main_grow.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/gfs2/mkfs/main_grow.c b/gfs2/mkfs/main_grow.c
index cc32585..4757ac7 100644
--- a/gfs2/mkfs/main_grow.c
+++ b/gfs2/mkfs/main_grow.c
@@ -38,6 +38,9 @@ int print_level = MSG_NOTICE;
extern int create_new_inode(struct gfs2_sbd *sdp);
extern int rename2system(struct gfs2_sbd *sdp, char *new_dir, char *new_name);
+#ifndef FALLOC_FL_KEEP_SIZE
+#define FALLOC_FL_KEEP_SIZE 0x01
+#endif
#ifndef BLKDISCARD
#define BLKDISCARD _IO(0x12,119)
#endif