xfs
[Top] [All Lists]

Re: [PATCH 7/8] xfstests: Add fallocate zero range operation to fsx

To: Lukas Czerner <lczerner@xxxxxxxxxx>
Subject: Re: [PATCH 7/8] xfstests: Add fallocate zero range operation to fsx
From: Andreas Dilger <adilger@xxxxxxxxx>
Date: Fri, 28 Feb 2014 12:08:47 -0700
Cc: linux-ext4@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1393603865-26198-7-git-send-email-lczerner@xxxxxxxxxx>
References: <1393603865-26198-1-git-send-email-lczerner@xxxxxxxxxx> <1393603865-26198-7-git-send-email-lczerner@xxxxxxxxxx>
On Feb 28, 2014, at 9:11 AM, Lukas Czerner <lczerner@xxxxxxxxxx> wrote:
> This commit adds fallocate FALLOC_FL_ZERO_RANGE support for fsx.

It looks like this patch breaks the option handling for these
fallocate features.

>  int     fallocate_calls = 1;            /* -F flag disables */
>  int     punch_hole_calls = 1;           /* -H flag disables */
> +int     zero_range_calls = 1;           /* -z flag disables */

So these fallocate tests are on by default...

> +int
> +test_fallocate(int mode)
>  {
>  #ifdef HAVE_LINUX_FALLOC_H
> +     int ret = 0;
>       if (!lite && fallocate_calls) {
> +             if (fallocate(fd, mode, 0, 1) && errno == EOPNOTSUPP) {
>                       if(!quiet)
>                               warn("main: filesystem does not support 
> fallocate, disabling\n");
>               } else {
> +                     ret = 1;
>                       ftruncate(fd, 0);
>               }
>       }
> #endif
> }

And this returns 1 or 0 depending if they are supported or not...

> +     while ((ch = getopt(argc, argv, 
> "b:c:dfl:m:no:p:qr:s:t:w:xyAD:FHzLN:OP:RS:WZ"))
>              != EOF)
>               switch (ch) {
>               case 'H':
>                       punch_hole_calls = 0;
>                       break;
> +             case 'z':
> +                     zero_range_calls = 0;
> +                     break;

And the option parsing sets the values to zero if they are disabled,
so far, so good...

> +     fallocate_calls = test_fallocate(0);
> +     punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE | 
> FALLOC_FL_KEEP_SIZE);
> +     zero_range_calls = test_fallocate(FALLOC_FL_ZERO_RANGE);

But here, the values set by option parsing are clobbered and the
tests are only enabled or disabled depending on whether the kernel
supports that feature or not.  I think you need something like:

        if (fallocate_calls)
                fallocate_calls = test_fallocate(0);
        if (punch_hole_calls)
                punch_hole_calls = test_fallocate(FALLOC_FL_PUNCH_HOLE |
                                                  FALLOC_FL_KEEP_SIZE);
        if (zero_range_calls)
                zero_range_calls = test_fallocate(FALLOC_FL_ZERO_RANGE);

Cheers, Andreas





Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

<Prev in Thread] Current Thread [Next in Thread>