Re: [PATCH v11 10/14] btrfs: add send stream v2 definitions

[Omar Sandoval <osandov@osandov.com>]

On Mon, Oct 18, 2021 at 06:11:54PM +0300, Nikolay Borisov wrote:
> 
> 
> On 1.09.21 г. 20:01, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > This adds the definitions of the new commands for send stream version 2
> > and their respective attributes: fallocate, FS_IOC_SETFLAGS (a.k.a.
> > chattr), and encoded writes. It also documents two changes to the send
> > stream format in v2: the receiver shouldn't assume a maximum command
> > size, and the DATA attribute is encoded differently to allow for writes
> > larger than 64k. These will be implemented in subsequent changes, and
> > then the ioctl will accept the new flags.
> > 
> > Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> > Signed-off-by: Omar Sandoval <osandov@fb.com>
> > ---
> >  fs/btrfs/send.c            |  2 +-
> >  fs/btrfs/send.h            | 30 +++++++++++++++++++++++++++++-
> >  include/uapi/linux/btrfs.h | 13 +++++++++++++
> >  3 files changed, 43 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> > index afdcbe7844e0..2ec07943f173 100644
> > --- a/fs/btrfs/send.c
> > +++ b/fs/btrfs/send.c
> > @@ -7287,7 +7287,7 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
> >  
> >  	sctx->clone_roots_cnt = arg->clone_sources_count;
> >  
> > -	sctx->send_max_size = BTRFS_SEND_BUF_SIZE;
> > +	sctx->send_max_size = BTRFS_SEND_BUF_SIZE_V1;
> >  	sctx->send_buf = kvmalloc(sctx->send_max_size, GFP_KERNEL);
> >  	if (!sctx->send_buf) {
> >  		ret = -ENOMEM;
> > diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
> > index de91488b7cd0..9f4f7b96b1eb 100644
> > --- a/fs/btrfs/send.h
> > +++ b/fs/btrfs/send.h
> > @@ -12,7 +12,11 @@
> >  #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
> >  #define BTRFS_SEND_STREAM_VERSION 1
> >  
> > -#define BTRFS_SEND_BUF_SIZE SZ_64K
> > +/*
> > + * In send stream v1, no command is larger than 64k. In send stream v2, no limit
> > + * should be assumed.
> > + */
> > +#define BTRFS_SEND_BUF_SIZE_V1 SZ_64K
> >  
> >  enum btrfs_tlv_type {
> >  	BTRFS_TLV_U8,
> > @@ -76,6 +80,13 @@ enum btrfs_send_cmd {
> >  
> >  	BTRFS_SEND_C_END,
> >  	BTRFS_SEND_C_UPDATE_EXTENT,
> > +
> > +	/* The following commands were added in send stream v2. */
> > +
> > +	BTRFS_SEND_C_FALLOCATE,
> > +	BTRFS_SEND_C_SETFLAGS,
> > +	BTRFS_SEND_C_ENCODED_WRITE,
> > +
> >  	__BTRFS_SEND_C_MAX,
> >  };
> >  #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1)
> > @@ -106,6 +117,11 @@ enum {
> >  	BTRFS_SEND_A_PATH_LINK,
> >  
> >  	BTRFS_SEND_A_FILE_OFFSET,
> > +	/*
> > +	 * In send stream v2, this attribute is special: it must be the last
> > +	 * attribute in a command, its header contains only the type, and its
> > +	 * length is implicitly the remaining length of the command.
> > +	 */
> >  	BTRFS_SEND_A_DATA,
> 
> Now that I think more about this, it would be best if this logic is
> actually codified in the code. I.e first set of SEND_A_DATA would set
> some bool/flag in the sctx and subsequent calls would be able to
> ASSERT/WARN ?

I suppose I could do something like this, is that what you had in mind?

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 90ca915fed78..46443d80b431 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -81,6 +81,7 @@ struct send_ctx {
 	char *send_buf;
 	u32 send_size;
 	u32 send_max_size;
+	bool put_data;
 	struct page **send_buf_pages;
 	u64 total_send_size;
 	u64 cmd_send_size[BTRFS_SEND_C_MAX + 1];
@@ -575,6 +576,9 @@ static int tlv_put(struct send_ctx *sctx, u16 attr, const void *data, int len)
 	int total_len = sizeof(*hdr) + len;
 	int left = sctx->send_max_size - sctx->send_size;
 
+	if (WARN_ON(sctx->put_data))
+		return -EINVAL;
+
 	if (unlikely(left < total_len))
 		return -EOVERFLOW;
 
@@ -718,6 +722,7 @@ static int send_cmd(struct send_ctx *sctx)
 	sctx->total_send_size += sctx->send_size;
 	sctx->cmd_send_size[get_unaligned_le16(&hdr->cmd)] += sctx->send_size;
 	sctx->send_size = 0;
+	sctx->put_data = false;
 
 	return ret;
 }
@@ -4913,6 +4918,9 @@ static inline u64 max_send_read_size(const struct send_ctx *sctx)
 
 static int put_data_header(struct send_ctx *sctx, u32 len)
 {
+	if (WARN_ON(sctx->put_data))
+		return -EINVAL;
+	sctx->put_data = true;
 	if (sctx->flags & BTRFS_SEND_FLAG_STREAM_V2) {
 		/*
 		 * In v2, the data attribute header doesn't include a length; it
@@ -5368,6 +5376,7 @@ static int send_encoded_extent(struct send_ctx *sctx, struct btrfs_path *path,
 	sctx->cmd_send_size[le16_to_cpu(hdr->cmd)] +=
 		sctx->send_size + block_len;
 	sctx->send_size = 0;
+	sctx->put_data = false;
 
 tlv_put_failure:
 out: