Re: [External Mail][PATCH v4] erofs-utils: fsck: introduce exporting xattrs

[Hongzhen Luo]

On 2024/9/5 20:02, Huang Jianan wrote:

On 2024/9/5 19:37, 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 <hongz...@linux.alibaba.com>
---
v4: Optimize the error messages and code.
v3: 
https://lore.kernel.org/all/20240903113729.3539578-1-hongz...@linux.alibaba.com/
v2: 
https://lore.kernel.org/all/20240903085643.3393012-1-hongz...@linux.alibaba.com/
v1: 
https://lore.kernel.org/all/20240903073517.3311407-1-hongz...@linux.alibaba.com/
---
   fsck/main.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++---
   1 file changed, 79 insertions(+), 4 deletions(-)

diff --git a/fsck/main.c b/fsck/main.c
index 28f1e7e..183665c 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -9,6 +9,7 @@
   #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"
@@ -31,6 +32,7 @@ struct erofsfsck_cfg {
          bool overwrite;
          bool preserve_owner;
          bool preserve_perms;
+       bool dump_xattrs;
   };
   static struct erofsfsck_cfg fsckcfg;

@@ -48,6 +50,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 +102,8 @@ 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"
+               " --xattrs               dump extended attributes (default)\n"
+               " --no-xattrs            do not dump extended attributes\n"

How about:

" --[no-]xattrs  whether to dump extended attributes (default on)\n"

To keep the same style as the other switch options.

Thanks,
Jianan

Yes, that would be more concise. I'll send a new version soon.

Thanks,

Hongzhen

                  "\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,60 @@ out:
          return ret;
   }

+static int erofsfsck_dump_xattrs(struct erofs_inode *inode)
+{
+       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 < 0)
+               goto out;
+       for (key = keylst; key < keylst + kllen; key += strlen(key) + 1) {
+               void *value = NULL;
+               size_t size = 0;
+
+               ret = erofs_getxattr(inode, key, NULL, 0);
+               if (ret < 0)
+                       break;
+               if (ret) {
+                       size = ret;
+                       value = malloc(size);
+                       if (!value) {
+                               ret = -ENOMEM;
+                               break;
+                       }
+                       ret = erofs_getxattr(inode, key, value, size);
+                       if (ret < 0) {
+                               erofs_err("fail to get xattr %s @ nid %llu",
+                                         key, inode->nid | 0ULL);
+                               free(value);
+                               break;
+                       }
+                       if (fsckcfg.extract_path)
+                               ret = setxattr(fsckcfg.extract_path, key, value,
+                                              size, 0);
+                       else
+                               ret = 0;
+                       free(value);
+                       if (ret) {
+                               erofs_err("fail to set xattr %s @ nid %llu",
+                                         key, inode->nid | 0ULL);
+                               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 +966,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) {
+               /* 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 +1029,7 @@ int main(int argc, char *argv[])
          fsckcfg.overwrite = false;
          fsckcfg.preserve_owner = fsckcfg.superuser;
          fsckcfg.preserve_perms = fsckcfg.superuser;
+       fsckcfg.dump_xattrs = true;

          err = erofsfsck_parse_options_cfg(argc, argv);
          if (err) {
--
2.43.5