Re: [PATCH v11 09/10] btrfs-progs: send: stream v2 ioctl flags

From: Nikolay Borisov <nborisov@suse.com>
To: Omar Sandoval <osandov@osandov.com>, linux-btrfs@vger.kernel.org
Cc: kernel-team@fb.com, linux-fsdevel@vger.kernel.org,
	linux-api@vger.kernel.org
Subject: Re: [PATCH v11 09/10] btrfs-progs: send: stream v2 ioctl flags
Date: Fri, 22 Oct 2021 09:35:37 +0300	[thread overview]
Message-ID: <b5f0cc44-6900-8cb8-a21e-4289102dbe23@suse.com> (raw)
In-Reply-To: <9228a836d43b1d721fbdbd662e1c8558cb27be67.1630515568.git.osandov@fb.com>

On 1.09.21 г. 20:01, Omar Sandoval wrote:
> From: Boris Burkov <boris@bur.io>
> 
> To make the btrfs send ioctl use the stream v2 format requires passing
> BTRFS_SEND_FLAG_STREAM_V2 in flags. Further, to cause the ioctl to emit
> encoded_write commands for encoded extents, we must set that flag as
> well as BTRFS_SEND_FLAG_COMPRESSED. Finally, we bump up the version in
> send.h as well, since we are now fully compatible with v2.
> 
> Add two command line arguments to btrfs send: --stream-version and
> --compressed-data. --stream-version requires an argument which it parses
> as an integer and sets STREAM_V2 if the argument is 2. --compressed-data
> does not require an argument and automatically implies STREAM_V2 as well
> (COMPRESSED alone causes the ioctl to error out).
> 
> Some examples to illustrate edge cases:
> 
> // v1, old format and no encoded_writes
> btrfs send subvol
> btrfs send --stream-version 1 subvol
> 
> // v2 and compressed, we will see encoded_writes
> btrfs send --compressed-data subvol
> btrfs send --compressed-data --stream-version 2 subvol
> 
> // v2 only, new format but no encoded_writes
> btrfs send --stream-version 2 subvol
> 
> // error: compressed needs version >= 2
> btrfs send --compressed-data --stream-version 1 subvol
> 
> // error: invalid version (not 1 or 2)
> btrfs send --stream-version 3 subvol
> btrfs send --compressed-data --stream-version 0 subvol
> btrfs send --compressed-data --stream-version 10 subvol

Why would we want to predicate the compressed writes usage on anything
other than the stream version?

> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
>  Documentation/btrfs-send.asciidoc | 16 ++++++++-
>  cmds/send.c                       | 54 ++++++++++++++++++++++++++++++-
>  ioctl.h                           | 17 +++++++++-
>  libbtrfsutil/btrfs.h              | 17 +++++++++-
>  send.h                            |  2 +-
>  5 files changed, 101 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/btrfs-send.asciidoc b/Documentation/btrfs-send.asciidoc
> index c4a05672..202bcd97 100644
> --- a/Documentation/btrfs-send.asciidoc
> +++ b/Documentation/btrfs-send.asciidoc
> @@ -55,7 +55,21 @@ send in 'NO_FILE_DATA' mode
>  The output stream does not contain any file
>  data and thus cannot be used to transfer changes. This mode is faster and
>  is useful to show the differences in metadata.
> --q|--quiet::::
> +
> +--stream-version <1|2>::
> +Use the given send stream version. The default is 1. Version 2 encodes file
> +data slightly more efficiently; it is also required for sending compressed data
> +directly (see '--compressed-data'). Version 2 requires at least btrfs-progs
> +5.12 on both the sender and receiver and at least Linux 5.12 on the sender.
> +

The version of progs needs to be adjusted but I assume this will be done
by David when this patchset is merged as we don't have a target ATM.

> +--compressed-data::
> +Send data that is compressed on the filesystem directly without decompressing
> +it. If the receiver supports encoded I/O (see `encoded_io`(7)), it can also
> +write it directly without decompressing it. Otherwise, the receiver will fall
> +back to decompressing it and writing it normally. This implies
> +'--stream-version 2'.
> +
> +-q|--quiet::
>  (deprecated) alias for global '-q' option
>  -v|--verbose::
>  (deprecated) alias for global '-v' option
> diff --git a/cmds/send.c b/cmds/send.c
> index 3bfc69f5..80eb2510 100644
> --- a/cmds/send.c
> +++ b/cmds/send.c
> @@ -452,6 +452,21 @@ static const char * const cmd_send_usage[] = {
>  	"                 does not contain any file data and thus cannot be used",
>  	"                 to transfer changes. This mode is faster and useful to",
>  	"                 show the differences in metadata.",
> +	"--stream-version <1|2>",
> +	"                 Use the given send stream version. The default is",
> +	"                 1. Version 2 encodes file data slightly more",
> +	"                 efficiently; it is also required for sending",
> +	"                 compressed data directly (see --compressed-data).",
> +	"                 Version 2 requires at least btrfs-progs 5.12 on both",
> +	"                 the sender and receiver and at least Linux 5.12 on the",
> +	"                 sender.",

Ditto

<snip>