[Cluster-devel] [GFS2 PATCH] GFS2: check and correct zero i_goal

[Cluster-devel] [GFS2 PATCH] GFS2: check and correct zero i_goal

[Abhi Das]

A GFS1->GFS2 converted filesystem can have the ip->i_goal field
set to zero for inodes. This incorrect value results in -EBADSLT
when the user attempts to allocate blocks to such inodes. This
patch assigns the goal block to be the block address of the inode
itself, which serves as a reasonable starting point for the
allocation logic to find the next available block.

Resolves: rhbz#1130684
Signed-off-by: Abhi Das <adas@redhat.com>
---
 fs/gfs2/glops.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
index 2ffc67d..799427b 100644
--- a/fs/gfs2/glops.c
+++ b/fs/gfs2/glops.c
@@ -349,6 +349,9 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
 	ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
 
 	ip->i_goal = be64_to_cpu(str->di_goal_meta);
+	if (!ip->i_goal) /* From a previous gfs2_convert, perhaps */
+		ip->i_goal = ip->i_no_addr;
+
 	ip->i_generation = be64_to_cpu(str->di_generation);
 
 	ip->i_diskflags = be32_to_cpu(str->di_flags);
-- 
1.8.1.4

[Cluster-devel] [GFS2 PATCH] GFS2: check and correct zero i_goal

[Steven Whitehouse]

Hi,

On 11/09/14 12:01, Abhi Das wrote:
> A GFS1->GFS2 converted filesystem can have the ip->i_goal field
> set to zero for inodes. This incorrect value results in -EBADSLT
> when the user attempts to allocate blocks to such inodes. This
> patch assigns the goal block to be the block address of the inode
> itself, which serves as a reasonable starting point for the
> allocation logic to find the next available block.
>
> Resolves: rhbz#1130684
> Signed-off-by: Abhi Das <adas@redhat.com>
> ---
>   fs/gfs2/glops.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/fs/gfs2/glops.c b/fs/gfs2/glops.c
> index 2ffc67d..799427b 100644
> --- a/fs/gfs2/glops.c
> +++ b/fs/gfs2/glops.c
> @@ -349,6 +349,9 @@ static int gfs2_dinode_in(struct gfs2_inode *ip, const void *buf)
>   	ip->i_inode.i_ctime.tv_nsec = be32_to_cpu(str->di_ctime_nsec);
>   
>   	ip->i_goal = be64_to_cpu(str->di_goal_meta);
> +	if (!ip->i_goal) /* From a previous gfs2_convert, perhaps */
> +		ip->i_goal = ip->i_no_addr;
> +
>   	ip->i_generation = be64_to_cpu(str->di_generation);
>   
>   	ip->i_diskflags = be32_to_cpu(str->di_flags);

I don't think that is the right place to make the change, since if the 
fs is read only then the in-kernel copy will be different to the on-disk 
copy of the inode. Better to catch the problem at block allocation time 
and use i_no_addr if i_goal is invalid then (and that doesn't just mean 
0 - it might be pointing at another "impossible" block)

Steve.