[PATCH v9 2/2] erofs-utils: fsck: introduce exporting xattrs

[Gao Xiang]

On 2024/9/13 14:49, Hongzhen Luo wrote:
> Currently `fsck --extract` does not support exporting
> extended attributes. This patch adds the `--xattrs` option
> to dump extended attributes and the `--no-xattrs` option to
> omit them (the default behavior).
> 
> Signed-off-by: Hongzhen Luo <hongzhen at linux.alibaba.com>
> ---
> v9: Adjust the contents of fsck.erofs.1.
> v8: https://lore.kernel.org/all/20240912131108.3742683-2-hongzhen@linux.alibaba.com/
> v7: https://lore.kernel.org/all/20240906095853.3167228-2-hongzhen@linux.alibaba.com/
> v6: https://lore.kernel.org/all/20240906083849.3090392-2-hongzhen@linux.alibaba.com/
> v5: https://lore.kernel.org/all/20240906022206.2725584-1-hongzhen@linux.alibaba.com/
> v4: https://lore.kernel.org/all/20240905113723.1937634-1-hongzhen@linux.alibaba.com/
> v3: https://lore.kernel.org/all/20240903113729.3539578-1-hongzhen@linux.alibaba.com/
> v2: https://lore.kernel.org/all/20240903085643.3393012-1-hongzhen@linux.alibaba.com/
> v1: https://lore.kernel.org/all/20240903073517.3311407-1-hongzhen@linux.alibaba.com/
> ---
>   fsck/main.c      | 107 +++++++++++++++++++++++++++++++++++++++++++++--
>   man/fsck.erofs.1 |   3 ++
>   2 files changed, 106 insertions(+), 4 deletions(-)
> 
> diff --git a/fsck/main.c b/fsck/main.c
> index 28f1e7e..83f29c5 100644
> --- a/fsck/main.c
> +++ b/fsck/main.c
> @@ -9,10 +9,12 @@
>   #include <utime.h>
>   #include <unistd.h>
>   #include <sys/stat.h>
> +#include <sys/xattr.h>
>   #include "erofs/print.h"
>   #include "erofs/compress.h"
>   #include "erofs/decompress.h"
>   #include "erofs/dir.h"
> +#include "erofs/xattr.h"
>   #include "../lib/compressor.h"
>   
>   static int erofsfsck_check_inode(erofs_nid_t pnid, erofs_nid_t nid);
> @@ -31,6 +33,7 @@ struct erofsfsck_cfg {
>   	bool overwrite;
>   	bool preserve_owner;
>   	bool preserve_perms;
> +	bool dump_xattrs;
>   };
>   static struct erofsfsck_cfg fsckcfg;
>   
> @@ -48,6 +51,8 @@ static struct option long_options[] = {
>   	{"no-preserve-owner", no_argument, 0, 10},
>   	{"no-preserve-perms", no_argument, 0, 11},
>   	{"offset", required_argument, 0, 12},
> +	{"xattrs", no_argument, 0, 13},
> +	{"no-xattrs", no_argument, 0, 14},
>   	{0, 0, 0, 0},
>   };
>   
> @@ -98,6 +103,7 @@ static void usage(int argc, char **argv)
>   		" --extract[=X]          check if all files are well encoded, optionally\n"
>   		"                        extract to X\n"
>   		" --offset=#             skip # bytes at the beginning of IMAGE\n"
> +		" --[no-]xattrs          whether to dump extended attributes (default off)\n"
>   		"\n"
>   		" -a, -A, -y             no-op, for compatibility with fsck of other filesystems\n"
>   		"\n"
> @@ -225,6 +231,12 @@ static int erofsfsck_parse_options_cfg(int argc, char **argv)
>   				return -EINVAL;
>   			}
>   			break;
> +		case 13:
> +			fsckcfg.dump_xattrs = true;
> +			break;
> +		case 14:
> +			fsckcfg.dump_xattrs = false;
> +			break;
>   		default:
>   			return -EINVAL;
>   		}
> @@ -411,6 +423,84 @@ out:
>   	return ret;
>   }
>   
> +static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
> +{
> +	static bool ignore_xattrs = false;
> +	char *keylst, *key;
> +	ssize_t kllen;
> +	int ret;
> +
> +	kllen = erofs_listxattr(inode, NULL, 0);
> +	if (kllen <= 0)
> +		return kllen;
> +	keylst = malloc(kllen);
> +	if (!keylst)
> +		return -ENOMEM;
> +	ret = erofs_listxattr(inode, keylst, kllen);
> +	if (ret != kllen) {
> +		erofs_err("failed to list xattrs @ nid %llu",
> +			  inode->nid | 0ULL);
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +	ret = 0;
> +	for (key = keylst; key < keylst + kllen; key += strlen(key) + 1) {
> +		unsigned int index, len;
> +		void *value = NULL;
> +		size_t size = 0;
> +
> +		ret = erofs_getxattr(inode, key, NULL, 0);
> +		if (ret <= 0)
> +			break;

When does erofs_getxattr() return <= 0?

Should we add some print messages too? like:
		erofs_err("failed to get xattr value size of `%s` @ nid %llu",
			  key, inode->nid | 0ULL);

Otherwise this patch looks good to me.

Thanks,
Gao Xiang