diff mbox series

[v12,13/17] btrfs: add send stream v2 definitions

Message ID 09f343aa74b5cb2ce0a715f90c71ae64ae74ce94.1637179348.git.osandov@fb.com (mailing list archive)
State New, archived
Headers show
Series btrfs: add ioctls and send/receive support for reading/writing compressed data | expand

Commit Message

Omar Sandoval Nov. 17, 2021, 8:19 p.m. UTC 
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 version and flag.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
 fs/btrfs/send.c            |  2 +-
 fs/btrfs/send.h            | 35 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/btrfs.h |  7 +++++++
 3 files changed, 41 insertions(+), 3 deletions(-)

Comments

David Sterba Nov. 18, 2021, 2:18 p.m. UTC | #1 
On Wed, Nov 17, 2021 at 12:19:23PM -0800, Omar Sandoval wrote:
> @@ -113,6 +121,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,

I don't like the conditional meaning of the DATA attribute, I'd rather
see a new one that's v2+. It's adding a complexity that's not obvious
without some context.
David Sterba Nov. 18, 2021, 2:20 p.m. UTC | #2 
On Wed, Nov 17, 2021 at 12:19:23PM -0800, Omar Sandoval wrote:
> --- 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,
> @@ -80,7 +84,10 @@ enum btrfs_send_cmd {
>  	BTRFS_SEND_C_MAX_V1 = BTRFS_SEND_C_UPDATE_EXTENT,
>  
>  	/* Version 2 */
> -	BTRFS_SEND_C_MAX_V2 = BTRFS_SEND_C_MAX_V1,
> +	BTRFS_SEND_C_FALLOCATE,
> +	BTRFS_SEND_C_SETFLAGS,
> +	BTRFS_SEND_C_ENCODED_WRITE,
> +	BTRFS_SEND_C_MAX_V2 = BTRFS_SEND_C_ENCODED_WRITE,

The previous patch changes the MAX_V command to be equal to the previous
command but that's exactly what I wanted to avoid in the protocol
definition list and keep it linear.
Omar Sandoval Nov. 18, 2021, 7:08 p.m. UTC | #3 
On Thu, Nov 18, 2021 at 03:18:47PM +0100, David Sterba wrote:
> On Wed, Nov 17, 2021 at 12:19:23PM -0800, Omar Sandoval wrote:
> > @@ -113,6 +121,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,
> 
> I don't like the conditional meaning of the DATA attribute, I'd rather
> see a new one that's v2+. It's adding a complexity that's not obvious
> without some context.

Hm, I could add a BTRFS_SEND_A_DATA2, but then we'd need something like
this on the parsing side:

diff --git a/common/send-stream.c b/common/send-stream.c
index f25450c8..d6b0c10b 100644
--- a/common/send-stream.c
+++ b/common/send-stream.c
@@ -189,7 +189,7 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 
 		pos += sizeof(tlv_type);
 		data += sizeof(tlv_type);
-		if (sctx->version == 2 && tlv_type == BTRFS_SEND_A_DATA) {
+		if (tlv_type == BTRFS_SEND_A_DATA2) {
 			send_attr->tlv_len = cmd_len - pos;
 		} else {
 			if (cmd_len - pos < sizeof(__le16)) {
@@ -456,7 +456,10 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	case BTRFS_SEND_C_WRITE:
 		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
 		TLV_GET_U64(sctx, BTRFS_SEND_A_FILE_OFFSET, &offset);
-		TLV_GET(sctx, BTRFS_SEND_A_DATA, &data, &len);
+		if (sctx->cmd_attrs[BTRFS_SEND_A_DATA2].data)
+			TLV_GET(sctx, BTRFS_SEND_A_DATA2, &data, &len);
+		else
+			TLV_GET(sctx, BTRFS_SEND_A_DATA, &data, &len);
 		ret = sctx->ops->write(path, data, offset, len, sctx->user);
 		break;
 	case BTRFS_SEND_C_ENCODED_WRITE:
@@ -476,7 +479,10 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 			TLV_GET_U32(sctx, BTRFS_SEND_A_ENCRYPTION, &encryption);
 		else
 			encryption = BTRFS_ENCODED_IO_ENCRYPTION_NONE;
-		TLV_GET(sctx, BTRFS_SEND_A_DATA, &data, &len);
+		if (sctx->cmd_attrs[BTRFS_SEND_A_DATA2].data)
+			TLV_GET(sctx, BTRFS_SEND_A_DATA2, &data, &len);
+		else
+			TLV_GET(sctx, BTRFS_SEND_A_DATA, &data, &len);
 		ret = sctx->ops->encoded_write(path, data, offset, len,
 					       unencoded_file_len,
 					       unencoded_len, unencoded_offset,

It doesn't really make reading the attribute any clearer, and then we
have to check for two attributes. But, if you prefer it this way, I can
change it.

P.S. if we stick with my way, that sctx->version == 2 should probably be
sctx->version >= 2
diff mbox series

Patch

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 450c873684e8..53b3cc2276ea 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -7292,7 +7292,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 59a4be3b09cd..50c2284f08af 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,
@@ -80,7 +84,10 @@  enum btrfs_send_cmd {
 	BTRFS_SEND_C_MAX_V1 = BTRFS_SEND_C_UPDATE_EXTENT,
 
 	/* Version 2 */
-	BTRFS_SEND_C_MAX_V2 = BTRFS_SEND_C_MAX_V1,
+	BTRFS_SEND_C_FALLOCATE,
+	BTRFS_SEND_C_SETFLAGS,
+	BTRFS_SEND_C_ENCODED_WRITE,
+	BTRFS_SEND_C_MAX_V2 = BTRFS_SEND_C_ENCODED_WRITE,
 
 	/* End */
 	__BTRFS_SEND_C_MAX,
@@ -91,6 +98,7 @@  enum btrfs_send_cmd {
 enum {
 	BTRFS_SEND_A_UNSPEC,
 
+	/* Version 1 */
 	BTRFS_SEND_A_UUID,
 	BTRFS_SEND_A_CTRANSID,
 
@@ -113,6 +121,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,
 
 	BTRFS_SEND_A_CLONE_UUID,
@@ -120,7 +133,25 @@  enum {
 	BTRFS_SEND_A_CLONE_PATH,
 	BTRFS_SEND_A_CLONE_OFFSET,
 	BTRFS_SEND_A_CLONE_LEN,
+	BTRFS_SEND_A_MAX_V1 = BTRFS_SEND_A_CLONE_LEN,
 
+	/* Version 2 */
+	BTRFS_SEND_A_FALLOCATE_MODE,
+
+	BTRFS_SEND_A_SETFLAGS_FLAGS,
+
+	BTRFS_SEND_A_UNENCODED_FILE_LEN,
+	BTRFS_SEND_A_UNENCODED_LEN,
+	BTRFS_SEND_A_UNENCODED_OFFSET,
+	/*
+	 * COMPRESSION and ENCRYPTION default to NONE (0) if omitted from
+	 * BTRFS_SEND_C_ENCODED_WRITE.
+	 */
+	BTRFS_SEND_A_COMPRESSION,
+	BTRFS_SEND_A_ENCRYPTION,
+	BTRFS_SEND_A_MAX_V2 = BTRFS_SEND_A_ENCRYPTION,
+
+	/* End */
 	__BTRFS_SEND_A_MAX,
 };
 #define BTRFS_SEND_A_MAX (__BTRFS_SEND_A_MAX - 1)
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 7505acfa18d7..9d5fbe8c36c4 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -776,6 +776,13 @@  struct btrfs_ioctl_received_subvol_args {
  */
 #define BTRFS_SEND_FLAG_VERSION			0x8
 
+/*
+ * Send compressed data using the ENCODED_WRITE command instead of decompressing
+ * the data and sending it with the WRITE command. This requires protocol
+ * version >= 2.
+ */
+#define BTRFS_SEND_FLAG_COMPRESSED		0x10
+
 #define BTRFS_SEND_FLAG_MASK \
 	(BTRFS_SEND_FLAG_NO_FILE_DATA | \
 	 BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \