[PATCH] generic/386: check the correct field from df output

[PATCH] generic/386: check the correct field from df output

[Theodore Ts'o]

The generic/386 test was checking the "Available" field when it should
have been checking the "1k-blocks" field, which represents the project
quota's hard limit.  On xfs, an empty directory takes no space, so it
doesn't matter.  But for ext4, an empty directory still takes 4k (or
whatever the file system's block size happens to be):

Filesystem           1K-blocks       Used  Available  Use% Pathname
/dev/vdc                512000          4     511996    0% /vdc/test

This causes generic/386 to falsely fail.  There was a confusing
comment claiming that for a very long device name, the df output would
have a line break, and for that reason, the test would extract the
field using $(NF-2).  However, looking at xfsprogs's quota command, I
see no evidence that there is any line breaking logic.  Since we now
want to use the second field, even if there was some line breaking
going on, using $2 should be a better choice.

This fix is needed to fix generic/386 from failing on ext4:

    hard limit 524283904 bytes, expected 524288000

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/generic/386 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/generic/386 b/tests/generic/386
index 0c44c80e..37e9b943 100755
--- a/tests/generic/386
+++ b/tests/generic/386
@@ -62,8 +62,6 @@ _require_scratch
 # both the "df" and the "report" output.  For "report", the line we're
 # interested in contains our project name in the first field.  For "df"
 # it contains our project directory in the last field.
-# But if the device name is too long, the "df" output is broke into two
-# lines, the fourth field is not correct, so take $(NF-2) of "df"
 _filter_quota_rpt() {
 	awk '
 	BEGIN {
@@ -89,7 +87,7 @@ _filter_quota_rpt() {
 			bsize = byte_size($4);
 		} else if ($NF ~ proj_dir) {
 			# this is the "df" output
-			bsize = byte_size($(NF-2));
+			bsize = byte_size($2));
 		} else {
 			next;
 		}
-- 
2.24.1

Re: [PATCH] generic/386: check the correct field from df output

[Theodore Y. Ts'o]

On Sun, Dec 22, 2019 at 04:27:30PM -0500, Theodore Ts'o wrote:
> ...  However, looking at xfsprogs's quota command, I
> see no evidence that there is any line breaking logic.

Eric, since you're the xfsprogs maintainer --- am I missing something?
Was there a line breaking feature in the past?

    	    	 	  	     - Ted

Re: [PATCH] generic/386: check the correct field from df output

[Eric Sandeen]

On 12/22/19 3:30 PM, Theodore Y. Ts'o wrote:
> On Sun, Dec 22, 2019 at 04:27:30PM -0500, Theodore Ts'o wrote:
>> ...  However, looking at xfsprogs's quota command, I
>> see no evidence that there is any line breaking logic.
> 
> Eric, since you're the xfsprogs maintainer --- am I missing something?
> Was there a line breaking feature in the past?
> 
>     	    	 	  	     - Ted
> 

The logic is in the free_space() function.

        if (flags & HUMAN_FLAG) {
                count = fprintf(fp, "%-12s", path->fs_name);
                if (count > 13)
                        fprintf(fp, "\n%12s", " ");
        } else {
                count = fprintf(fp, "%-19s", path->fs_name);
                if (count > 20)
                        fprintf(fp, "\n%19s", " ");
        }

eguan made this change to xfstests to handle it:

commit 50f2346560487303b381d11bbe52b93cd0a887bd
Author: Eryu Guan <eguan@redhat.com>
Date:   Tue Oct 14 22:59:38 2014 +1100

    xfs/262: update filter to deal with long device name correctly
    
    If the device name is too long, the output of xfs_quota -c "df" will be
    broke into two lines as
    
    Filesystem           1K-blocks       Used  Available  Use% Pathname
    /dev/mapper/rhel_hp--dl388eg8--01-testlv2
                          15718400      32932   15685468    0% /mnt/testarea/scratch
    /dev/mapper/rhel_hp--dl388eg8--01-testlv2
                            512000          0     512000    0% /mnt/testarea/scratch/test
    
    and _filter_quota_rpt() couldn't catch the correct available number and
    test will fail as
    
        [root@hp-dl388g8-01 xfstests]# diff -u tests/xfs/262.out /root/xfstests/results//xfs/262.out.bad
        --- tests/xfs/262.out   2014-10-08 20:16:19.000000000 +0800
        +++ /root/xfstests/results//xfs/262.out.bad  2014-10-09 14:29:38.795813323 +0800
        @@ -1,2 +1,4 @@
         QA output created by 262
         Silence is golden.
        +hard limit 0 bytes, expected 524288000
        +hard limit 0 bytes, expected 524288000
    
    Update the filter so it could catch the correct value.
    
    Signed-off-by: Eryu Guan <eguan@redhat.com>
    Reviewed-by: Dave Chinner <dchinner@redhat.com>
    Signed-off-by: Dave Chinner <david@fromorbit.com>


Does:

-			bsize = byte_size($(NF-2));
+			bsize = byte_size($(NF-4));

fix it properly on ext4?

Re: [PATCH] generic/386: check the correct field from df output

[Yang Xu]

on 2019/12/23 5:27, Theodore Ts'o wrote:
> The generic/386 test was checking the "Available" field when it should
> have been checking the "1k-blocks" field, which represents the project
> quota's hard limit.  On xfs, an empty directory takes no space, so it
> doesn't matter.  But for ext4, an empty directory still takes 4k (or
> whatever the file system's block size happens to be):
> 
> Filesystem           1K-blocks       Used  Available  Use% Pathname
> /dev/vdc                512000          4     511996    0% /vdc/test
> 
> This causes generic/386 to falsely fail.  There was a confusing
> comment claiming that for a very long device name, the df output would
> have a line break, and for that reason, the test would extract the
> field using $(NF-2).  However, looking at xfsprogs's quota command, I
> see no evidence that there is any line breaking logic.  Since we now
> want to use the second field, even if there was some line breaking
> going on, using $2 should be a better choice.
> 
> This fix is needed to fix generic/386 from failing on ext4:
Hi Theodore

I have a same fix but I use $(NF-4) field of df command. Also I wrongly 
think project quota uses a block size.
url:
https://patchwork.kernel.org/patch/11296657/

Best Regards
Yang Xu
> 
>      hard limit 524283904 bytes, expected 524288000
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>   tests/generic/386 | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/generic/386 b/tests/generic/386
> index 0c44c80e..37e9b943 100755
> --- a/tests/generic/386
> +++ b/tests/generic/386
> @@ -62,8 +62,6 @@ _require_scratch
>   # both the "df" and the "report" output.  For "report", the line we're
>   # interested in contains our project name in the first field.  For "df"
>   # it contains our project directory in the last field.
> -# But if the device name is too long, the "df" output is broke into two
> -# lines, the fourth field is not correct, so take $(NF-2) of "df"
>   _filter_quota_rpt() {
>   	awk '
>   	BEGIN {
> @@ -89,7 +87,7 @@ _filter_quota_rpt() {
>   			bsize = byte_size($4);
>   		} else if ($NF ~ proj_dir) {
>   			# this is the "df" output
> -			bsize = byte_size($(NF-2));
> +			bsize = byte_size($2));
>   		} else {
>   			next;
>   		}
> 

Re: [PATCH] generic/386: check the correct field from df output

[Eryu Guan]

On Mon, Dec 23, 2019 at 09:24:56AM +0800, Yang Xu wrote:
> 
> 
> on 2019/12/23 5:27, Theodore Ts'o wrote:
> > The generic/386 test was checking the "Available" field when it should
> > have been checking the "1k-blocks" field, which represents the project
> > quota's hard limit.  On xfs, an empty directory takes no space, so it
> > doesn't matter.  But for ext4, an empty directory still takes 4k (or
> > whatever the file system's block size happens to be):
> > 
> > Filesystem           1K-blocks       Used  Available  Use% Pathname
> > /dev/vdc                512000          4     511996    0% /vdc/test
> > 
> > This causes generic/386 to falsely fail.  There was a confusing
> > comment claiming that for a very long device name, the df output would
> > have a line break, and for that reason, the test would extract the
> > field using $(NF-2).  However, looking at xfsprogs's quota command, I
> > see no evidence that there is any line breaking logic.  Since we now
> > want to use the second field, even if there was some line breaking
> > going on, using $2 should be a better choice.
> > 
> > This fix is needed to fix generic/386 from failing on ext4:
> Hi Theodore
> 
> I have a same fix but I use $(NF-4) field of df command. Also I wrongly
> think project quota uses a block size.
> url:
> https://patchwork.kernel.org/patch/11296657/

Yes, $(NF-4) works, as Eric suggested as well. I applied Ted's patch
(with $2->$(NF-4) fix), as the commit log is explaining well what's the
problem and why taking "1k blocks" is correct. And thank you for the fix
all the same!

Thanks,
Eryu

> 
> Best Regards
> Yang Xu
> > 
> >      hard limit 524283904 bytes, expected 524288000
> > 
> > Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> > ---
> >   tests/generic/386 | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/tests/generic/386 b/tests/generic/386
> > index 0c44c80e..37e9b943 100755
> > --- a/tests/generic/386
> > +++ b/tests/generic/386
> > @@ -62,8 +62,6 @@ _require_scratch
> >   # both the "df" and the "report" output.  For "report", the line we're
> >   # interested in contains our project name in the first field.  For "df"
> >   # it contains our project directory in the last field.
> > -# But if the device name is too long, the "df" output is broke into two
> > -# lines, the fourth field is not correct, so take $(NF-2) of "df"
> >   _filter_quota_rpt() {
> >   	awk '
> >   	BEGIN {
> > @@ -89,7 +87,7 @@ _filter_quota_rpt() {
> >   			bsize = byte_size($4);
> >   		} else if ($NF ~ proj_dir) {
> >   			# this is the "df" output
> > -			bsize = byte_size($(NF-2));
> > +			bsize = byte_size($2));
> >   		} else {
> >   			next;
> >   		}
> > 
> 
>