Re: [RFC PATCH v3 6/8] selftests/fuse: add some extra ACL caching tests

[Luis Henriques]

On Sat, Sep 05 2026, Amir Goldstein wrote:

> On Fri, Sep 4, 2026 at 12:38 PM Luis Henriques <luis@xxxxxxxxxx> wrote:
>>
>> This adds some extra tests to ACL caching:
>> - Verify that reading ACLs results in the expected number of requests
>>   being sent user-space, depending on whether cache is enabled or disabled
>> - Verify caching behaviour on some caching invalidation scenarios
>>
>> While there, add test binary to .gitignore.
>>
>> Signed-off-by: Luis Henriques <luis@xxxxxxxxxx>
>> ---
>>  .../selftests/filesystems/fuse/.gitignore     |   1 +
>>  .../filesystems/fuse/fuse_acl_cache_test.c    | 179 ++++++++++++++++++
>>  2 files changed, 180 insertions(+)
>>
>> diff --git a/tools/testing/selftests/filesystems/fuse/.gitignore b/tools/testing/selftests/filesystems/fuse/.gitignore
>> index fb51603fe419..f7f3dd345a50 100644
>> --- a/tools/testing/selftests/filesystems/fuse/.gitignore
>> +++ b/tools/testing/selftests/filesystems/fuse/.gitignore
>> @@ -2,3 +2,4 @@
>>  fuse_mnt
>>  fusectl_test
>>  write_extend_eof_test
>> +fuse_acl_cache_test
>> diff --git a/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c b/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c
>> index c2d6658ff7de..9608a0adb967 100644
>> --- a/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c
>> +++ b/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c
>> @@ -83,6 +83,7 @@ struct daemon_state {
>>         uint8_t        *acl;
>>         size_t          acl_size;
>>         int             getxattr_count;
>> +       bool            cache;
>>  };
>>
>>  /*
>> @@ -91,9 +92,17 @@ struct daemon_state {
>>   */
>>  static struct daemon_state g_ds = {
>>         .lock = PTHREAD_MUTEX_INITIALIZER,
>> +       .cache = false,
>>  };
>>
>>  /* ---- FUSE lowlevel callbacks -------------------------------------------- */
>> +static void fs_init(void *userdata, struct fuse_conn_info *conn)
>> +{
>> +       pthread_mutex_lock(&g_ds.lock);
>> +       if (g_ds.cache)
>> +               fuse_set_feature_flag(conn, FUSE_CAP_POSIX_ACL);
>> +       pthread_mutex_unlock(&g_ds.lock);
>> +}
>>
>>  static void fs_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
>>  {
>> @@ -115,6 +124,8 @@ static void fs_lookup(fuse_req_t req, fuse_ino_t parent, const char *name)
>>         e.attr.st_ino     = FILE_INO;
>>         e.attr.st_mode    = S_IFREG | 0644;
>>         e.attr.st_nlink   = 1;
>> +       e.attr.st_uid = getuid();
>> +       e.attr.st_gid = getgid();
>>         fuse_reply_entry(req, &e);
>>  }
>>
>> @@ -175,10 +186,38 @@ static void fs_getxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
>>         free(acl);
>>  }
>>
>> +static void fs_setxattr(fuse_req_t req, fuse_ino_t ino, const char *name,
>> +                           const char *value, size_t size, int flags)
>> +{
>> +       int ret = 0;
>> +       uint8_t *acl;
>> +
>> +       if (ino != FILE_INO)
>> +               ret = ENOENT;
>> +       else if (!strcmp(name, "system.posix_acl_access")) {
>> +               acl = malloc(size);
>> +               if (acl) {
>> +                       memcpy(acl, value, size);
>> +                       pthread_mutex_lock(&g_ds.lock);
>> +                       if (g_ds.acl)
>> +                               free(g_ds.acl);
>> +                       g_ds.acl = acl;
>> +                       g_ds.acl_size = size;
>> +                       pthread_mutex_unlock(&g_ds.lock);
>> +               } else
>> +                       ret = ENOMEM;
>> +       } else
>> +               ret = ENOTSUP;
>> +
>
> I am allergic to mismatching {} in if/else statements.

Ah! Ah! OK, I'll fix that.  (And I also admit I'm not consistent with
this, which probably means I'm not allergic :-) )

> I personally think that code will be cleaner with a goto error
> without all these multi nesting levels.

Sure, I'll refactor it accordingly.

Cheers,
-- 
Luís