Re: [PATCH 1/1 hurd] ext2fs: New default: use xattrs to store translator records

[Samuel Thibault]

Applied, thanks!

Damien Zammit, le dim. 03 mars 2024 01:00:53 +0000, a ecrit:
> Replaces experimental option --x-xattr-translator-records
> with --no-xattr-translator-records to allow rolling back to
> previous behaviour.
> 
> NB:
>   - Legacy records still work with either setting.
>   - Adding a new record removes a legacy one.
> ---
>  ext2fs/ext2fs.c | 18 +++++++++---------
>  ext2fs/inode.c  | 25 +++++++++++++++----------
>  2 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/ext2fs/ext2fs.c b/ext2fs/ext2fs.c
> index 3c76d8c8..3836bdf6 100644
> --- a/ext2fs/ext2fs.c
> +++ b/ext2fs/ext2fs.c
> @@ -89,15 +89,14 @@ struct ext2_group_desc *group_desc_image;
>  
>  struct pokel global_pokel;
>  
> -int use_xattr_translator_records;
>  
>  #ifdef EXT2FS_DEBUG
>  int ext2_debug_flag;
>  #endif
>  
>  /* Use extended attribute-based translator records.  */
> -int use_xattr_translator_records;
> -#define X_XATTR_TRANSLATOR_RECORDS   -1
> +int use_xattr_translator_records = 1;
> +#define NO_XATTR_TRANSLATOR_RECORDS  -1
>  
>  /* Ext2fs-specific options.  */
>  static const struct argp_option
> @@ -108,8 +107,8 @@ options[] =
>     " (not compiled in)"
>  #endif
>    },
> -  {"x-xattr-translator-records", X_XATTR_TRANSLATOR_RECORDS, 0, 0,
> -   "Store translator records in extended attributes (experimental)"},
> +  {"no-xattr-translator-records", NO_XATTR_TRANSLATOR_RECORDS, 0, 0,
> +   "Do not store translator records in extended attributes (legacy)"},
>  #ifdef ALTERNATE_SBLOCK
>    /* XXX This is not implemented.  */
>    {"sblock", 'S', "BLOCKNO", 0,
> @@ -138,8 +137,8 @@ parse_opt (int key, char *arg, struct argp_state *state)
>      case 'D':
>        values->debug_flag = 1;
>        break;
> -    case X_XATTR_TRANSLATOR_RECORDS:
> -      values->use_xattr_translator_records = 1;
> +    case NO_XATTR_TRANSLATOR_RECORDS:
> +      values->use_xattr_translator_records = 0;
>        break;
>  #ifdef ALTERNATE_SBLOCK
>      case 'S':
> @@ -159,6 +158,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
>       return ENOMEM;
>        state->hook = values;
>        memset (values, 0, sizeof *values);
> +      values->use_xattr_translator_records = use_xattr_translator_records;
>  #ifdef ALTERNATE_SBLOCK
>        values->sb_block = SBLOCK_BLOCK;
>  #endif
> @@ -194,8 +194,8 @@ diskfs_append_args (char **argz, size_t *argz_len)
>    /* Get the standard things.  */
>    err = diskfs_append_std_options (argz, argz_len);
>  
> -  if (!err && use_xattr_translator_records)
> -    err = argz_add (argz, argz_len, "--x-xattr-translator-records");
> +  if (!err && !use_xattr_translator_records)
> +    err = argz_add (argz, argz_len, "--no-xattr-translator-records");
>  
>  #ifdef EXT2FS_DEBUG
>    if (!err && ext2_debug_flag)
> diff --git a/ext2fs/inode.c b/ext2fs/inode.c
> index 5b99069e..ead82678 100644
> --- a/ext2fs/inode.c
> +++ b/ext2fs/inode.c
> @@ -764,19 +764,24 @@ diskfs_get_translator (struct node *np, char **namep, 
> mach_msg_type_number_t *na
>        return err;
>      }
>  
> -  err = ext2_get_xattr (np, "gnu.translator", NULL, &datalen);
> -  if (err)
> -    return err;
> +  /* If xattr is supported by this filesystem, check for new translator 
> record
> +   * regardless of flag to use it or not */
> +  if (EXT2_HAS_COMPAT_FEATURE (sblock, EXT2_FEATURE_COMPAT_EXT_ATTR))
> +    {
> +      err = ext2_get_xattr (np, "gnu.translator", NULL, &datalen);
> +      if (err)
> +        return err;
>  
> -  *namep = malloc (datalen);
> -  if (!*namep)
> -    err = ENOMEM;
> -  else
> -    err = ext2_get_xattr (np, "gnu.translator", *namep, &datalen);
> +      *namep = malloc (datalen);
> +      if (!*namep)
> +        err = ENOMEM;
> +      else
> +        err = ext2_get_xattr (np, "gnu.translator", *namep, &datalen);
>  
> -  diskfs_end_catch_exception ();
> +      diskfs_end_catch_exception ();
>  
> -  *namelen = datalen;
> +      *namelen = datalen;
> +    }
>    return err;
>  }
>  
> -- 
> 2.43.0
> 
> 
> 

-- 
Samuel
---
Pour une évaluation indépendante, transparente et rigoureuse !
Je soutiens la Commission d'Évaluation de l'Inria.