[PATCH] btrfs-progs: receive: retry encoded_write on EPERM

[PATCH] btrfs-progs: receive: retry encoded_write on EPERM

[Boris Burkov]

encoded_write fails if we run without CAP_SYS_ADMIN, but the decompress
and write fallback could succeed if we are running with write
permissions on the file. Therefore, it is helpful to fall back on EPERM
as well. While this will increase the "silent failure" rate of encoded
writes, we do have the verbose log in place to debug that while setting
up a receive workflow that expects encoded_write.

Signed-off-by: Boris Burkov <boris@bur.io>
---
 cmds/receive.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmds/receive.c b/cmds/receive.c
index 412bc8afe..9101e8ccf 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -1285,7 +1285,8 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
 		if (ret >= 0)
 			return 0;
 		/* Fall back for these errors, fail hard for anything else. */
-		if (errno != ENOSPC && errno != ENOTTY && errno != EINVAL) {
+		if (errno != ENOSPC && errno != ENOTTY &&
+		    errno != EINVAL && errno != EPERM) {
 			ret = -errno;
 			error("encoded_write: writing to %s failed: %m", path);
 			return ret;
-- 
2.45.2

Re: [PATCH] btrfs-progs: receive: retry encoded_write on EPERM

[Josef Bacik]

On Tue, Jul 02, 2024 at 09:27:49AM -0700, Boris Burkov wrote:
> encoded_write fails if we run without CAP_SYS_ADMIN, but the decompress
> and write fallback could succeed if we are running with write
> permissions on the file. Therefore, it is helpful to fall back on EPERM
> as well. While this will increase the "silent failure" rate of encoded
> writes, we do have the verbose log in place to debug that while setting
> up a receive workflow that expects encoded_write.
> 
> Signed-off-by: Boris Burkov <boris@bur.io>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH] btrfs-progs: receive: retry encoded_write on EPERM

[Qu Wenruo]

在 2024/7/3 01:57, Boris Burkov 写道:
> encoded_write fails if we run without CAP_SYS_ADMIN, but the decompress
> and write fallback could succeed if we are running with write
> permissions on the file. Therefore, it is helpful to fall back on EPERM
> as well. While this will increase the "silent failure" rate of encoded
> writes, we do have the verbose log in place to debug that while setting
> up a receive workflow that expects encoded_write.
>
> Signed-off-by: Boris Burkov <boris@bur.io>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   cmds/receive.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/cmds/receive.c b/cmds/receive.c
> index 412bc8afe..9101e8ccf 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -1285,7 +1285,8 @@ static int process_encoded_write(const char *path, const void *data, u64 offset,
>   		if (ret >= 0)
>   			return 0;
>   		/* Fall back for these errors, fail hard for anything else. */
> -		if (errno != ENOSPC && errno != ENOTTY && errno != EINVAL) {
> +		if (errno != ENOSPC && errno != ENOTTY &&
> +		    errno != EINVAL && errno != EPERM) {
>   			ret = -errno;
>   			error("encoded_write: writing to %s failed: %m", path);
>   			return ret;

Re: [PATCH] btrfs-progs: receive: retry encoded_write on EPERM

[David Sterba]

On Tue, Jul 02, 2024 at 09:27:49AM -0700, Boris Burkov wrote:
> encoded_write fails if we run without CAP_SYS_ADMIN, but the decompress
> and write fallback could succeed if we are running with write
> permissions on the file. Therefore, it is helpful to fall back on EPERM
> as well. While this will increase the "silent failure" rate of encoded
> writes, we do have the verbose log in place to debug that while setting
> up a receive workflow that expects encoded_write.

I'm not sure this is good to have the fallback by default. There's
option --force-decompress so it needs to be user's choice to avoid
encoded writes. Otherwise if I have a stream with encoded data and
receive it I expect to be as close to the original as possible.

Once there's encryption implemented the fallback will interfere with
availability of the key.

The problem with automatic fallback is that it cannot be disabled and it
does not fail so user can't know that it's been silenly not encoded
written.

Whether a steram contains an encoded write cannot be known upfront, I
think we should add more options how to handle the encoded write.
Modes like 'always', 'fallback'.

The code change itself is simple, this is about defining the expected
use cases.