[Libguestfs] [libnbd PATCH 01/20] fuse: Only support defined fallocate modes

[Martin Kletzander]

On Tue, Jun 08, 2021 at 07:44:20AM -0500, Eric Blake wrote:
>On Tue, Jun 08, 2021 at 09:53:42AM +0200, Martin Kletzander wrote:
>> None of them is defined on FreeBSD, for example, and the only other way to make
>> the code compile would be to define it ourselves.
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>>  fuse/operations.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/fuse/operations.c b/fuse/operations.c
>> index de04ba7b6d72..098613f66f33 100644
>> --- a/fuse/operations.c
>> +++ b/fuse/operations.c
>> @@ -440,6 +440,9 @@ nbdfuse_fallocate (const char *path, int mode, off_t offset, off_t len,
>>    if (readonly)
>>      return -EACCES;
>>
>> +#if defined(FALLOC_FL_PUNCH_HOLE) || defined(FALLOC_FL_ZERO_RANGE)
>> +# if defined(FALLOC_FL_PUNCH_HOLE)
>
>I like this indentation of preprocessor directives...
>
>> +
>>    if (mode & FALLOC_FL_PUNCH_HOLE) {
>>      if (!nbd_can_trim (nbd.ptr[0]))
>>        return -EOPNOTSUPP;       /* Trim not supported. */
>> @@ -448,6 +451,9 @@ nbdfuse_fallocate (const char *path, int mode, off_t offset, off_t len,
>>        return 0;
>>      }
>>    }
>> +#endif /*# if defined(FALLOC_FL_PUNCH_HOLE) */
>> +
>> +#if defined(FALLOC_FL_ZERO_RANGE)
>
>...but you forgot it here
>
>>    /* As of FUSE 35 this is not supported by the kernel module and it
>>     * always returns EOPNOTSUPP.
>>     */
>> @@ -470,7 +476,9 @@ nbdfuse_fallocate (const char *path, int mode, off_t offset, off_t len,
>>        return 0;
>>      }
>>    }
>> +#endif /* defined(FALLOC_FL_ZERO_RANGE) */
>
>...as well as here.
>
>>    else
>> +#endif /* defined(FALLOC_FL_PUNCH_HOLE) || defined(FALLOC_FL_ZERO_RANGE) */
>>      return -EOPNOTSUPP;
>>  }
>
>To some extent, it would also be possible to write:
>
>#ifndef FALLOC_FL_PUNCH_HOLE
># define FALLOC_FL_PUNCH_HOLE 0
>#endif
>#ifndef FALLOC_FL_ZERO_RANGE
># define FALLOC_FL_ZERO_RANGE 0
>#endif
>
>static int
>nbdfuse_fallocate(...) { ...
>
>  if (mode & FALLOC_FL_PUNCH_HOLE) {
>    ...
>  }
>  else if (mode & FALLOC_FL_ZERO_RANGE) {
>    ...
>  }
>  else
>    return -EOPNOTSUPP;
>}
>
>for fewer in-function #ifdefs, but that is a style choice, not a
>correctness issue, so I won't insist.
>

I like your version way more, thanks!

>--
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.           +1-919-301-3266
>Virtualization:  qemu.org | libvirt.org
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20210608/a22370a5/attachment.sig>