[Cluster-devel] Re: [PATCH] Copy i_flags to gfs2 inode flags on write

[Steven Whitehouse]

Hi,

On Mon, 2007-04-23 at 20:41 +0200, Jan Kara wrote:
>   Hi,
> 
>   attached is a patch to copy i_flags to GFS2 specific i_di_di_flags. The
> patch is compile-tested only. Steven, does it look OK to you?
> 
> 								Honza
> 
I can't see why we need this at the moment... what other interface is
there aside from the ioctl() for setting these flags?

Also:

        diff -rupX /home/jack/.kerndiffexclude linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.c linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.c
        ---
        linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.c    2007-04-10 17:09:55.000000000 +0200
        +++
        linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.c     2007-04-23 20:10:55.000000000 +0200
        @@ -207,6 +207,7 @@ static int gfs2_get_flags(struct file *f
                if (error)
                        return error;
         
        +       gfs2_get_inode_flags(ip);
                fsflags = fsflags_cvt(gfs2_to_fsflags,
        ip->i_di.di_flags);

Why do you call gfs2_get_inode_flags() here? There is no reason that the
gfs2 flags will not match the vfs inode flags at this point that I can
see (its a bug if they don't match). It is not valid to change the flags
while only holding a read lock and when outside of a transaction anyway.

        diff -rupX /home/jack/.kerndiffexclude linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.h linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.h
        ---
        linux-2.6.21-rc6-4-ocfs2_flags_update/fs/gfs2/ops_file.h    2007-02-07 12:03:23.000000000 +0100
        +++
        linux-2.6.21-rc6-5-gfs2_flags_update/fs/gfs2/ops_file.h     2007-04-23 20:11:43.000000000 +0200
        @@ -18,6 +18,7 @@ extern int gfs2_internal_read(struct gfs
                                      struct file_ra_state *ra_state,
                                      char *buf, loff_t *pos, unsigned
        size);
         extern void gfs2_set_inode_flags(struct inode *inode);
        +extern void gfs2_get_inode_flags(struct gfs2_inode *inode);

Please use *ip for struct gfs2_inode and *inode for struct inode in
common with the rest of the code as it helps avoid confusion,

Steve.