* [PATCH 1/2] btrfs-progs: receive: fix parsing of attributes field from the fileattr command
2022-11-03 16:53 [PATCH 0/2] btrfs-progs: receive: address setattr failures fdmanana
@ 2022-11-03 16:53 ` fdmanana
2022-11-03 16:53 ` [PATCH 2/2] btrfs-progs: receive: work around failure of fileattr commands fdmanana
2022-11-11 15:36 ` [PATCH 0/2] btrfs-progs: receive: address setattr failures David Sterba
2 siblings, 0 replies; 4+ messages in thread
From: fdmanana @ 2022-11-03 16:53 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
We're trying to get a U32 for the attributes, but the kernel sends a U64
(which is correct as we store attributes in a u64 flags field of the
inode). This makes anyone trying to receive a v2 send stream to fail with:
ERROR: invalid size for attribute, expected = 4, got = 8
We actually recently got such a report of someone using send stream v2 and
getting such failure. See the Link tag below.
Link: https://lore.kernel.org/linux-btrfs/6cb11fa5-c60d-e65b-0295-301a694e66ad@inbox.ru/
Fixes: 7a6fb356dc65 ("btrfs-progs: receive: process setflags ioctl commands")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
common/send-stream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/send-stream.c b/common/send-stream.c
index 72a25729..e9f565e6 100644
--- a/common/send-stream.c
+++ b/common/send-stream.c
@@ -570,7 +570,7 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
break;
case BTRFS_SEND_C_FILEATTR:
TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
- TLV_GET_U32(sctx, BTRFS_SEND_A_FILEATTR, &fileattr);
+ TLV_GET_U64(sctx, BTRFS_SEND_A_FILEATTR, &fileattr);
ret = sctx->ops->fileattr(path, fileattr, sctx->user);
break;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] btrfs-progs: receive: work around failure of fileattr commands
2022-11-03 16:53 [PATCH 0/2] btrfs-progs: receive: address setattr failures fdmanana
2022-11-03 16:53 ` [PATCH 1/2] btrfs-progs: receive: fix parsing of attributes field from the fileattr command fdmanana
@ 2022-11-03 16:53 ` fdmanana
2022-11-11 15:36 ` [PATCH 0/2] btrfs-progs: receive: address setattr failures David Sterba
2 siblings, 0 replies; 4+ messages in thread
From: fdmanana @ 2022-11-03 16:53 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Currently fileattr commands, introduced in the send stream v2, always
fail, since we have commented the FS_IOC_SETFLAGS ioctl() call and set
'ret' to -EOPNOTSUPP, which is then overwritten to -errno, which may
have a random value since it was not initialized before. This results
in a failure like this:
ERROR: fileattr: set file attributes on p0/f1 failed: Invalid argument
The error reason may be something else, since errno is undefined at
this point.
Unfortunatelly we don't have a way yet to apply attributes, since the
attributes value we get from the kernel is what we store in flags field
of the inode item. This means that for example we can not just call
FS_IOC_SETFLAGS with the values we got, since they need to be converted
from BTRFS_INODE_* flags to FS_* flags
Besides that we'll have to reorder how we apply certain attributes like
FS_NOCOW_FL for example, which must happen always on an empty file and
right now we run write commands before attempting to change attributes,
as that's the order the kernel sends the operations.
So for now comment all the code, so that anyone using the v2 stream will
not have a receive failure but will get a behaviour like the v1 stream:
file attributes are ignored. This will have to be fixed later, but right
now people can't use a send stream v2 for the purpose of getting better
performance by avoid decompressing extents at the source and compression
of the data at the destination.
Link: https://lore.kernel.org/linux-btrfs/6cb11fa5-c60d-e65b-0295-301a694e66ad@inbox.ru/
Fixes: 8356c423e619 ("btrfs-progs: receive: implement FILEATTR command")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
cmds/receive.c | 44 ++++++++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 18 deletions(-)
diff --git a/cmds/receive.c b/cmds/receive.c
index af3138d5..6f475544 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1304,25 +1304,33 @@ static int process_fallocate(const char *path, int mode, u64 offset, u64 len,
static int process_fileattr(const char *path, u64 attr, void *user)
{
- int ret;
- struct btrfs_receive *rctx = user;
- char full_path[PATH_MAX];
+ /*
+ * Not yet supported, ignored for now, just like in send stream v1.
+ * The content of 'attr' matches the flags in the btrfs inode item,
+ * we can't apply them directly with FS_IOC_SETFLAGS, as we need to
+ * convert them from BTRFS_INODE_* flags to FS_* flags. Plus some
+ * flags are special and must be applied in a special way.
+ * The commented code below therefore does not work.
+ */
- ret = path_cat_out(full_path, rctx->full_subvol_path, path);
- if (ret < 0) {
- error("fileattr: path invalid: %s", path);
- return ret;
- }
- ret = open_inode_for_write(rctx, full_path);
- if (ret < 0)
- return ret;
- ret = -EOPNOTSUPP;
- /* ret = ioctl(rctx->write_fd, FS_IOC_SETFLAGS, &flags); */
- if (ret < 0) {
- ret = -errno;
- error("fileattr: set file attributes on %s failed: %m", path);
- return ret;
- }
+ /* int ret; */
+ /* struct btrfs_receive *rctx = user; */
+ /* char full_path[PATH_MAX]; */
+
+ /* ret = path_cat_out(full_path, rctx->full_subvol_path, path); */
+ /* if (ret < 0) { */
+ /* error("fileattr: path invalid: %s", path); */
+ /* return ret; */
+ /* } */
+ /* ret = open_inode_for_write(rctx, full_path); */
+ /* if (ret < 0) */
+ /* return ret; */
+ /* ret = ioctl(rctx->write_fd, FS_IOC_SETFLAGS, &attr); */
+ /* if (ret < 0) { */
+ /* ret = -errno; */
+ /* error("fileattr: set file attributes on %s failed: %m", path); */
+ /* return ret; */
+ /* } */
return 0;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 4+ messages in thread