[PATCH v8 2/2] erofs-utils: fsck: introduce exporting xattrs
Hongzhen Luo
hongzhen at linux.alibaba.com
Fri Sep 13 16:21:14 AEST 2024
On 2024/9/13 14:13, Gao Xiang wrote:
>
>
> On 2024/9/12 21:11, Hongzhen Luo wrote:
>> The current `fsck --extract` does not support exporting the extended
>> attributes of files. This patch adds `--xattrs` option to dump the
>> extended attributes, as well as the `--no-xattrs` option to not dump
>> the extended attributes.
>>
>> Signed-off-by: Hongzhen Luo <hongzhen at linux.alibaba.com>
>> ---
>> v8: The code has been adjusted to look better.
>> 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..af0c01f 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;
>> + size = ret;
>> + value = malloc(size);
>> + if (!value) {
>> + ret = -ENOMEM;
>> + break;
>> + }
>> + ret = erofs_getxattr(inode, key, value, size);
>> + if (ret < 0) {
>> + erofs_err("failed to get xattr `%s` @ nid %llu, because
>> of `%s`", key,
>> + inode->nid | 0ULL, erofs_strerror(ret));
>> + free(value);
>> + break;
>> + }
>> + if (fsckcfg.extract_path)
>> + ret = lsetxattr(fsckcfg.extract_path, key, value, size,
>> + 0);
>> + else
>> + ret = 0;
>> + free(value);
>> + if (ret == -EPERM && !fsckcfg.superuser) {
>> + if (__erofs_unlikely(!erofs_xattr_prefix_matches(key,
>> + &index, &len))) {
>> + erofs_err("failed to match the prefix of `%s` @ nid
>> %llu",
>> + key, inode->nid | 0ULL);
>> + ret = -EINVAL;
>> + break;
>> + }
>> + if (index != EROFS_XATTR_INDEX_USER) {
>> + if (!ignore_xattrs) {
>> + erofs_warn("ignored xattr `%s` @ nid %llu, due
>> to non-superuser",
>> + key, inode->nid | 0ULL);
>> + ignore_xattrs = true;
>> + }
>> + ret = 0;
>> + continue;
>> + }
>> +
>> + }
>> + if (ret) {
>> + erofs_err("failed to set xattr `%s` @ nid %llu because
>> of `%s`",
>> + key, inode->nid | 0ULL, erofs_strerror(ret));
>> + break;
>> + }
>> + }
>> +out:
>> + free(keylst);
>> + return ret;
>> +}
>> +
>> static int erofs_verify_inode_data(struct erofs_inode *inode, int
>> outfd)
>> {
>> struct erofs_map_blocks map = {
>> @@ -900,15 +990,23 @@ static int erofsfsck_check_inode(erofs_nid_t
>> pnid, erofs_nid_t nid)
>> goto out;
>> }
>> - /* verify xattr field */
>> - ret = erofs_verify_xattr(&inode);
>> - if (ret)
>> - goto out;
>> + if (!fsckcfg.check_decomp) {
>
> It should be `!(fsckcfg.check_decomp && fsckcfg.dump_xattr)`?
>
Yes, I will fix this in the next patch.
>> + /* verify xattr field */
>> + ret = erofs_verify_xattr(&inode);
>> + if (ret)
>> + goto out;
>> + }
>> ret = erofsfsck_extract_inode(&inode);
>> if (ret && ret != -ECANCELED)
>> goto out;
>> + if (fsckcfg.check_decomp && fsckcfg.dump_xattrs) {
>> + ret = erofsfsck_dump_xattrs(&inode);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> /* XXXX: the dir depth should be restricted in order to avoid
>> loops */
>> if (S_ISDIR(inode.i_mode)) {
>> struct erofs_dir_context ctx = {
>> @@ -955,6 +1053,7 @@ int main(int argc, char *argv[])
>> fsckcfg.overwrite = false;
>> fsckcfg.preserve_owner = fsckcfg.superuser;
>> fsckcfg.preserve_perms = fsckcfg.superuser;
>> + fsckcfg.dump_xattrs = false;
>> err = erofsfsck_parse_options_cfg(argc, argv);
>> if (err) {
>> diff --git a/man/fsck.erofs.1 b/man/fsck.erofs.1
>> index 393ae9e..fc862e2 100644
>> --- a/man/fsck.erofs.1
>> +++ b/man/fsck.erofs.1
>> @@ -27,6 +27,9 @@ You may give multiple
>> .B --device
>> options in the correct order.
>> .TP
>> +.BI "--[no-]xattrs"
>> +Whether to dump extended attributes (default off).
>
> Why is it placed here? (it should be located according
> to alphabetic order)
>
> Also better to be
> `Whether to dump extended attributes during extraction (default off)`
>
Yes. I will send a new version soon.
Thanks,
Hongzhen Luo
> Thanks,
> Gao Xiang
More information about the Linux-erofs
mailing list