[PATCH] libxcmd: fix counting of xfs entries in fs_table_insert

[PATCH] libxcmd: fix counting of xfs entries in fs_table_insert

[Eryu Guan]

Commit bb80e3d6cd04 ("libxcmd: populate fs table with xfs entries
first, foreign entries last") adds a new counter "xfs_fs_count" and
increases the counter when inserting an XFS entry.

But it missed a counter when fs_count is zero (inserting the first
path) and the entry has no FS_FOREIGN bit set, i.e. the first XFS
entry doesn't increase xfs_fs_count.

This results in args_command() mess and infinite loop in xfs/244
when testing v4 XFS (xfs/244 notrun on v5 XFS, but this bug still
reproduces on v5 XFS). e.g.

  mkfs -t xfs -f /dev/sda5
  mount -o pquota /dev/sda5 /mnt/xfs
  mkdir /mnt/xfs/project
  touch /mnt/xfs/project/testfile
  xfs_quota -x -c "project -s -p /mnt/xfs/project/testfile 1" /dev/sda5

Fix it by increasing xfs_fs_count when flags has no FS_FOREIGN bit.

Signed-off-by: Eryu Guan <eguan@redhat.com>
---
 libxcmd/paths.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libxcmd/paths.c b/libxcmd/paths.c
index 3455217..97f47cf 100644
--- a/libxcmd/paths.c
+++ b/libxcmd/paths.c
@@ -147,7 +147,6 @@ fs_table_insert(
 		memmove(&fs_table[xfs_fs_count + 1], &fs_table[xfs_fs_count],
 			sizeof(fs_path_t)*(fs_count - xfs_fs_count));
 		fs_path = &fs_table[xfs_fs_count];
-		xfs_fs_count++;
 	}
 	fs_path->fs_dir = dir;
 	fs_path->fs_prid = prid;
@@ -159,6 +158,8 @@ fs_table_insert(
 	fs_path->fs_logdev = logdev;
 	fs_path->fs_rtdev = rtdev;
 	fs_count++;
+	if (!(flags & FS_FOREIGN))
+		xfs_fs_count++;
 
 	return 0;
 
-- 
2.7.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] libxcmd: fix counting of xfs entries in fs_table_insert

[Christoph Hellwig]

On Sun, Sep 25, 2016 at 06:09:22PM +0800, Eryu Guan wrote:
> Commit bb80e3d6cd04 ("libxcmd: populate fs table with xfs entries
> first, foreign entries last") adds a new counter "xfs_fs_count" and
> increases the counter when inserting an XFS entry.
> 
> But it missed a counter when fs_count is zero (inserting the first
> path) and the entry has no FS_FOREIGN bit set, i.e. the first XFS
> entry doesn't increase xfs_fs_count.
> 
> This results in args_command() mess and infinite loop in xfs/244
> when testing v4 XFS (xfs/244 notrun on v5 XFS, but this bug still
> reproduces on v5 XFS). e.g.
> 
>   mkfs -t xfs -f /dev/sda5
>   mount -o pquota /dev/sda5 /mnt/xfs
>   mkdir /mnt/xfs/project
>   touch /mnt/xfs/project/testfile
>   xfs_quota -x -c "project -s -p /mnt/xfs/project/testfile 1" /dev/sda5
> 
> Fix it by increasing xfs_fs_count when flags has no FS_FOREIGN bit.

Looks ifne,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Can I assume you'll add the above test to xfstests?

Re: [PATCH] libxcmd: fix counting of xfs entries in fs_table_insert

[Eryu Guan]

On Sun, Sep 25, 2016 at 07:31:55AM -0700, Christoph Hellwig wrote:
> On Sun, Sep 25, 2016 at 06:09:22PM +0800, Eryu Guan wrote:
> > Commit bb80e3d6cd04 ("libxcmd: populate fs table with xfs entries
> > first, foreign entries last") adds a new counter "xfs_fs_count" and
> > increases the counter when inserting an XFS entry.
> > 
> > But it missed a counter when fs_count is zero (inserting the first
> > path) and the entry has no FS_FOREIGN bit set, i.e. the first XFS
> > entry doesn't increase xfs_fs_count.
> > 
> > This results in args_command() mess and infinite loop in xfs/244
> > when testing v4 XFS (xfs/244 notrun on v5 XFS, but this bug still
> > reproduces on v5 XFS). e.g.
> > 
> >   mkfs -t xfs -f /dev/sda5
> >   mount -o pquota /dev/sda5 /mnt/xfs
> >   mkdir /mnt/xfs/project
> >   touch /mnt/xfs/project/testfile
> >   xfs_quota -x -c "project -s -p /mnt/xfs/project/testfile 1" /dev/sda5
> > 
> > Fix it by increasing xfs_fs_count when flags has no FS_FOREIGN bit.
> 
> Looks ifne,
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Can I assume you'll add the above test to xfstests?

It's already part of xfs/244, I noticed this bug because xfs/244 kept
running. I just put the minimum steps in commit log. So I think we're
good :)

Thanks,
Eryu

Re: [PATCH] libxcmd: fix counting of xfs entries in fs_table_insert

[Christoph Hellwig]

On Sun, Sep 25, 2016 at 10:58:16PM +0800, Eryu Guan wrote:
> It's already part of xfs/244, I noticed this bug because xfs/244 kept
> running. I just put the minimum steps in commit log. So I think we're
> good :)

But only as part of xfs/244 which doesn't work for v5 file systems.
To have good coverage we should not rely on testing an old format.
That beeing said I can't see a good reason for why xfs/244 should not
be run for v5 file systems, so I'll look into that instead.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] libxcmd: fix counting of xfs entries in fs_table_insert

[Dave Chinner]

On Sun, Sep 25, 2016 at 09:12:56AM -0700, Christoph Hellwig wrote:
> On Sun, Sep 25, 2016 at 10:58:16PM +0800, Eryu Guan wrote:
> > It's already part of xfs/244, I noticed this bug because xfs/244 kept
> > running. I just put the minimum steps in commit log. So I think we're
> > good :)
> 
> But only as part of xfs/244 which doesn't work for v5 file systems.
> To have good coverage we should not rely on testing an old format.
> That beeing said I can't see a good reason for why xfs/244 should not
> be run for v5 file systems, so I'll look into that instead.

It's because it's testing the projid32bit mkfs option works
correctly. i.e. that project IDs > 16 bits fail on a a
filesystem that only supports 16 bit project IDs. v5 filesystems
only support 32 bit project IDs, so setting a > 16bit ID will
succeed, not fail like the test is expecting.

A new test would be simplest.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs