[PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[David Sterba]

This is send protocol update to version 2 with example new commands.

We have many pending protocol update requests but still don't have the
basic protocol rev in place, the first thing that must happen is to do
the actual versioning support. In order to have something to test,
there's an extended and a new command, that should be otherwise harmless
and nobody should depend on it. This should be enough to validate the
non-protocol changes and backward compatibility before we do the big
protocol update.

The protocol version is u32 and is a new member in the send ioctl
struct. Validity of the version field is backed by a new flag bit. Old
kernels would fail when a higher version is requested. Version protocol
0 will pick the highest supported version, BTRFS_SEND_STREAM_VERSION,
that's also exported in sysfs.

Protocol changes:

- new command BTRFS_SEND_C_UTIMES2
- appends OTIME after the output of BTRFS_SEND_C_UTIMES
- this is an example how to extend an existing command based on protocol
  version

- new command BTRFS_SEND_C_OTIME
- path BTRFS_SEND_A_PATH
- timespec attribute BTRFS_SEND_A_OTIME
- it's a separate command so it does not bloat any UTIMES2 commands,
  and is emitted only after inode creation (file, dir, special files).

The patch should be a template for further protocol extensions

RFC:

- set __BTRFS_SEND_C_MAX_V1 to the last command of the version or one
  beyond?
- drop UTIMES2 before release?
- naming?

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/send.c            | 73 ++++++++++++++++++++++++++++++++++++--
 fs/btrfs/send.h            | 11 +++++-
 include/uapi/linux/btrfs.h | 12 +++++--
 3 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index afdcbe7844e0..ca9eba5f2de3 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -84,6 +84,8 @@ struct send_ctx {
 	u64 total_send_size;
 	u64 cmd_send_size[BTRFS_SEND_C_MAX + 1];
 	u64 flags;	/* 'flags' member of btrfs_ioctl_send_args is u64 */
+	/* Protocol version compatibility requested */
+	u32 proto;
 
 	struct btrfs_root *send_root;
 	struct btrfs_root *parent_root;
@@ -312,6 +314,15 @@ static void inconsistent_snapshot_error(struct send_ctx *sctx,
 		   sctx->parent_root->root_key.objectid : 0));
 }
 
+static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
+{
+	switch (sctx->proto) {
+	case 1:	 return cmd < __BTRFS_SEND_C_MAX_V1;
+	case 2:	 return cmd < __BTRFS_SEND_C_MAX_V2;
+	default: return false;
+	}
+}
+
 static int is_waiting_for_move(struct send_ctx *sctx, u64 ino);
 
 static struct waiting_dir_move *
@@ -2507,6 +2518,7 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
 	struct extent_buffer *eb;
 	struct btrfs_key key;
 	int slot;
+	int cmd;
 
 	btrfs_debug(fs_info, "send_utimes %llu", ino);
 
@@ -2533,7 +2545,12 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
 	slot = path->slots[0];
 	ii = btrfs_item_ptr(eb, slot, struct btrfs_inode_item);
 
-	ret = begin_cmd(sctx, BTRFS_SEND_C_UTIMES);
+	if (proto_cmd_ok(sctx, BTRFS_SEND_C_UTIMES2))
+		cmd = BTRFS_SEND_C_UTIMES2;
+	else
+		cmd = BTRFS_SEND_C_UTIMES;
+
+	ret = begin_cmd(sctx, cmd);
 	if (ret < 0)
 		goto out;
 
@@ -2544,7 +2561,8 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
 	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_ATIME, eb, &ii->atime);
 	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_MTIME, eb, &ii->mtime);
 	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_CTIME, eb, &ii->ctime);
-	/* TODO Add otime support when the otime patches get into upstream */
+	if (proto_cmd_ok(sctx, BTRFS_SEND_C_UTIMES2))
+		TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, eb, &ii->otime);
 
 	ret = send_cmd(sctx);
 
@@ -2555,6 +2573,43 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
 	return ret;
 }
 
+static int send_inode_otime(struct send_ctx *sctx, const struct fs_path *fsp, u64 ino)
+{
+	int ret;
+	struct btrfs_path *path;
+	struct btrfs_inode_item *ii;
+	struct btrfs_key key;
+
+	if (!proto_cmd_ok(sctx, BTRFS_SEND_C_OTIME))
+		return 0;
+
+	path = alloc_path_for_send();
+	if (!path)
+		return -ENOMEM;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+	ret = btrfs_search_slot(NULL, sctx->send_root, &key, path, 0, 0);
+	if (ret) {
+		if (ret > 0)
+			ret = -ENOENT;
+		goto out;
+	}
+
+	ret = begin_cmd(sctx, BTRFS_SEND_C_OTIME);
+	ii = btrfs_item_ptr(path->nodes[0], path->slots[0], struct btrfs_inode_item);
+	TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, fsp);
+	TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, path->nodes[0], &ii->otime);
+	ret = send_cmd(sctx);
+
+tlv_put_failure:
+out:
+	btrfs_free_path(path);
+
+	return ret;
+}
+
 /*
  * Sends a BTRFS_SEND_C_MKXXX or SYMLINK command to user space. We don't have
  * a valid path yet because we did not process the refs yet. So, the inode
@@ -2633,6 +2688,9 @@ static int send_create_inode(struct send_ctx *sctx, u64 ino)
 	if (ret < 0)
 		goto out;
 
+	ret = send_inode_otime(sctx, p, ino);
+	if (ret < 0)
+		goto out;
 
 tlv_put_failure:
 out:
@@ -7269,6 +7327,17 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 
 	sctx->flags = arg->flags;
 
+	if (arg->flags & BTRFS_SEND_FLAG_VERSION) {
+		if (arg->version > BTRFS_SEND_STREAM_VERSION) {
+			ret = -EPROTO;
+			goto out;
+		}
+		/* Zero means "use the highest version" */
+		sctx->proto = arg->version ?: BTRFS_SEND_STREAM_VERSION;
+	} else {
+		sctx->proto = 1;
+	}
+
 	sctx->send_filp = fget(arg->send_fd);
 	if (!sctx->send_filp) {
 		ret = -EBADF;
diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
index de91488b7cd0..eeafbe2fdd9f 100644
--- a/fs/btrfs/send.h
+++ b/fs/btrfs/send.h
@@ -10,7 +10,7 @@
 #include "ctree.h"
 
 #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
-#define BTRFS_SEND_STREAM_VERSION 1
+#define BTRFS_SEND_STREAM_VERSION 2
 
 #define BTRFS_SEND_BUF_SIZE SZ_64K
 
@@ -48,6 +48,7 @@ struct btrfs_tlv_header {
 enum btrfs_send_cmd {
 	BTRFS_SEND_C_UNSPEC,
 
+	/* Version 1 */
 	BTRFS_SEND_C_SUBVOL,
 	BTRFS_SEND_C_SNAPSHOT,
 
@@ -76,6 +77,14 @@ enum btrfs_send_cmd {
 
 	BTRFS_SEND_C_END,
 	BTRFS_SEND_C_UPDATE_EXTENT,
+	__BTRFS_SEND_C_MAX_V1,
+
+	/* Version 2 */
+	BTRFS_SEND_C_UTIMES2,
+	BTRFS_SEND_C_OTIME,
+	__BTRFS_SEND_C_MAX_V2,
+
+	/* End */
 	__BTRFS_SEND_C_MAX,
 };
 #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1)
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index d7d3cfead056..c1a665d87f61 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -771,10 +771,16 @@ struct btrfs_ioctl_received_subvol_args {
  */
 #define BTRFS_SEND_FLAG_OMIT_END_CMD		0x4
 
+/*
+ * Read the protocol version in the structure
+ */
+#define BTRFS_SEND_FLAG_VERSION			0x8
+
 #define BTRFS_SEND_FLAG_MASK \
 	(BTRFS_SEND_FLAG_NO_FILE_DATA | \
 	 BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \
-	 BTRFS_SEND_FLAG_OMIT_END_CMD)
+	 BTRFS_SEND_FLAG_OMIT_END_CMD | \
+	 BTRFS_SEND_FLAG_VERSION)
 
 struct btrfs_ioctl_send_args {
 	__s64 send_fd;			/* in */
@@ -782,7 +788,9 @@ struct btrfs_ioctl_send_args {
 	__u64 __user *clone_sources;	/* in */
 	__u64 parent_root;		/* in */
 	__u64 flags;			/* in */
-	__u64 reserved[4];		/* in */
+	__u32 version;			/* in */
+	__u32 reserved32;
+	__u64 reserved[3];		/* in */
 };
 
 /*
-- 
2.33.0

[PATCH RFC] btrfs-progs: send protocol v2 stub, UTIMES2, OTIME

[David Sterba]

This is counterpart for the protocol version update.

- version 2 protocol
- new protocol command UTIMES2, same as utimes with additional otime
  data
- send: add command line options to specify version, compare against
  current running kernel supported version
- receive: parse UTIMES2
- receive: parse OTIME

TODO:

- libbtrfs compatibility is missing, ie. this will break anything that
  uses send stream (snapper), this needs library version update and
  maybe some ifdefs in the headers

Signed-off-by: David Sterba <dsterba@suse.com>
---
 cmds/receive-dump.c  | 31 ++++++++++++++++-
 cmds/receive.c       | 79 ++++++++++++++++++++++++++++++++++++++++++++
 cmds/send.c          | 66 ++++++++++++++++++++++++++++++++++--
 common/send-stream.c | 14 ++++++++
 common/send-stream.h |  5 +++
 ioctl.h              | 12 +++++--
 kernel-shared/send.h | 11 +++++-
 7 files changed, 212 insertions(+), 6 deletions(-)

diff --git a/cmds/receive-dump.c b/cmds/receive-dump.c
index 47a0a30e47db..7250228e6c42 100644
--- a/cmds/receive-dump.c
+++ b/cmds/receive-dump.c
@@ -312,6 +312,33 @@ static int print_utimes(const char *path, struct timespec *at,
 			  at_str, mt_str, ct_str);
 }
 
+static int print_utimes2(const char *path, struct timespec *at,
+			struct timespec *mt, struct timespec *ct,
+			struct timespec *ot, void *user)
+{
+	char at_str[TIME_STRING_MAX];
+	char mt_str[TIME_STRING_MAX];
+	char ct_str[TIME_STRING_MAX];
+	char ot_str[TIME_STRING_MAX];
+
+	if (sprintf_timespec(at, at_str, TIME_STRING_MAX - 1) < 0 ||
+	    sprintf_timespec(mt, mt_str, TIME_STRING_MAX - 1) < 0 ||
+	    sprintf_timespec(ct, ct_str, TIME_STRING_MAX - 1) < 0 ||
+	    sprintf_timespec(ot, ot_str, TIME_STRING_MAX - 1) < 0)
+		return -EINVAL;
+	return PRINT_DUMP(user, path, "utimes2", "atime=%s mtime=%s ctime=%s otime=%s",
+			  at_str, mt_str, ct_str, ot_str);
+}
+
+static int print_otime(const char *path, struct timespec *ot, void *user)
+{
+	char ot_str[TIME_STRING_MAX];
+
+	if (sprintf_timespec(ot, ot_str, TIME_STRING_MAX - 1) < 0)
+		return -EINVAL;
+	return PRINT_DUMP(user, path, "otime", "otime=%s", ot_str);
+}
+
 static int print_update_extent(const char *path, u64 offset, u64 len,
 			       void *user)
 {
@@ -340,5 +367,7 @@ struct btrfs_send_ops btrfs_print_send_ops = {
 	.chmod = print_chmod,
 	.chown = print_chown,
 	.utimes = print_utimes,
-	.update_extent = print_update_extent
+	.update_extent = print_update_extent,
+	.utimes2 = print_utimes2,
+	.otime = print_otime,
 };
diff --git a/cmds/receive.c b/cmds/receive.c
index 4d123a1f8782..dfb37a502598 100644
--- a/cmds/receive.c
+++ b/cmds/receive.c
@@ -967,6 +967,83 @@ static int process_utimes(const char *path, struct timespec *at,
 	return ret;
 }
 
+static int process_utimes2(const char *path, struct timespec *at,
+			  struct timespec *mt, struct timespec *ct,
+			  struct timespec *ot, void *user)
+{
+	int ret = 0;
+	struct btrfs_receive *rctx = user;
+	char full_path[PATH_MAX];
+	struct timespec tv[2];
+
+	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
+	if (ret < 0) {
+		error("utimes2: path invalid: %s", path);
+		goto out;
+	}
+
+	if (bconf.verbose >= 3)
+		fprintf(stderr, "utimes2 %s\n", path);
+
+	tv[0] = *at;
+	tv[1] = *mt;
+	ret = utimensat(AT_FDCWD, full_path, tv, AT_SYMLINK_NOFOLLOW);
+	if (ret < 0) {
+		ret = -errno;
+		error("utimes2 %s failed: %m", path);
+		goto out;
+	}
+
+out:
+	return ret;
+}
+
+/* TODO: Copied from receive-dump.c */
+static int sprintf_timespec(struct timespec *ts, char *dest, int max_size)
+{
+	struct tm tm;
+	int ret;
+
+	if (!localtime_r(&ts->tv_sec, &tm)) {
+		error("failed to convert time %lld.%.9ld to local time",
+		      (long long)ts->tv_sec, ts->tv_nsec);
+		return -EINVAL;
+	}
+	ret = strftime(dest, max_size, "%FT%T%z", &tm);
+	if (ret == 0) {
+		error(
+		"time %lld.%ld is too long to convert into readable string",
+		      (long long)ts->tv_sec, ts->tv_nsec);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+
+static int process_otime(const char *path, struct timespec *ot, void *user)
+{
+	int ret;
+	struct btrfs_receive *rctx = user;
+	char full_path[PATH_MAX];
+
+	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
+	if (ret < 0) {
+		error("otime: path invalid: %s", path);
+		goto out;
+	}
+
+	if (bconf.verbose >= 3) {
+		char ot_str[128];
+
+		if (sprintf_timespec(ot, ot_str, sizeof(ot_str) - 1) < 0)
+			goto out;
+		fprintf(stderr, "otime %s\n", ot_str);
+	}
+
+out:
+	return 0;
+}
+
 static int process_update_extent(const char *path, u64 offset, u64 len,
 		void *user)
 {
@@ -1004,6 +1081,8 @@ static struct btrfs_send_ops send_ops = {
 	.chown = process_chown,
 	.utimes = process_utimes,
 	.update_extent = process_update_extent,
+	.utimes2 = process_utimes2,
+	.otime = process_otime,
 };
 
 static int do_receive(struct btrfs_receive *rctx, const char *tomnt,
diff --git a/cmds/send.c b/cmds/send.c
index 1810233137aa..f529f0b0a6a0 100644
--- a/cmds/send.c
+++ b/cmds/send.c
@@ -57,6 +57,8 @@ struct btrfs_send {
 	u64 clone_sources_count;
 
 	char *root_path;
+	u32 proto;
+	u32 proto_supported;
 };
 
 static int get_root_id(struct btrfs_send *sctx, const char *path, u64 *root_id)
@@ -257,6 +259,13 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
 	memset(&io_send, 0, sizeof(io_send));
 	io_send.send_fd = pipefd[1];
 	send->send_fd = pipefd[0];
+	io_send.flags = flags;
+
+	if (send->proto_supported > 1) {
+		/* Versioned stream supported, requesting default or specific number */
+		io_send.version = send->proto;
+		io_send.flags |= BTRFS_SEND_FLAG_VERSION;
+	}
 
 	if (!ret)
 		ret = pthread_create(&t_read, NULL, read_sent_data, send);
@@ -267,7 +276,6 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
 		goto out;
 	}
 
-	io_send.flags = flags;
 	io_send.clone_sources = (__u64*)send->clone_sources;
 	io_send.clone_sources_count = send->clone_sources_count;
 	io_send.parent_root = parent_root_id;
@@ -275,6 +283,7 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
 		io_send.flags |= BTRFS_SEND_FLAG_OMIT_STREAM_HEADER;
 	if (!is_last_subvol)
 		io_send.flags |= BTRFS_SEND_FLAG_OMIT_END_CMD;
+
 	ret = ioctl(subvol_fd, BTRFS_IOC_SEND, &io_send);
 	if (ret < 0) {
 		ret = -errno;
@@ -419,6 +428,33 @@ static void free_send_info(struct btrfs_send *sctx)
 	sctx->root_path = NULL;
 }
 
+static u32 get_sysfs_proto_supported(void)
+{
+	int fd;
+	int ret;
+	char buf[32] = {};
+	char *end = NULL;
+	u64 version;
+
+	fd = sysfs_open_file("features/send_stream_version");
+	if (fd < 0) {
+		/* No file is either no version support or old kernel with just v1 */
+		return 1;
+	}
+	ret = sysfs_read_file(fd, buf, sizeof(buf));
+	close(fd);
+	if (ret <= 0)
+		return 1;
+	version = strtoull(buf, &end, 10);
+	if (version == ULLONG_MAX && errno == ERANGE)
+		return 1;
+	if (version > U32_MAX) {
+		warning("sysfs/send_stream_version too big: %llu", version);
+		version = 1;
+	}
+	return version;
+}
+
 static const char * const cmd_send_usage[] = {
 	"btrfs send [-ve] [-p <parent>] [-c <clone-src>] [-f <outfile>] <subvol> [<subvol>...]",
 	"Send the subvolume(s) to stdout.",
@@ -447,6 +483,7 @@ 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.",
+	"--proto N        request maximum protocol version N (default: highest supported by running kernel)",
 	"-v|--verbose     deprecated, alias for global -v option",
 	"-q|--quiet       deprecated, alias for global -q option",
 	HELPINFO_INSERT_GLOBALS,
@@ -472,6 +509,7 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 
 	memset(&send, 0, sizeof(send));
 	send.dump_fd = fileno(stdout);
+	send.proto = 0;
 	outname[0] = 0;
 
 	/*
@@ -487,11 +525,12 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 
 	optind = 0;
 	while (1) {
-		enum { GETOPT_VAL_SEND_NO_DATA = 256 };
+		enum { GETOPT_VAL_SEND_NO_DATA = 256, GETOPT_VAL_PROTO };
 		static const struct option long_options[] = {
 			{ "verbose", no_argument, NULL, 'v' },
 			{ "quiet", no_argument, NULL, 'q' },
 			{ "no-data", no_argument, NULL, GETOPT_VAL_SEND_NO_DATA },
+			{ "proto" , required_argument, NULL, GETOPT_VAL_PROTO },
 			{ NULL, 0, NULL, 0 }
 		};
 		int c = getopt_long(argc, argv, "vqec:f:i:p:", long_options, NULL);
@@ -580,6 +619,17 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 		case GETOPT_VAL_SEND_NO_DATA:
 			send_flags |= BTRFS_SEND_FLAG_NO_FILE_DATA;
 			break;
+		case GETOPT_VAL_PROTO: {
+			u64 tmp;
+
+			tmp = arg_strtou64(optarg);
+			if (tmp > U32_MAX) {
+				error("protocol version number too big %llu", tmp);
+				ret = 1;
+				goto out;
+			}
+			send.proto = tmp;
+			break; }
 		default:
 			usage_unknown_option(cmd, argv);
 		}
@@ -687,6 +737,18 @@ static int cmd_send(const struct cmd_struct *cmd, int argc, char **argv)
 	if ((send_flags & BTRFS_SEND_FLAG_NO_FILE_DATA) && bconf.verbose > 1)
 		if (bconf.verbose > 1)
 			fprintf(stderr, "Mode NO_FILE_DATA enabled\n");
+	send.proto_supported = get_sysfs_proto_supported();
+	if (send.proto_supported == 1) {
+		if (send.proto > send.proto_supported) {
+			error("requested version %u but kernel supports only %u",
+				send.proto, send.proto_supported);
+			ret = -EPROTO;
+			goto out;
+		}
+	}
+	if (bconf.verbose > 1)
+		fprintf(stderr, "Protocol version requested: %u (supported %u)\n",
+				send.proto, send.proto_supported);
 
 	for (i = optind; i < argc; i++) {
 		int is_first_subvol;
diff --git a/common/send-stream.c b/common/send-stream.c
index d07748ced1fe..0b0558cec671 100644
--- a/common/send-stream.c
+++ b/common/send-stream.c
@@ -314,6 +314,7 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 	struct timespec at;
 	struct timespec ct;
 	struct timespec mt;
+	struct timespec ot;
 	u8 uuid[BTRFS_UUID_SIZE];
 	u8 clone_uuid[BTRFS_UUID_SIZE];
 	u64 tmp;
@@ -451,6 +452,19 @@ static int read_and_process_cmd(struct btrfs_send_stream *sctx)
 		TLV_GET_U64(sctx, BTRFS_SEND_A_SIZE, &tmp);
 		ret = sctx->ops->update_extent(path, offset, tmp, sctx->user);
 		break;
+	case BTRFS_SEND_C_UTIMES2:
+		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
+		TLV_GET_TIMESPEC(sctx, BTRFS_SEND_A_ATIME, &at);
+		TLV_GET_TIMESPEC(sctx, BTRFS_SEND_A_MTIME, &mt);
+		TLV_GET_TIMESPEC(sctx, BTRFS_SEND_A_CTIME, &ct);
+		TLV_GET_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, &ot);
+		ret = sctx->ops->utimes2(path, &at, &mt, &ct, &ot, sctx->user);
+		break;
+	case BTRFS_SEND_C_OTIME:
+		TLV_GET_STRING(sctx, BTRFS_SEND_A_PATH, &path);
+		TLV_GET_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, &ot);
+		ret = sctx->ops->otime(path, &ot, sctx->user);
+		break;
 	case BTRFS_SEND_C_END:
 		ret = 1;
 		break;
diff --git a/common/send-stream.h b/common/send-stream.h
index 2de51eac5548..be6c71031db0 100644
--- a/common/send-stream.h
+++ b/common/send-stream.h
@@ -53,6 +53,11 @@ struct btrfs_send_ops {
 		      struct timespec *mt, struct timespec *ct,
 		      void *user);
 	int (*update_extent)(const char *path, u64 offset, u64 len, void *user);
+	/* V2 */
+	int (*utimes2)(const char *path, struct timespec *at,
+		      struct timespec *mt, struct timespec *ct,
+		      struct timespec *ot, void *user);
+	int (*otime)(const char *path, struct timespec *ot, void *user);
 };
 
 int btrfs_read_and_process_send_stream(int fd,
diff --git a/ioctl.h b/ioctl.h
index 368a87b27711..2901d92b64e8 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -655,10 +655,16 @@ BUILD_ASSERT(sizeof(struct btrfs_ioctl_received_subvol_args_32) == 192);
  */
 #define BTRFS_SEND_FLAG_OMIT_END_CMD		0x4
 
+/*
+ * Read the protocol version in the structure
+ */
+#define BTRFS_SEND_FLAG_VERSION			0x8
+
 #define BTRFS_SEND_FLAG_MASK \
 	(BTRFS_SEND_FLAG_NO_FILE_DATA | \
 	 BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \
-	 BTRFS_SEND_FLAG_OMIT_END_CMD)
+	 BTRFS_SEND_FLAG_OMIT_END_CMD | \
+	 BTRFS_SEND_FLAG_VERSION)
 
 struct btrfs_ioctl_send_args {
 	__s64 send_fd;			/* in */
@@ -666,7 +672,9 @@ struct btrfs_ioctl_send_args {
 	__u64 __user *clone_sources;	/* in */
 	__u64 parent_root;		/* in */
 	__u64 flags;			/* in */
-	__u64 reserved[4];		/* in */
+	__u32 version;			/* in */
+	__u32 reserved32;
+	__u64 reserved[3];		/* in */
 };
 /*
  * Size of structure depends on pointer width, was not caught in the early
diff --git a/kernel-shared/send.h b/kernel-shared/send.h
index e73f09dfe5d7..8b927313ff76 100644
--- a/kernel-shared/send.h
+++ b/kernel-shared/send.h
@@ -31,7 +31,7 @@ extern "C" {
 #endif
 
 #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
-#define BTRFS_SEND_STREAM_VERSION 1
+#define BTRFS_SEND_STREAM_VERSION 2
 
 #define BTRFS_SEND_BUF_SIZE  (64 * 1024)
 #define BTRFS_SEND_READ_SIZE (1024 * 48)
@@ -70,6 +70,7 @@ struct btrfs_tlv_header {
 enum btrfs_send_cmd {
 	BTRFS_SEND_C_UNSPEC,
 
+	/* Version 1 */
 	BTRFS_SEND_C_SUBVOL,
 	BTRFS_SEND_C_SNAPSHOT,
 
@@ -98,6 +99,14 @@ enum btrfs_send_cmd {
 
 	BTRFS_SEND_C_END,
 	BTRFS_SEND_C_UPDATE_EXTENT,
+	__BTRFS_SEND_C_MAX_V1,
+
+	/* Version 2 */
+	BTRFS_SEND_C_UTIMES2,
+	BTRFS_SEND_C_OTIME,
+	__BTRFS_SEND_C_MAX_V2,
+
+	/* End */
 	__BTRFS_SEND_C_MAX,
 };
 #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1)
-- 
2.33.0

Re: [PATCH RFC] btrfs-progs: send protocol v2 stub, UTIMES2, OTIME

[Omar Sandoval]

On Mon, Oct 18, 2021 at 04:47:17PM +0200, David Sterba wrote:
> This is counterpart for the protocol version update.
> 
> - version 2 protocol
> - new protocol command UTIMES2, same as utimes with additional otime
>   data
> - send: add command line options to specify version, compare against
>   current running kernel supported version
> - receive: parse UTIMES2
> - receive: parse OTIME
> 
> TODO:
> 
> - libbtrfs compatibility is missing, ie. this will break anything that
>   uses send stream (snapper), this needs library version update and
>   maybe some ifdefs in the headers
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  cmds/receive-dump.c  | 31 ++++++++++++++++-
>  cmds/receive.c       | 79 ++++++++++++++++++++++++++++++++++++++++++++
>  cmds/send.c          | 66 ++++++++++++++++++++++++++++++++++--
>  common/send-stream.c | 14 ++++++++
>  common/send-stream.h |  5 +++
>  ioctl.h              | 12 +++++--
>  kernel-shared/send.h | 11 +++++-
>  7 files changed, 212 insertions(+), 6 deletions(-)
> 
> diff --git a/cmds/receive.c b/cmds/receive.c
> index 4d123a1f8782..dfb37a502598 100644
> --- a/cmds/receive.c
> +++ b/cmds/receive.c
> @@ -967,6 +967,83 @@ static int process_utimes(const char *path, struct timespec *at,
>  	return ret;
>  }
>  
> +static int process_utimes2(const char *path, struct timespec *at,
> +			  struct timespec *mt, struct timespec *ct,
> +			  struct timespec *ot, void *user)
> +{
> +	int ret = 0;
> +	struct btrfs_receive *rctx = user;
> +	char full_path[PATH_MAX];
> +	struct timespec tv[2];
> +
> +	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
> +	if (ret < 0) {
> +		error("utimes2: path invalid: %s", path);
> +		goto out;
> +	}
> +
> +	if (bconf.verbose >= 3)
> +		fprintf(stderr, "utimes2 %s\n", path);
> +
> +	tv[0] = *at;
> +	tv[1] = *mt;
> +	ret = utimensat(AT_FDCWD, full_path, tv, AT_SYMLINK_NOFOLLOW);
> +	if (ret < 0) {
> +		ret = -errno;
> +		error("utimes2 %s failed: %m", path);
> +		goto out;
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +/* TODO: Copied from receive-dump.c */
> +static int sprintf_timespec(struct timespec *ts, char *dest, int max_size)
> +{
> +	struct tm tm;
> +	int ret;
> +
> +	if (!localtime_r(&ts->tv_sec, &tm)) {
> +		error("failed to convert time %lld.%.9ld to local time",
> +		      (long long)ts->tv_sec, ts->tv_nsec);
> +		return -EINVAL;
> +	}
> +	ret = strftime(dest, max_size, "%FT%T%z", &tm);
> +	if (ret == 0) {
> +		error(
> +		"time %lld.%ld is too long to convert into readable string",
> +		      (long long)ts->tv_sec, ts->tv_nsec);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +
> +static int process_otime(const char *path, struct timespec *ot, void *user)
> +{
> +	int ret;
> +	struct btrfs_receive *rctx = user;
> +	char full_path[PATH_MAX];
> +
> +	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
> +	if (ret < 0) {
> +		error("otime: path invalid: %s", path);
> +		goto out;
> +	}
> +
> +	if (bconf.verbose >= 3) {
> +		char ot_str[128];
> +
> +		if (sprintf_timespec(ot, ot_str, sizeof(ot_str) - 1) < 0)
> +			goto out;
> +		fprintf(stderr, "otime %s\n", ot_str);
> +	}
> +
> +out:
> +	return 0;
> +}

Are you planning to do anything with otime (e.g., storing it in an
xattr) in the future?

>  static int do_receive(struct btrfs_receive *rctx, const char *tomnt,
> diff --git a/cmds/send.c b/cmds/send.c
> index 1810233137aa..f529f0b0a6a0 100644
> --- a/cmds/send.c
> +++ b/cmds/send.c
> @@ -57,6 +57,8 @@ struct btrfs_send {
>  	u64 clone_sources_count;
>  
>  	char *root_path;
> +	u32 proto;
> +	u32 proto_supported;
>  };
>  
>  static int get_root_id(struct btrfs_send *sctx, const char *path, u64 *root_id)
> @@ -257,6 +259,13 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
>  	memset(&io_send, 0, sizeof(io_send));
>  	io_send.send_fd = pipefd[1];
>  	send->send_fd = pipefd[0];
> +	io_send.flags = flags;
> +
> +	if (send->proto_supported > 1) {
> +		/* Versioned stream supported, requesting default or specific number */
> +		io_send.version = send->proto;
> +		io_send.flags |= BTRFS_SEND_FLAG_VERSION;
> +	}
>  
>  	if (!ret)
>  		ret = pthread_create(&t_read, NULL, read_sent_data, send);
> @@ -267,7 +276,6 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
>  		goto out;
>  	}
>  
> -	io_send.flags = flags;
>  	io_send.clone_sources = (__u64*)send->clone_sources;
>  	io_send.clone_sources_count = send->clone_sources_count;
>  	io_send.parent_root = parent_root_id;
> @@ -275,6 +283,7 @@ static int do_send(struct btrfs_send *send, u64 parent_root_id,
>  		io_send.flags |= BTRFS_SEND_FLAG_OMIT_STREAM_HEADER;
>  	if (!is_last_subvol)
>  		io_send.flags |= BTRFS_SEND_FLAG_OMIT_END_CMD;
> +
>  	ret = ioctl(subvol_fd, BTRFS_IOC_SEND, &io_send);
>  	if (ret < 0) {
>  		ret = -errno;
> @@ -419,6 +428,33 @@ static void free_send_info(struct btrfs_send *sctx)
>  	sctx->root_path = NULL;
>  }
>  
> +static u32 get_sysfs_proto_supported(void)
> +{
> +	int fd;
> +	int ret;
> +	char buf[32] = {};
> +	char *end = NULL;
> +	u64 version;
> +
> +	fd = sysfs_open_file("features/send_stream_version");
> +	if (fd < 0) {
> +		/* No file is either no version support or old kernel with just v1 */
> +		return 1;
> +	}
> +	ret = sysfs_read_file(fd, buf, sizeof(buf));
> +	close(fd);
> +	if (ret <= 0)
> +		return 1;
> +	version = strtoull(buf, &end, 10);
> +	if (version == ULLONG_MAX && errno == ERANGE)
> +		return 1;
> +	if (version > U32_MAX) {
> +		warning("sysfs/send_stream_version too big: %llu", version);
> +		version = 1;
> +	}
> +	return version;
> +}
> +
>  static const char * const cmd_send_usage[] = {
>  	"btrfs send [-ve] [-p <parent>] [-c <clone-src>] [-f <outfile>] <subvol> [<subvol>...]",
>  	"Send the subvolume(s) to stdout.",
> @@ -447,6 +483,7 @@ 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.",
> +	"--proto N        request maximum protocol version N (default: highest supported by running kernel)",

Can we default to version 1 and provide a way to opt in to the latest
version? I'm concerned with a kernel upgrade suddenly creating a send
stream that the receiving side can't handle. Making this opt-in rather
than opt-out seems safer.

Re: [PATCH RFC] btrfs-progs: send protocol v2 stub, UTIMES2, OTIME

[David Sterba]

On Mon, Oct 18, 2021 at 02:42:34PM -0700, Omar Sandoval wrote:
> On Mon, Oct 18, 2021 at 04:47:17PM +0200, David Sterba wrote:
> > +	int ret;
> > +	struct btrfs_receive *rctx = user;
> > +	char full_path[PATH_MAX];
> > +
> > +	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
> > +	if (ret < 0) {
> > +		error("otime: path invalid: %s", path);
> > +		goto out;
> > +	}
> > +
> > +	if (bconf.verbose >= 3) {
> > +		char ot_str[128];
> > +
> > +		if (sprintf_timespec(ot, ot_str, sizeof(ot_str) - 1) < 0)
> > +			goto out;
> > +		fprintf(stderr, "otime %s\n", ot_str);
> > +	}
> > +
> > +out:
> > +	return 0;
> > +}
> 
> Are you planning to do anything with otime (e.g., storing it in an
> xattr) in the future?

At this point I don't have further plan to use the value on the receive
side, there's no standard way to track otime outside of the inode. This
is up to the application, it can be stored in a xattr or just
cross-referenced with some other data.

I don't remember if there was a specific request for the otime in the
protocol, but for completeness of the information it makes sense to
transfer it to the receiving side.

> >  static const char * const cmd_send_usage[] = {
> >  	"btrfs send [-ve] [-p <parent>] [-c <clone-src>] [-f <outfile>] <subvol> [<subvol>...]",
> >  	"Send the subvolume(s) to stdout.",
> > @@ -447,6 +483,7 @@ 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.",
> > +	"--proto N        request maximum protocol version N (default: highest supported by running kernel)",
> 
> Can we default to version 1 and provide a way to opt in to the latest
> version? I'm concerned with a kernel upgrade suddenly creating a send
> stream that the receiving side can't handle. Making this opt-in rather
> than opt-out seems safer.

Default to v1 is safe, but what's your idea when to change it?

The send/receive usecase has 4 moving parts, kernel/send, progs/send,
progs/receive, kernel/receive. On different hosts and both parts
happening at potentially different times. Getting out of that with sane
defaults will be fun.

I agree that bumping the default any time kernel with new protocol
version support can break things quite easily, so that's against the
usability principles.

As the default is in progs it's a bit more flexible as it does not
require kernel update/reboot. The maximum supported value is known and
the 'default' value 0 for proto version allows that in a way that does
not require updating scripts.

So for first public release, I'll keep the default 1. Changes in the
default version can follow a similar way like mkfs, ie. when there's a
sufficient support in stable kernels. We'll probably also gather some
feedback over time so can decide accordingly.

Re: [PATCH RFC] btrfs-progs: send protocol v2 stub, UTIMES2, OTIME

[Josef Bacik]

On Tue, Oct 19, 2021 at 02:53:59PM +0200, David Sterba wrote:
> On Mon, Oct 18, 2021 at 02:42:34PM -0700, Omar Sandoval wrote:
> > On Mon, Oct 18, 2021 at 04:47:17PM +0200, David Sterba wrote:
> > > +	int ret;
> > > +	struct btrfs_receive *rctx = user;
> > > +	char full_path[PATH_MAX];
> > > +
> > > +	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
> > > +	if (ret < 0) {
> > > +		error("otime: path invalid: %s", path);
> > > +		goto out;
> > > +	}
> > > +
> > > +	if (bconf.verbose >= 3) {
> > > +		char ot_str[128];
> > > +
> > > +		if (sprintf_timespec(ot, ot_str, sizeof(ot_str) - 1) < 0)
> > > +			goto out;
> > > +		fprintf(stderr, "otime %s\n", ot_str);
> > > +	}
> > > +
> > > +out:
> > > +	return 0;
> > > +}
> > 
> > Are you planning to do anything with otime (e.g., storing it in an
> > xattr) in the future?
> 
> At this point I don't have further plan to use the value on the receive
> side, there's no standard way to track otime outside of the inode. This
> is up to the application, it can be stored in a xattr or just
> cross-referenced with some other data.
> 
> I don't remember if there was a specific request for the otime in the
> protocol, but for completeness of the information it makes sense to
> transfer it to the receiving side.
> 
> > >  static const char * const cmd_send_usage[] = {
> > >  	"btrfs send [-ve] [-p <parent>] [-c <clone-src>] [-f <outfile>] <subvol> [<subvol>...]",
> > >  	"Send the subvolume(s) to stdout.",
> > > @@ -447,6 +483,7 @@ 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.",
> > > +	"--proto N        request maximum protocol version N (default: highest supported by running kernel)",
> > 
> > Can we default to version 1 and provide a way to opt in to the latest
> > version? I'm concerned with a kernel upgrade suddenly creating a send
> > stream that the receiving side can't handle. Making this opt-in rather
> > than opt-out seems safer.
> 
> Default to v1 is safe, but what's your idea when to change it?
> 
> The send/receive usecase has 4 moving parts, kernel/send, progs/send,
> progs/receive, kernel/receive. On different hosts and both parts
> happening at potentially different times. Getting out of that with sane
> defaults will be fun.
> 
> I agree that bumping the default any time kernel with new protocol
> version support can break things quite easily, so that's against the
> usability principles.
> 
> As the default is in progs it's a bit more flexible as it does not
> require kernel update/reboot. The maximum supported value is known and
> the 'default' value 0 for proto version allows that in a way that does
> not require updating scripts.
> 
> So for first public release, I'll keep the default 1. Changes in the
> default version can follow a similar way like mkfs, ie. when there's a
> sufficient support in stable kernels. We'll probably also gather some
> feedback over time so can decide accordingly.

It's waaaaaaay easier to go "oh shit, I need a new btrfs-progs" than "oh shit, I
need a new kernel."  I think that having the kernel generate new send streams is
reasonable, because distro's are more likely to lag behind on kernels than
userspace.  If we have users on the cutting edge they're likely to be on the
cutting edge in both cases.  And if they get a sendstream that doesn't apply we
have a nice (at least I hope) message that says "this send stream is too new,
try a newer btrfs-progs", so it's an easy remedy.  Thanks,

Josef

Re: [PATCH RFC] btrfs-progs: send protocol v2 stub, UTIMES2, OTIME

[Omar Sandoval]

On Tue, Oct 19, 2021 at 02:53:59PM +0200, David Sterba wrote:
> On Mon, Oct 18, 2021 at 02:42:34PM -0700, Omar Sandoval wrote:
> > On Mon, Oct 18, 2021 at 04:47:17PM +0200, David Sterba wrote:
> > > +	int ret;
> > > +	struct btrfs_receive *rctx = user;
> > > +	char full_path[PATH_MAX];
> > > +
> > > +	ret = path_cat_out(full_path, rctx->full_subvol_path, path);
> > > +	if (ret < 0) {
> > > +		error("otime: path invalid: %s", path);
> > > +		goto out;
> > > +	}
> > > +
> > > +	if (bconf.verbose >= 3) {
> > > +		char ot_str[128];
> > > +
> > > +		if (sprintf_timespec(ot, ot_str, sizeof(ot_str) - 1) < 0)
> > > +			goto out;
> > > +		fprintf(stderr, "otime %s\n", ot_str);
> > > +	}
> > > +
> > > +out:
> > > +	return 0;
> > > +}
> > 
> > Are you planning to do anything with otime (e.g., storing it in an
> > xattr) in the future?
> 
> At this point I don't have further plan to use the value on the receive
> side, there's no standard way to track otime outside of the inode. This
> is up to the application, it can be stored in a xattr or just
> cross-referenced with some other data.

In that case, for the majority of users that are just sending subvolumes
from point A to point B, this is unnecessary metadata for no benefit.
But it's so small relative to the actual file data that it probably
doesn't matter, and I think it's a good enough proof of concept.

> I don't remember if there was a specific request for the otime in the
> protocol, but for completeness of the information it makes sense to
> transfer it to the receiving side.
> 
> > >  static const char * const cmd_send_usage[] = {
> > >  	"btrfs send [-ve] [-p <parent>] [-c <clone-src>] [-f <outfile>] <subvol> [<subvol>...]",
> > >  	"Send the subvolume(s) to stdout.",
> > > @@ -447,6 +483,7 @@ 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.",
> > > +	"--proto N        request maximum protocol version N (default: highest supported by running kernel)",
> > 
> > Can we default to version 1 and provide a way to opt in to the latest
> > version? I'm concerned with a kernel upgrade suddenly creating a send
> > stream that the receiving side can't handle. Making this opt-in rather
> > than opt-out seems safer.
> 
> Default to v1 is safe, but what's your idea when to change it?
> 
> The send/receive usecase has 4 moving parts, kernel/send, progs/send,
> progs/receive, kernel/receive. On different hosts and both parts
> happening at potentially different times. Getting out of that with sane
> defaults will be fun.
> 
> I agree that bumping the default any time kernel with new protocol
> version support can break things quite easily, so that's against the
> usability principles.
> 
> As the default is in progs it's a bit more flexible as it does not
> require kernel update/reboot. The maximum supported value is known and
> the 'default' value 0 for proto version allows that in a way that does
> not require updating scripts.
> 
> So for first public release, I'll keep the default 1. Changes in the
> default version can follow a similar way like mkfs, ie. when there's a
> sufficient support in stable kernels. We'll probably also gather some
> feedback over time so can decide accordingly.

Josef has a good point. For the majority of new commands/attributes, you
only need to update btrfs-progs on the receive side, which is easy. We
just need to be more careful about new commands that require new kernel
support. Compressed data is one of those cases, which is why I made it
opt-in separate from the new protocol version.

So, I think we can change the default to the latest version after only
one or two releases, once people become aware that protocol version
revisions are happening.

Re: [PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[Filipe Manana]

On Mon, Oct 18, 2021 at 3:43 PM David Sterba <dsterba@suse.com> wrote:
>
> This is send protocol update to version 2 with example new commands.
>
> We have many pending protocol update requests but still don't have the
> basic protocol rev in place, the first thing that must happen is to do
> the actual versioning support. In order to have something to test,
> there's an extended and a new command, that should be otherwise harmless
> and nobody should depend on it. This should be enough to validate the
> non-protocol changes and backward compatibility before we do the big
> protocol update.
>
> The protocol version is u32 and is a new member in the send ioctl
> struct. Validity of the version field is backed by a new flag bit. Old
> kernels would fail when a higher version is requested. Version protocol
> 0 will pick the highest supported version, BTRFS_SEND_STREAM_VERSION,
> that's also exported in sysfs.
>
> Protocol changes:
>
> - new command BTRFS_SEND_C_UTIMES2
> - appends OTIME after the output of BTRFS_SEND_C_UTIMES
> - this is an example how to extend an existing command based on protocol
>   version
>
> - new command BTRFS_SEND_C_OTIME
> - path BTRFS_SEND_A_PATH
> - timespec attribute BTRFS_SEND_A_OTIME
> - it's a separate command so it does not bloat any UTIMES2 commands,
>   and is emitted only after inode creation (file, dir, special files).
>
> The patch should be a template for further protocol extensions

Seems very neat and good to me.

>
> RFC:
>
> - set __BTRFS_SEND_C_MAX_V1 to the last command of the version or one
>   beyond?

Is there any advantage of one approach compared to the other?
I don't see any, and it seems good as it is now .

> - drop UTIMES2 before release?

Seems pointless.
Not only because of the dedicated otime command, issued only on inode
creation, but because the utimes command can actually
be issued several times when processing an inode, mostly due to
delayed renames and rmdir (and possibly other cases I don't remember
atm).
So not having otime in the utimes command helps reduce the stream size
and processing.

> - naming?

Seems fine to me.

>
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>  fs/btrfs/send.c            | 73 ++++++++++++++++++++++++++++++++++++--
>  fs/btrfs/send.h            | 11 +++++-
>  include/uapi/linux/btrfs.h | 12 +++++--
>  3 files changed, 91 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index afdcbe7844e0..ca9eba5f2de3 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -84,6 +84,8 @@ struct send_ctx {
>         u64 total_send_size;
>         u64 cmd_send_size[BTRFS_SEND_C_MAX + 1];
>         u64 flags;      /* 'flags' member of btrfs_ioctl_send_args is u64 */
> +       /* Protocol version compatibility requested */
> +       u32 proto;
>
>         struct btrfs_root *send_root;
>         struct btrfs_root *parent_root;
> @@ -312,6 +314,15 @@ static void inconsistent_snapshot_error(struct send_ctx *sctx,
>                    sctx->parent_root->root_key.objectid : 0));
>  }
>
> +static bool proto_cmd_ok(const struct send_ctx *sctx, int cmd)
> +{
> +       switch (sctx->proto) {
> +       case 1:  return cmd < __BTRFS_SEND_C_MAX_V1;
> +       case 2:  return cmd < __BTRFS_SEND_C_MAX_V2;
> +       default: return false;
> +       }
> +}
> +
>  static int is_waiting_for_move(struct send_ctx *sctx, u64 ino);
>
>  static struct waiting_dir_move *
> @@ -2507,6 +2518,7 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
>         struct extent_buffer *eb;
>         struct btrfs_key key;
>         int slot;
> +       int cmd;
>
>         btrfs_debug(fs_info, "send_utimes %llu", ino);
>
> @@ -2533,7 +2545,12 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
>         slot = path->slots[0];
>         ii = btrfs_item_ptr(eb, slot, struct btrfs_inode_item);
>
> -       ret = begin_cmd(sctx, BTRFS_SEND_C_UTIMES);
> +       if (proto_cmd_ok(sctx, BTRFS_SEND_C_UTIMES2))
> +               cmd = BTRFS_SEND_C_UTIMES2;
> +       else
> +               cmd = BTRFS_SEND_C_UTIMES;
> +
> +       ret = begin_cmd(sctx, cmd);
>         if (ret < 0)
>                 goto out;
>
> @@ -2544,7 +2561,8 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
>         TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_ATIME, eb, &ii->atime);
>         TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_MTIME, eb, &ii->mtime);
>         TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_CTIME, eb, &ii->ctime);
> -       /* TODO Add otime support when the otime patches get into upstream */
> +       if (proto_cmd_ok(sctx, BTRFS_SEND_C_UTIMES2))
> +               TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, eb, &ii->otime);
>
>         ret = send_cmd(sctx);
>
> @@ -2555,6 +2573,43 @@ static int send_utimes(struct send_ctx *sctx, u64 ino, u64 gen)
>         return ret;
>  }
>
> +static int send_inode_otime(struct send_ctx *sctx, const struct fs_path *fsp, u64 ino)
> +{
> +       int ret;
> +       struct btrfs_path *path;
> +       struct btrfs_inode_item *ii;
> +       struct btrfs_key key;
> +
> +       if (!proto_cmd_ok(sctx, BTRFS_SEND_C_OTIME))
> +               return 0;
> +
> +       path = alloc_path_for_send();
> +       if (!path)
> +               return -ENOMEM;
> +
> +       key.objectid = ino;
> +       key.type = BTRFS_INODE_ITEM_KEY;
> +       key.offset = 0;
> +       ret = btrfs_search_slot(NULL, sctx->send_root, &key, path, 0, 0);
> +       if (ret) {
> +               if (ret > 0)
> +                       ret = -ENOENT;
> +               goto out;
> +       }
> +
> +       ret = begin_cmd(sctx, BTRFS_SEND_C_OTIME);

Missing error handling here.

Thanks.

> +       ii = btrfs_item_ptr(path->nodes[0], path->slots[0], struct btrfs_inode_item);
> +       TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, fsp);
> +       TLV_PUT_BTRFS_TIMESPEC(sctx, BTRFS_SEND_A_OTIME, path->nodes[0], &ii->otime);
> +       ret = send_cmd(sctx);
> +
> +tlv_put_failure:
> +out:
> +       btrfs_free_path(path);
> +
> +       return ret;
> +}
> +
>  /*
>   * Sends a BTRFS_SEND_C_MKXXX or SYMLINK command to user space. We don't have
>   * a valid path yet because we did not process the refs yet. So, the inode
> @@ -2633,6 +2688,9 @@ static int send_create_inode(struct send_ctx *sctx, u64 ino)
>         if (ret < 0)
>                 goto out;
>
> +       ret = send_inode_otime(sctx, p, ino);
> +       if (ret < 0)
> +               goto out;
>
>  tlv_put_failure:
>  out:
> @@ -7269,6 +7327,17 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
>
>         sctx->flags = arg->flags;
>
> +       if (arg->flags & BTRFS_SEND_FLAG_VERSION) {
> +               if (arg->version > BTRFS_SEND_STREAM_VERSION) {
> +                       ret = -EPROTO;
> +                       goto out;
> +               }
> +               /* Zero means "use the highest version" */
> +               sctx->proto = arg->version ?: BTRFS_SEND_STREAM_VERSION;
> +       } else {
> +               sctx->proto = 1;
> +       }
> +
>         sctx->send_filp = fget(arg->send_fd);
>         if (!sctx->send_filp) {
>                 ret = -EBADF;
> diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
> index de91488b7cd0..eeafbe2fdd9f 100644
> --- a/fs/btrfs/send.h
> +++ b/fs/btrfs/send.h
> @@ -10,7 +10,7 @@
>  #include "ctree.h"
>
>  #define BTRFS_SEND_STREAM_MAGIC "btrfs-stream"
> -#define BTRFS_SEND_STREAM_VERSION 1
> +#define BTRFS_SEND_STREAM_VERSION 2
>
>  #define BTRFS_SEND_BUF_SIZE SZ_64K
>
> @@ -48,6 +48,7 @@ struct btrfs_tlv_header {
>  enum btrfs_send_cmd {
>         BTRFS_SEND_C_UNSPEC,
>
> +       /* Version 1 */
>         BTRFS_SEND_C_SUBVOL,
>         BTRFS_SEND_C_SNAPSHOT,
>
> @@ -76,6 +77,14 @@ enum btrfs_send_cmd {
>
>         BTRFS_SEND_C_END,
>         BTRFS_SEND_C_UPDATE_EXTENT,
> +       __BTRFS_SEND_C_MAX_V1,
> +
> +       /* Version 2 */
> +       BTRFS_SEND_C_UTIMES2,
> +       BTRFS_SEND_C_OTIME,
> +       __BTRFS_SEND_C_MAX_V2,
> +
> +       /* End */
>         __BTRFS_SEND_C_MAX,
>  };
>  #define BTRFS_SEND_C_MAX (__BTRFS_SEND_C_MAX - 1)
> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> index d7d3cfead056..c1a665d87f61 100644
> --- a/include/uapi/linux/btrfs.h
> +++ b/include/uapi/linux/btrfs.h
> @@ -771,10 +771,16 @@ struct btrfs_ioctl_received_subvol_args {
>   */
>  #define BTRFS_SEND_FLAG_OMIT_END_CMD           0x4
>
> +/*
> + * Read the protocol version in the structure
> + */
> +#define BTRFS_SEND_FLAG_VERSION                        0x8
> +
>  #define BTRFS_SEND_FLAG_MASK \
>         (BTRFS_SEND_FLAG_NO_FILE_DATA | \
>          BTRFS_SEND_FLAG_OMIT_STREAM_HEADER | \
> -        BTRFS_SEND_FLAG_OMIT_END_CMD)
> +        BTRFS_SEND_FLAG_OMIT_END_CMD | \
> +        BTRFS_SEND_FLAG_VERSION)
>
>  struct btrfs_ioctl_send_args {
>         __s64 send_fd;                  /* in */
> @@ -782,7 +788,9 @@ struct btrfs_ioctl_send_args {
>         __u64 __user *clone_sources;    /* in */
>         __u64 parent_root;              /* in */
>         __u64 flags;                    /* in */
> -       __u64 reserved[4];              /* in */
> +       __u32 version;                  /* in */
> +       __u32 reserved32;
> +       __u64 reserved[3];              /* in */
>  };
>
>  /*
> --
> 2.33.0
>


-- 
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”

Re: [PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[Omar Sandoval]

On Mon, Oct 18, 2021 at 04:41:09PM +0200, David Sterba wrote:
> This is send protocol update to version 2 with example new commands.
> 
> We have many pending protocol update requests but still don't have the
> basic protocol rev in place, the first thing that must happen is to do
> the actual versioning support. In order to have something to test,
> there's an extended and a new command, that should be otherwise harmless
> and nobody should depend on it. This should be enough to validate the
> non-protocol changes and backward compatibility before we do the big
> protocol update.
> 
> The protocol version is u32 and is a new member in the send ioctl
> struct. Validity of the version field is backed by a new flag bit. Old
> kernels would fail when a higher version is requested. Version protocol
> 0 will pick the highest supported version, BTRFS_SEND_STREAM_VERSION,
> that's also exported in sysfs.
> 
> Protocol changes:
> 
> - new command BTRFS_SEND_C_UTIMES2
> - appends OTIME after the output of BTRFS_SEND_C_UTIMES
> - this is an example how to extend an existing command based on protocol
>   version
> 
> - new command BTRFS_SEND_C_OTIME
> - path BTRFS_SEND_A_PATH
> - timespec attribute BTRFS_SEND_A_OTIME
> - it's a separate command so it does not bloat any UTIMES2 commands,
>   and is emitted only after inode creation (file, dir, special files).
> 
> The patch should be a template for further protocol extensions
> 
> RFC:
> 
> - set __BTRFS_SEND_C_MAX_V1 to the last command of the version or one
>   beyond?
> - drop UTIMES2 before release?
> - naming?

If I'm understanding correctly, the main difference between this send
stream v2 and mine is the BTRFS_SEND_FLAG_VERSION flag and
btrfs_ioctl_send_args::version rather than my BTRFS_SEND_FLAG_STREAM_V2?
That's definitely a better way to do it.

What's your plan for merging this? Did you want to do this as a "trial
run" before merging the compressed send/receive stuff as protocol v3, or
did you want me to integrate these changes?

Re: [PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[David Sterba]

On Mon, Oct 18, 2021 at 02:26:56PM -0700, Omar Sandoval wrote:
> On Mon, Oct 18, 2021 at 04:41:09PM +0200, David Sterba wrote:
> > - set __BTRFS_SEND_C_MAX_V1 to the last command of the version or one
> >   beyond?
> > - drop UTIMES2 before release?
> > - naming?
> 
> If I'm understanding correctly, the main difference between this send
> stream v2 and mine is the BTRFS_SEND_FLAG_VERSION flag and
> btrfs_ioctl_send_args::version rather than my BTRFS_SEND_FLAG_STREAM_V2?
> That's definitely a better way to do it.

Yesh, I think we should track an integer in it's own value, this would
allow us to do more updates in the future once new features appear.
The protocol version is level of stream command compatibility, while the
flags are for mode of operation (no data, encoded, ...).

> What's your plan for merging this? Did you want to do this as a "trial
> run" before merging the compressed send/receive stuff as protocol v3, or
> did you want me to integrate these changes?

The ioctl update and versioned protocol support code can be merged to
5.16 queue, as we all seem to agree. To have something material in the
protocol the otime can be there too, enough to test the whole usecase,
without risk of getting it wrong.

V3 and following can be a bigger update, with enough time for testing.
With the versioned protocol we can do (as the worst case) one version
per release but hopefully we'll get all the current pending commands
into one version.

Re: [PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[Nikolay Borisov]

On 18.10.21 г. 17:41, David Sterba wrote:
> This is send protocol update to version 2 with example new commands.
> 
> We have many pending protocol update requests but still don't have the
> basic protocol rev in place, the first thing that must happen is to do
> the actual versioning support. In order to have something to test,
> there's an extended and a new command, that should be otherwise harmless
> and nobody should depend on it. This should be enough to validate the
> non-protocol changes and backward compatibility before we do the big
> protocol update.
> 
> The protocol version is u32 and is a new member in the send ioctl
> struct. Validity of the version field is backed by a new flag bit. Old
> kernels would fail when a higher version is requested. Version protocol
> 0 will pick the highest supported version, BTRFS_SEND_STREAM_VERSION,
> that's also exported in sysfs.
> 

W.r.t to the bits which add the infrastructure to do versioned support
you have an ACK from me. In the final submission I'd like to see those
bits separate from the rest of the code which implements particular
protocol changes, once this lands then Omar can also base his code on
the added infrastructure.

Re: [PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[Omar Sandoval]

On Mon, Oct 18, 2021 at 04:41:09PM +0200, David Sterba wrote:
> This is send protocol update to version 2 with example new commands.
> 
> We have many pending protocol update requests but still don't have the
> basic protocol rev in place, the first thing that must happen is to do
> the actual versioning support. In order to have something to test,
> there's an extended and a new command, that should be otherwise harmless
> and nobody should depend on it. This should be enough to validate the
> non-protocol changes and backward compatibility before we do the big
> protocol update.
> 
> The protocol version is u32 and is a new member in the send ioctl
> struct. Validity of the version field is backed by a new flag bit. Old
> kernels would fail when a higher version is requested. Version protocol
> 0 will pick the highest supported version, BTRFS_SEND_STREAM_VERSION,
> that's also exported in sysfs.
> 
> Protocol changes:
> 
> - new command BTRFS_SEND_C_UTIMES2
> - appends OTIME after the output of BTRFS_SEND_C_UTIMES
> - this is an example how to extend an existing command based on protocol
>   version
> 
> - new command BTRFS_SEND_C_OTIME
> - path BTRFS_SEND_A_PATH
> - timespec attribute BTRFS_SEND_A_OTIME
> - it's a separate command so it does not bloat any UTIMES2 commands,
>   and is emitted only after inode creation (file, dir, special files).

Why do we need new commands for otime? I think it would make the most
sense to include the BTRFS_SEND_A_OTIME attribute with an existing
command: either the BTRFS_SEND_C_MK{DIR,FILE,NOD,FIFO,SOCK} command, or
the first BTRFS_SEND_C_UTIME command after creation. We might as well
take advantage of the TLV protocol, which allows us to add or remove
attributes for commands as needed.

Re: [PATCH RFC] btrfs: send: v2 protocol and example OTIME changes

[David Sterba]

On Tue, Oct 19, 2021 at 10:38:27AM -0700, Omar Sandoval wrote:
> On Mon, Oct 18, 2021 at 04:41:09PM +0200, David Sterba wrote:

> Why do we need new commands for otime? I think it would make the most
> sense to include the BTRFS_SEND_A_OTIME attribute with an existing
> command: either the BTRFS_SEND_C_MK{DIR,FILE,NOD,FIFO,SOCK} command, or
> the first BTRFS_SEND_C_UTIME command after creation. We might as well
> take advantage of the TLV protocol, which allows us to add or remove
> attributes for commands as needed.

Yes there are more ways to extend the protocol, the patch demonstrated
two, and partially it was for me to refresh how the bits are put
together.

As long as the changes are hidden behind the version, we can insert the
attributes to existing commands, like the MKFILE group, that's a nice
trick.

On the receiving side the raw protocol is translated to the callbacks.
I've decoupled the libbtrfs implementation from the internal one, so
we're now free to do any changes to the callback prototypes without
worries to break snapper. Which means either mapping new attributes to
parameters (where we can sensibly detect "n/a") or add new callbacks
with more parameters.

I've found out that eg. the mkfile sends the inode number but it's never
parsed in receive. So for one I'd like to audit such things and write a
machine readable spec of the protocol.