| Message ID | 1474798162-25960-1-git-send-email-eguan@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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?
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
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.
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.
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;
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(-)