Re: [PATCH v12 13/17] btrfs: add send stream v2 definitions
From: Omar Sandoval <osandov@osandov.com>
Date: 2021-11-18 19:08:48
Subsystem:
the rest · Maintainer:
Linus Torvalds
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:quoted
@@ -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