On Fri, Aug 21, 2015 at 07:55:22PM +0200, Jan Kara wrote: > Users have occasionally reported that file type for some directory > entries is wrong. This mostly happened after updating libraries some > libraries. After some debugging the problem was traced down to > xfs_dir2_node_replace(). The function uses args->filetype as a file type > to store in the replaced directory entry however it also calls > xfs_da3_node_lookup_int() which will store file type of the current > directory entry in args->filetype. Thus we fail to change file type of a > directory entry to a proper type. > > Fix the problem by storing new file type in a local variable before > calling xfs_da3_node_lookup_int(). > > Reported-by: Giacomo Comes <comes@xxxxxxxx> > Signed-off-by: Jan Kara <jack@xxxxxxxx> So this is being triggered by a rename() operation on a large directory? node format is the optimised form form for large dirs, so I suspect that's why only few people see this. Can you see if you can write a reproducer for it baseed on a large single directory and renaming two files of different types (e.g. BLKDEV over REG) to see if the cause is that simple? i.e. I'm looking for something that can be put into a regression test, because vlealry we don't cover this case right now.... > --- > fs/xfs/libxfs/xfs_dir2_node.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c > index 41b80d3d3877..1006710a7c92 100644 > --- a/fs/xfs/libxfs/xfs_dir2_node.c > +++ b/fs/xfs/libxfs/xfs_dir2_node.c > @@ -2132,6 +2132,7 @@ xfs_dir2_node_replace( > int error; /* error return value */ > int i; /* btree level */ > xfs_ino_t inum; /* new inode number */ > + int ftype; /* new file type */ > xfs_dir2_leaf_t *leaf; /* leaf structure */ > xfs_dir2_leaf_entry_t *lep; /* leaf entry being changed */ > int rval; /* internal return value */ > @@ -2145,7 +2146,12 @@ xfs_dir2_node_replace( > state = xfs_da_state_alloc(); > state->args = args; > state->mp = args->dp->i_mount; > + /* > + * We have to save new inode number and ftype since > + * xfs_da3_node_lookup_int() is going to overwrite them > + */ > inum = args->inumber; > + ftype = args->filetype; > /* > * Lookup the entry to change in the btree. > */ > @@ -2183,7 +2189,7 @@ xfs_dir2_node_replace( > * Fill in the new inode number and log the entry. > */ > dep->inumber = cpu_to_be64(inum); > - args->dp->d_ops->data_put_ftype(dep, args->filetype); > + args->dp->d_ops->data_put_ftype(dep, ftype); > xfs_dir2_data_log_entry(args, state->extrablk.bp, dep); > rval = 0; > } The change looks sane (i'll add whitespace around the comments when i commit) and I'll also add the stable cc. I would like to have a reproducer to test it, though ;) Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
