[PATCH] btrfs-progs: restore of xattrs fixed

[PATCH] btrfs-progs: restore of xattrs fixed

[Holger Jakob]

    Fixed issue with metadata getting modified in dry run mode
    Fixed issue with restore of xattrs not working on directories


diff --git a/cmds/restore.c b/cmds/restore.c
index 19df6be2..c643f956 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -621,6 +621,26 @@ out:
 	return ret;
 }
 
+static int copy_xattrs(struct btrfs_root *root, int fd,
+		struct btrfs_key *key, const char *path_name)
+{
+	struct btrfs_path path;
+	int ret;
+
+	btrfs_init_path(&path);
+	ret = btrfs_lookup_inode(NULL, root, &path, key, 0);
+	if (ret == 0) {
+		ret = set_file_xattrs(root, key->objectid, fd, path_name);
+		if (ret) {
+			error("failed to set xattrs: %m");
+			goto out;
+		}
+	}
+out:
+	btrfs_release_path(&path);
+	return ret;
+}
+
 static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key,
 		     const char *file)
 {
@@ -1117,11 +1137,11 @@ next:
 		path.slots[0]++;
 	}
 
-	if (restore_metadata) {
+	if ((restore_metadata || get_xattrs) && !dry_run) {
 		snprintf(path_name, PATH_MAX, "%s%s", output_rootdir, in_dir);
 		fd = open(path_name, O_RDONLY);
 		if (fd < 0) {
-			error("failed to access '%s' to restore metadata: %m",
+			error("failed to access '%s' to restore metadata/xattrs: %m",
 					path_name);
 			if (!ignore_errors) {
 				ret = -1;
@@ -1132,7 +1152,20 @@ next:
 			 * Set owner/mode/time on the directory as well
 			 */
 			key->type = BTRFS_INODE_ITEM_KEY;
-			ret = copy_metadata(root, fd, key);
+			if (restore_metadata) {
+				ret = copy_metadata(root, fd, key);
+			}
+			if (ret && !ignore_errors) {
+				close(fd);
+				goto out;
+			}
+
+			/*
+			 * Set xattrs on the directory
+			 */
+			if (get_xattrs) {
+				ret = copy_xattrs(root, fd, key, path_name);
+			}
 			close(fd);
 			if (ret && !ignore_errors)
 				goto out;

Re: [PATCH] btrfs-progs: restore of xattrs fixed

[Anand Jain]

On 2/2/23 01:06, Holger Jakob wrote:
>      Fixed issue with metadata getting modified in dry run mode
>      Fixed issue with restore of xattrs not working on directories
> 
> 


Could you please split to one fix in a patch? And it would be a great 
idea to document the failing command in the change log too.

SOB tag is missing.

Thanks, Anand

> diff --git a/cmds/restore.c b/cmds/restore.c
> index 19df6be2..c643f956 100644
> --- a/cmds/restore.c
> +++ b/cmds/restore.c
> @@ -621,6 +621,26 @@ out:
>   	return ret;
>   }
>   
> +static int copy_xattrs(struct btrfs_root *root, int fd,
> +		struct btrfs_key *key, const char *path_name)
> +{
> +	struct btrfs_path path;
> +	int ret;
> +
> +	btrfs_init_path(&path);
> +	ret = btrfs_lookup_inode(NULL, root, &path, key, 0);
> +	if (ret == 0) {
> +		ret = set_file_xattrs(root, key->objectid, fd, path_name);
> +		if (ret) {
> +			error("failed to set xattrs: %m");
> +			goto out;
> +		}
> +	}
> +out:
> +	btrfs_release_path(&path);
> +	return ret;
> +}
> +
>   static int copy_file(struct btrfs_root *root, int fd, struct btrfs_key *key,
>   		     const char *file)
>   {
> @@ -1117,11 +1137,11 @@ next:
>   		path.slots[0]++;
>   	}
>   
> -	if (restore_metadata) {
> +	if ((restore_metadata || get_xattrs) && !dry_run) {
>   		snprintf(path_name, PATH_MAX, "%s%s", output_rootdir, in_dir);
>   		fd = open(path_name, O_RDONLY);
>   		if (fd < 0) {
> -			error("failed to access '%s' to restore metadata: %m",
> +			error("failed to access '%s' to restore metadata/xattrs: %m",
>   					path_name);
>   			if (!ignore_errors) {
>   				ret = -1;
> @@ -1132,7 +1152,20 @@ next:
>   			 * Set owner/mode/time on the directory as well
>   			 */
>   			key->type = BTRFS_INODE_ITEM_KEY;
> -			ret = copy_metadata(root, fd, key);
> +			if (restore_metadata) {
> +				ret = copy_metadata(root, fd, key);
> +			}
> +			if (ret && !ignore_errors) {
> +				close(fd);
> +				goto out;
> +			}
> +
> +			/*
> +			 * Set xattrs on the directory
> +			 */
> +			if (get_xattrs) {
> +				ret = copy_xattrs(root, fd, key, path_name);
> +			}
>   			close(fd);
>   			if (ret && !ignore_errors)
>   				goto out;

[PATCH v2 1/2] Fixed issue with restore of xattrs not working on directories

[Holger Jakob]

Restore was only setting xattrs on files but ignored directories.
The patch adds a missing set_file_xattrs

Signed-off-by: Holger Jakob <jakob@dsi.uni-stuttgart.de>
---
 cmds/restore.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/cmds/restore.c b/cmds/restore.c
index 19df6be2..18edc8ca 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -1117,11 +1117,11 @@ next:
 		path.slots[0]++;
 	}
 
-	if (restore_metadata) {
+	if (restore_metadata || get_xattrs) {
 		snprintf(path_name, PATH_MAX, "%s%s", output_rootdir, in_dir);
 		fd = open(path_name, O_RDONLY);
 		if (fd < 0) {
-			error("failed to access '%s' to restore metadata: %m",
+			error("failed to access '%s' to restore metadata/xattrs: %m",
 					path_name);
 			if (!ignore_errors) {
 				ret = -1;
@@ -1132,7 +1132,23 @@ next:
 			 * Set owner/mode/time on the directory as well
 			 */
 			key->type = BTRFS_INODE_ITEM_KEY;
-			ret = copy_metadata(root, fd, key);
+			if (restore_metadata) {
+				ret = copy_metadata(root, fd, key);
+				if (ret && !ignore_errors) {
+					close(fd);
+					goto out;
+				}
+			}
+
+			/*
+			 * Also set xattrs on the directory
+			 */
+			if (get_xattrs) {
+				ret = set_file_xattrs(root, key->objectid, fd, path_name);
+				if (ret) {
+					error("failed to set xattrs: %m");
+				}
+			}
 			close(fd);
 			if (ret && !ignore_errors)
 				goto out;
-- 
2.35.3

[PATCH v2 2/2] Fixed issue with metadata getting modified in dry run mode

[Holger Jakob]

In Dry run the following error appeared and aborted execution

ERROR: failed to access 'XYZ' to restore metadata/xattrs: No such file or directory

The patch is skipping the metadata and xattrs handling when dry run is enabled

Signed-off-by: Holger Jakob <jakob@dsi.uni-stuttgart.de>
---
 cmds/restore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmds/restore.c b/cmds/restore.c
index 18edc8ca..0b1f00a5 100644
--- a/cmds/restore.c
+++ b/cmds/restore.c
@@ -1117,7 +1117,7 @@ next:
 		path.slots[0]++;
 	}
 
-	if (restore_metadata || get_xattrs) {
+	if ((restore_metadata || get_xattrs) && !dry_run) {
 		snprintf(path_name, PATH_MAX, "%s%s", output_rootdir, in_dir);
 		fd = open(path_name, O_RDONLY);
 		if (fd < 0) {
-- 
2.35.3

Re: [PATCH v2 1/2] Fixed issue with restore of xattrs not working on directories

[David Sterba]

On Wed, Feb 01, 2023 at 08:33:44PM -0800, Holger Jakob wrote:
> Restore was only setting xattrs on files but ignored directories.
> The patch adds a missing set_file_xattrs
> 
> Signed-off-by: Holger Jakob <jakob@dsi.uni-stuttgart.de>

Added to devel with some minor updates, thanks.