[PATCH 0/3] Introduce dump option for btrfs-receive

[PATCH 0/3] Introduce dump option for btrfs-receive

[Qu Wenruo]

Introduce new "--dump" option for btrfs-receive, which will exam and dump
metadata info of a send stream.
This is quite handy to debug send stream.

Since such function is provided by old send-test tool, which doesn't even
compile now, remove the old send-test tool.

changelog:
v2:
  Move from inspect subcommand to receive subcommand.

Qu Wenruo (3):
  btrfs-progs: Introduce new send-dump object
  btrfs-progs: receive: Introduce option to exam and dump send stream
  btrfs-progs: Remove send-test tool

 Documentation/btrfs-receive.asciidoc |  17 +-
 Makefile.in                          |   8 +-
 cmds-receive.c                       |  41 +++-
 send-dump.c                          | 367 ++++++++++++++++++++++++++++
 send-dump.h                          |  24 ++
 send-test.c                          | 447 -----------------------------------
 6 files changed, 443 insertions(+), 461 deletions(-)
 create mode 100644 send-dump.c
 create mode 100644 send-dump.h
 delete mode 100644 send-test.c

-- 
2.9.3

[PATCH 1/3] btrfs-progs: Introduce new send-dump object

[Qu Wenruo]

Introduce send-dump.[ch] which implements a new btrfs_send_ops to
exam and output all operations inside a send stream.

It has a better output format than the old and no longer compilable
send-test tool, but still tries to be script friendly.

Provides the basis for later "inspect-internal dump-send" command.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 Makefile.in |   2 +-
 send-dump.c | 367 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 send-dump.h |  24 ++++
 3 files changed, 392 insertions(+), 1 deletion(-)
 create mode 100644 send-dump.c
 create mode 100644 send-dump.h

diff --git a/Makefile.in b/Makefile.in
index ac6b353..97caf95 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -80,7 +80,7 @@ objects = ctree.o disk-io.o radix-tree.o extent-tree.o print-tree.o \
 	  extent-cache.o extent_io.o volumes.o utils.o repair.o \
 	  qgroup.o raid6.o free-space-cache.o list_sort.o props.o \
 	  ulist.o qgroup-verify.o backref.o string-table.o task-utils.o \
-	  inode.o file.o find-root.o free-space-tree.o help.o
+	  inode.o file.o find-root.o free-space-tree.o help.o send-dump.o
 cmds_objects = cmds-subvolume.o cmds-filesystem.o cmds-device.o cmds-scrub.o \
 	       cmds-inspect.o cmds-balance.o cmds-send.o cmds-receive.o \
 	       cmds-quota.o cmds-qgroup.o cmds-replace.o cmds-check.o \
diff --git a/send-dump.c b/send-dump.c
new file mode 100644
index 0000000..bf451c7
--- /dev/null
+++ b/send-dump.c
@@ -0,0 +1,367 @@
+/*
+ * Copyright (C) 2016 Fujitsu.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program.
+ */
+
+#include <unistd.h>
+#include <stdint.h>
+#include <dirent.h>
+#include <pthread.h>
+#include <math.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/ioctl.h>
+#include <libgen.h>
+#include <mntent.h>
+#include <limits.h>
+#include <stdlib.h>
+#include <asm/types.h>
+#include <uuid/uuid.h>
+#include "utils.h"
+#include "commands.h"
+#include "send-utils.h"
+#include "send-stream.h"
+#include "send-dump.h"
+
+#define path_cat_out_with_error(function_name, out_path, path1, path2, ret) \
+ret = path_cat_out(out_path, path1, path2);			\
+if (ret < 0) {							\
+	error("%s: path invalid: %s\n", function_name, path2);	\
+	return ret;						\
+}
+
+#define TITLE_WIDTH	16
+#define PATH_WIDTH	32
+
+static void print_dump(const char *title, const char *path,
+	  	       const char *fmt, ...)
+{
+	va_list args;
+	char real_title[TITLE_WIDTH + 1];
+
+	real_title[0]='\0';
+	/* Append ':' to title*/
+	strncpy(real_title, title, TITLE_WIDTH - 1);
+	strncat(real_title, ":", TITLE_WIDTH);
+
+	/* Unified output */
+	printf("%-*s%-*s", TITLE_WIDTH, real_title, PATH_WIDTH, path);
+	va_start(args, fmt);
+	/* Command specified output */
+	vprintf(fmt, args);
+	va_end(args);
+	printf("\n");
+}
+
+static int print_subvol(const char *path, const u8 *uuid, u64 ctransid,
+			void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+	int ret;
+
+	path_cat_out_with_error("subvol", r->full_subvol_path, r->root_path,
+				path, ret);
+	uuid_unparse(uuid, uuid_str);
+
+	print_dump("subvol", r->full_subvol_path, "uuid: %s, transid: %llu",
+		   uuid_str, ctransid);
+	return 0;
+}
+
+static int print_snapshot(const char *path, const u8 *uuid, u64 ctransid,
+			  const u8 *parent_uuid, u64 parent_ctransid,
+			  void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+	char parent_uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+	int ret;
+
+	path_cat_out_with_error("snapshot", r->full_subvol_path, r->root_path,
+				path, ret);
+	uuid_unparse(uuid, uuid_str);
+	uuid_unparse(parent_uuid, parent_uuid_str);
+
+	print_dump("snapshot", r->full_subvol_path,
+		   "uuid: %s, transid: %llu, parent_uuid: %s, parent_transid: %llu",
+		   uuid_str, ctransid, parent_uuid_str, parent_ctransid);
+	return 0;
+}
+
+static int print_mkfile(const char *path, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("mkfile", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("mkfile", full_path, "");
+	return 0;
+}
+
+static int print_mkdir(const char *path, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("mkdir", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("mkdir", full_path, "");
+	return 0;
+}
+
+static int print_mknod(const char *path, u64 mode, u64 dev, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("mkdir", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("mknod", full_path, "mode: %llo, dev: 0x%llx", mode, dev);
+	return 0;
+}
+
+static int print_mkfifo(const char *path, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("mkfifo", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("mkfifo", full_path, "");
+	return 0;
+}
+
+static int print_mksock(const char *path, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("mksock", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("mksock", full_path, "");
+	return 0;
+}
+
+static int print_symlink(const char *path, const char *lnk, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("symlink", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("symlink", full_path, "lnk: %s", lnk);
+	return 0;
+}
+
+static int print_rename(const char *from, const char *to, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_from[PATH_MAX];
+	char full_to[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("rename", full_from, r->full_subvol_path, from,
+				ret);
+	path_cat_out_with_error("rename", full_to, r->full_subvol_path, to,
+				ret);
+	print_dump("rename", full_from, "to %s", full_to);
+	return 0;
+}
+
+static int print_link(const char *path, const char *lnk, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("link", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("link", full_path, "lnk: %s", lnk);
+	return 0;
+}
+
+static int print_unlink(const char *path, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("unlink", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("unlink", full_path, "");
+	return 0;
+}
+
+static int print_rmdir(const char *path, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("rmdir", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("rmdir", full_path, "");
+	return 0;
+}
+
+static int print_write(const char *path, const void *data, u64 offset,
+		       u64 len, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("write", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("write", full_path, "offset: %llu, len: %llu", offset, len);
+	return 0;
+}
+
+static int print_clone(const char *path, u64 offset, u64 len,
+		       const u8 *clone_uuid, u64 clone_ctransid,
+		       const char *clone_path, u64 clone_offset,
+		       void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("clone", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("clone", full_path, "offset: %llu, len: %llu from: %s, offset: %llu",
+		   offset, len, clone_path, clone_offset);
+	return 0;
+}
+
+static int print_set_xattr(const char *path, const char *name,
+			   const void *data, int len, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("set_xattr", full_path, r->full_subvol_path,
+				path, ret);
+	print_dump("set_xattr", full_path, "name: %s, len: %s", name, len);
+	return 0;
+}
+
+static int print_remove_xattr(const char *path, const char *name, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("remove_xattr", full_path, r->full_subvol_path,
+				path, ret);
+	print_dump("remove_xattr", full_path, name);
+	return 0;
+}
+
+static int print_truncate(const char *path, u64 size, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("truncate", full_path, r->full_subvol_path,
+				path, ret);
+	print_dump("truncate", full_path, "size: %llu", size);
+	return 0;
+}
+
+static int print_chmod(const char *path, u64 mode, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("chmod", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("chmod", full_path, "mode: %llo", mode);
+	return 0;
+}
+
+static int print_chown(const char *path, u64 uid, u64 gid, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("chown", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("chown", full_path, "gid: %llu, uid: %llu", gid, uid);
+	return 0;
+}
+
+static int print_utimes(const char *path, struct timespec *at,
+			struct timespec *mt, struct timespec *ct,
+			void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("utimes", full_path, r->full_subvol_path, path,
+				ret);
+	print_dump("utimes", full_path, "");
+	return 0;
+}
+
+static int print_update_extent(const char *path, u64 offset, u64 len,
+			       void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_path[PATH_MAX];
+	int ret;
+
+	path_cat_out_with_error("update_extent", full_path, r->full_subvol_path,
+				path, ret);
+	print_dump("update_extent", full_path, "offset: %llu, len: %llu",
+		   offset, len);
+	return 0;
+}
+
+struct btrfs_send_ops btrfs_print_send_ops = {
+	.subvol = print_subvol,
+	.snapshot = print_snapshot,
+	.mkfile = print_mkfile,
+	.mkdir = print_mkdir,
+	.mknod = print_mknod,
+	.mkfifo = print_mkfifo,
+	.mksock = print_mksock,
+	.symlink = print_symlink,
+	.rename = print_rename,
+	.link = print_link,
+	.unlink = print_unlink,
+	.rmdir = print_rmdir,
+	.write = print_write,
+	.clone = print_clone,
+	.set_xattr = print_set_xattr,
+	.remove_xattr = print_remove_xattr,
+	.truncate = print_truncate,
+	.chmod = print_chmod,
+	.chown = print_chown,
+	.utimes = print_utimes,
+	.update_extent = print_update_extent
+};
diff --git a/send-dump.h b/send-dump.h
new file mode 100644
index 0000000..48d821f
--- /dev/null
+++ b/send-dump.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2016 Fujitsu.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program.
+ */
+
+#ifndef __BTRFS_SEND_DUMP_H__
+#define __BTRFS_SEND_DUMP_H__
+struct btrfs_dump_send_args {
+	char *full_subvol_path;
+	char *root_path;
+};
+extern struct btrfs_send_ops btrfs_print_send_ops;
+#endif
-- 
2.9.3

[PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[Qu Wenruo]

Introduce new option, '--dump' for receive subcommand.

With this command, user can exam and dump the metadata info of a send
stream.
Which is quite useful for education purpose or bug reporting.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 Documentation/btrfs-receive.asciidoc | 17 +++++++++++++--
 cmds-receive.c                       | 41 ++++++++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/Documentation/btrfs-receive.asciidoc b/Documentation/btrfs-receive.asciidoc
index e246603..e4e3ba4 100644
--- a/Documentation/btrfs-receive.asciidoc
+++ b/Documentation/btrfs-receive.asciidoc
@@ -3,18 +3,25 @@ btrfs-receive(8)
 
 NAME
 ----
-btrfs-receive - receive subvolumes from send stream
+btrfs-receive - receive/exam subvolumes from send stream
 
 SYNOPSIS
 --------
 *btrfs receive* [options] <path>
 
+or
+
+*btrfs receive* --dump [options]
+
 DESCRIPTION
 -----------
 
 Receive a stream of changes and replicate one or more subvolumes that were
 previously used with *btrfs send* The received subvolumes are stored to
-'path'.
+'path', if '--dump' option is not given.
+
+If '--dump' option is given, *btrfs receive* will only exam the validation of
+the stream, and print the metadata info for debug usage.
 
 *btrfs receive* will fail int the following cases:
 
@@ -56,6 +63,12 @@ By default the mountpoint is searched in '/proc/self/mounts'.
 If you do not have '/proc', eg. in a chroot environment, use this option to tell
 us where this filesystem is mounted.
 
+--dump::
+exam the stream and print metadata info for debug/education purpose.
++
+Does not accept 'path' parameter. So with this option, *btrfs receive* won't
+modify your filesystem, and can be run by non-privileged users.
+
 EXIT STATUS
 -----------
 *btrfs receive* returns a zero exit status if it succeeds. Non zero is
diff --git a/cmds-receive.c b/cmds-receive.c
index f4a3a4f..32bf086 100644
--- a/cmds-receive.c
+++ b/cmds-receive.c
@@ -49,6 +49,7 @@
 #include "send.h"
 #include "send-stream.h"
 #include "send-utils.h"
+#include "send-dump.h"
 
 static int g_verbose = 0;
 
@@ -1194,6 +1195,7 @@ int cmd_receive(int argc, char **argv)
 	struct btrfs_receive r;
 	int receive_fd = fileno(stdin);
 	u64 max_errors = 1;
+	int dump = 0;
 	int ret = 0;
 
 	memset(&r, 0, sizeof(r));
@@ -1206,9 +1208,11 @@ int cmd_receive(int argc, char **argv)
 
 	while (1) {
 		int c;
+		enum { GETOPT_VAL_DUMP = 257 };
 		static const struct option long_opts[] = {
 			{ "max-errors", required_argument, NULL, 'E' },
 			{ "chroot", no_argument, NULL, 'C' },
+			{ "dump", no_argument, NULL, GETOPT_VAL_DUMP },
 			{ NULL, 0, NULL, 0 }
 		};
 
@@ -1245,6 +1249,9 @@ int cmd_receive(int argc, char **argv)
 				goto out;
 			}
 			break;
+		case GETOPT_VAL_DUMP:
+			dump = 1;
+			break;
 		case '?':
 		default:
 			error("receive args invalid");
@@ -1252,7 +1259,9 @@ int cmd_receive(int argc, char **argv)
 		}
 	}
 
-	if (check_argc_exact(argc - optind, 1))
+	if (dump && check_argc_exact(argc - optind, 0))
+		usage(cmd_receive_usage);
+	if (!dump && check_argc_exact(argc - optind, 1))
 		usage(cmd_receive_usage);
 
 	tomnt = argv[optind];
@@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
 		}
 	}
 
-	ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
+	if (dump) {
+		struct btrfs_dump_send_args dump_args;
+
+		dump_args.root_path = malloc(PATH_MAX);
+		dump_args.root_path[0] = '.';
+		dump_args.root_path[1] = '\0';
+		dump_args.full_subvol_path = malloc(PATH_MAX);
+		dump_args.full_subvol_path[0] = '.';
+		dump_args.full_subvol_path[1] = '\0';
+		ret = btrfs_read_and_process_send_stream(receive_fd,
+				&btrfs_print_send_ops, &dump_args, 0, 0);
+		if (ret < 0)
+			error("failed to dump the send stream: %s",
+			      strerror(-ret));
+	} else {
+		ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
+	}
+
 	if (receive_fd != fileno(stdin))
 		close(receive_fd);
-
 out:
 
 	return !!ret;
 }
 
 const char * const cmd_receive_usage[] = {
-	"btrfs receive [-ve] [-f <infile>] [--max-errors <N>] <mount>",
-	"Receive subvolumes from stdin.",
-	"Receives one or more subvolumes that were previously",
+	"btrfs receive [options] <mount>",
+	"or"
+	"btrfs receive --dump [options]",
+	"Receive/exam subvolumes from stdin.",
+	"Receives/exams one or more subvolumes that were previously",
 	"sent with btrfs send. The received subvolumes are stored",
 	"into <mount>.",
 	"btrfs receive will fail in case a receiving subvolume",
@@ -1302,5 +1329,7 @@ const char * const cmd_receive_usage[] = {
 	"-m <mountpoint>  The root mount point of the destination fs.",
 	"                 If you do not have /proc use this to tell us where ",
 	"                 this file system is mounted.",
+	"--dump           Exam and output metadata info of send stream.",
+	"                 Don't need <mount> parameter.",
 	NULL
 };
-- 
2.9.3

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[David Sterba]

On Wed, Sep 07, 2016 at 08:29:34AM +0800, Qu Wenruo wrote:
> @@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
>  		}
>  	}
>  
> -	ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
> +	if (dump) {
> +		struct btrfs_dump_send_args dump_args;
> +
> +		dump_args.root_path = malloc(PATH_MAX);
> +		dump_args.root_path[0] = '.';
> +		dump_args.root_path[1] = '\0';
> +		dump_args.full_subvol_path = malloc(PATH_MAX);

Please always check malloc return values. I'm fixing this for now.

> +		dump_args.full_subvol_path[0] = '.';
> +		dump_args.full_subvol_path[1] = '\0';
> +		ret = btrfs_read_and_process_send_stream(receive_fd,
> +				&btrfs_print_send_ops, &dump_args, 0, 0);
> +		if (ret < 0)
> +			error("failed to dump the send stream: %s",
> +			      strerror(-ret));
> +	} else {
> +		ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
> +	}
> +
>  	if (receive_fd != fileno(stdin))
>  		close(receive_fd);

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[David Sterba]

On Thu, Sep 08, 2016 at 11:42:29AM +0200, David Sterba wrote:
> On Wed, Sep 07, 2016 at 08:29:34AM +0800, Qu Wenruo wrote:
> > @@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
> >  		}
> >  	}
> >  
> > -	ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
> > +	if (dump) {
> > +		struct btrfs_dump_send_args dump_args;
> > +
> > +		dump_args.root_path = malloc(PATH_MAX);
> > +		dump_args.root_path[0] = '.';
> > +		dump_args.root_path[1] = '\0';
> > +		dump_args.full_subvol_path = malloc(PATH_MAX);
> 
> Please always check malloc return values. I'm fixing this for now.

Uh and the buffers are not freed either. Anyway, I'm switching it to an
array, there's no reason to allocate the memory dynamically.

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[Qu Wenruo]

Hi David,

Any updates?

If you're busy on these fix, I could fix these problem and send a new 
version.

Thanks,
Qu

At 09/08/2016 05:56 PM, David Sterba wrote:
> On Thu, Sep 08, 2016 at 11:42:29AM +0200, David Sterba wrote:
>> On Wed, Sep 07, 2016 at 08:29:34AM +0800, Qu Wenruo wrote:
>>> @@ -1265,19 +1274,37 @@ int cmd_receive(int argc, char **argv)
>>>  		}
>>>  	}
>>>
>>> -	ret = do_receive(&r, tomnt, realmnt, receive_fd, max_errors);
>>> +	if (dump) {
>>> +		struct btrfs_dump_send_args dump_args;
>>> +
>>> +		dump_args.root_path = malloc(PATH_MAX);
>>> +		dump_args.root_path[0] = '.';
>>> +		dump_args.root_path[1] = '\0';
>>> +		dump_args.full_subvol_path = malloc(PATH_MAX);
>>
>> Please always check malloc return values. I'm fixing this for now.
>
> Uh and the buffers are not freed either. Anyway, I'm switching it to an
> array, there's no reason to allocate the memory dynamically.
>
>

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[David Sterba]

On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote:
> Hi David,
> 
> Any updates?
> 
> If you're busy on these fix, I could fix these problem and send a new 
> version.

I was about to add the series to merge but the output format needs work.
At minimum, something that's parseable, so some commonly used format
(yaml or json). The current way is ok for quick overview, but I'd really
like to see more options of the output. The malloc/free problems are
minor.

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[Qu Wenruo]

At 10/18/2016 08:07 PM, David Sterba wrote:
> On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote:
>> Hi David,
>>
>> Any updates?
>>
>> If you're busy on these fix, I could fix these problem and send a new
>> version.
>
> I was about to add the series to merge but the output format needs work.
> At minimum, something that's parseable, so some commonly used format
> (yaml or json). The current way is ok for quick overview, but I'd really
> like to see more options of the output. The malloc/free problems are
> minor.
>

Thanks for the comment.

It makes sense. And it can be the first step to make all btrfs commands 
output parseable.

I could try to use json-c as our json library to add json output mode 
for send dump.

Meanwhile the original design for send-dump is only for human, and the 
json output support will be a huge work(not only for send-dump, but 
quite a lot of other sub-commands).

What about merge send dump first, and then add json support step-by-step?

Thanks,
Qu

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[David Sterba]

On Wed, Oct 19, 2016 at 09:18:09AM +0800, Qu Wenruo wrote:
> At 10/18/2016 08:07 PM, David Sterba wrote:
> > On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote:
> >> Hi David,
> >>
> >> Any updates?
> >>
> >> If you're busy on these fix, I could fix these problem and send a new
> >> version.
> >
> > I was about to add the series to merge but the output format needs work.
> > At minimum, something that's parseable, so some commonly used format
> > (yaml or json). The current way is ok for quick overview, but I'd really
> > like to see more options of the output. The malloc/free problems are
> > minor.
> 
> Thanks for the comment.
> 
> It makes sense. And it can be the first step to make all btrfs commands 
> output parseable.
> 
> I could try to use json-c as our json library to add json output mode 
> for send dump.
> 
> Meanwhile the original design for send-dump is only for human, and the 
> json output support will be a huge work(not only for send-dump, but 
> quite a lot of other sub-commands).
> 
> What about merge send dump first, and then add json support step-by-step?

Ok, so let's do it in another way.

The output format:
* one line per stream operation
* use key=value for all operation data
* file paths are unquoted, but escaped (same what seq_escape(" \t\n\\") does)
* two options where to put the filename, first or last, let's keep it
  first for now, we can move it later on,
  for multiple paths per operation, one of them will be just anoter
  key=value with descriptive key name

Code changes:
* fold my two patches from branch dev/send-dump-wip
* the common block at the beginniing of each callback can be factored
  into a macro (ie. full_path, ret, and path_cat with the name of the
  operation)

We'll also need some stream samples that trigger all operations so we
can tune the output.

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[Qu Wenruo]

At 11/01/2016 01:31 AM, David Sterba wrote:
> On Wed, Oct 19, 2016 at 09:18:09AM +0800, Qu Wenruo wrote:
>> At 10/18/2016 08:07 PM, David Sterba wrote:
>>> On Thu, Oct 13, 2016 at 03:32:53PM +0800, Qu Wenruo wrote:
>>>> Hi David,
>>>>
>>>> Any updates?
>>>>
>>>> If you're busy on these fix, I could fix these problem and send a new
>>>> version.
>>>
>>> I was about to add the series to merge but the output format needs work.
>>> At minimum, something that's parseable, so some commonly used format
>>> (yaml or json). The current way is ok for quick overview, but I'd really
>>> like to see more options of the output. The malloc/free problems are
>>> minor.
>>
>> Thanks for the comment.
>>
>> It makes sense. And it can be the first step to make all btrfs commands
>> output parseable.
>>
>> I could try to use json-c as our json library to add json output mode
>> for send dump.
>>
>> Meanwhile the original design for send-dump is only for human, and the
>> json output support will be a huge work(not only for send-dump, but
>> quite a lot of other sub-commands).
>>
>> What about merge send dump first, and then add json support step-by-step?
>
> Ok, so let's do it in another way.
>
> The output format:
> * one line per stream operation
> * use key=value for all operation data
> * file paths are unquoted, but escaped (same what seq_escape(" \t\n\\") does)
> * two options where to put the filename, first or last, let's keep it
>   first for now, we can move it later on,
>   for multiple paths per operation, one of them will be just anoter
>   key=value with descriptive key name
>
> Code changes:
> * fold my two patches from branch dev/send-dump-wip
> * the common block at the beginniing of each callback can be factored
>   into a macro (ie. full_path, ret, and path_cat with the name of the
>   operation)
>
> We'll also need some stream samples that trigger all operations so we
> can tune the output.

This sounds quite good to me.

I'll update the patchset.

I'm just a little curious about the stream samples.
Should I create a send-dump-test to include all these samples? Or put 
them somewhere else?

Thanks,
Qu

Re: [PATCH v2 2/3] btrfs-progs: receive: Introduce option to exam and dump send stream

[David Sterba]

On Tue, Nov 01, 2016 at 09:10:09AM +0800, Qu Wenruo wrote:
> I'm just a little curious about the stream samples.
> Should I create a send-dump-test to include all these samples? Or put 
> them somewhere else?

Misc tests would be fine for now, as you did in the new patchset.

[PATCH 3/3] btrfs-progs: Remove send-test tool

[Qu Wenruo]

Since new "receive --dump" has better output and structure, it's time
to remove old and function-weak send-test tool.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 Makefile.in |   6 +-
 send-test.c | 447 ------------------------------------------------------------
 2 files changed, 1 insertion(+), 452 deletions(-)
 delete mode 100644 send-test.c

diff --git a/Makefile.in b/Makefile.in
index 97caf95..5ae860d 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -346,10 +346,6 @@ ioctl-test: $(objects) $(libs) ioctl-test.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o ioctl-test $(objects) $(libs) ioctl-test.o $(LDFLAGS) $(LIBS)
 
-send-test: $(objects) $(libs) send-test.o
-	@echo "    [LD]     $@"
-	$(Q)$(CC) $(CFLAGS) -o send-test $(objects) $(libs) send-test.o $(LDFLAGS) $(LIBS)
-
 library-test: $(libs_shared) library-test.o
 	@echo "    [LD]     $@"
 	$(Q)$(CC) $(CFLAGS) -o library-test library-test.o $(LDFLAGS) -lbtrfs
@@ -381,7 +377,7 @@ clean-all: clean clean-doc clean-gen
 clean: $(CLEANDIRS)
 	@echo "Cleaning"
 	$(Q)$(RM) -f $(progs) cscope.out *.o *.o.d \
-	      dir-test ioctl-test quick-test send-test library-test library-test-static \
+	      dir-test ioctl-test quick-test library-test library-test-static \
 	      btrfs.static mkfs.btrfs.static \
 	      $(check_defs) \
 	      $(libs) $(lib_links) \
diff --git a/send-test.c b/send-test.c
deleted file mode 100644
index 4645b89..0000000
--- a/send-test.c
+++ /dev/null
@@ -1,447 +0,0 @@
-/*
- * Copyright (C) 2013 SUSE.  All rights reserved.
- *
- * This code is adapted from cmds-send.c and cmds-receive.c,
- * Both of which are:
- *
- * Copyright (C) 2012 Alexander Block.  All rights reserved.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public
- * License v2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public
- * License along with this program; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 021110-1307, USA.
- */
-
-#include <unistd.h>
-#include <stdint.h>
-#include <dirent.h>
-#include <pthread.h>
-#include <math.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <sys/ioctl.h>
-#include <libgen.h>
-#include <mntent.h>
-#include <limits.h>
-#include <stdlib.h>
-#include <asm/types.h>
-#include <uuid/uuid.h>
-
-/*
- * This should be compilable without the rest of the btrfs-progs
- * source distribution.
- */
-#if BTRFS_FLAT_INCLUDES
-#include "send-utils.h"
-#include "send-stream.h"
-#else
-#include <btrfs/send-utils.h>
-#include <btrfs/send-stream.h>
-#endif /* BTRFS_FLAT_INCLUDES */
-
-static int pipefd[2];
-struct btrfs_ioctl_send_args io_send = {0, };
-static char *subvol_path;
-static char *root_path;
-
-struct recv_args {
-	char *full_subvol_path;
-	char *root_path;
-};
-
-void usage(int error)
-{
-	printf("send-test <btrfs root> <subvol>\n");
-	if (error)
-		exit(error);
-}
-
-static int print_subvol(const char *path, const u8 *uuid, u64 ctransid,
-			void *user)
-{
-	struct recv_args *r = user;
-	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
-
-	r->full_subvol_path = path_cat(r->root_path, path);
-	uuid_unparse(uuid, uuid_str);
-
-	printf("subvol\t%s\t%llu\t%s\n", uuid_str,
-	       (unsigned long long)ctransid, r->full_subvol_path);
-
-	return 0;
-}
-
-static int print_snapshot(const char *path, const u8 *uuid, u64 ctransid,
-			  const u8 *parent_uuid, u64 parent_ctransid,
-			  void *user)
-{
-	struct recv_args *r = user;
-	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
-	char parent_uuid_str[BTRFS_UUID_UNPARSED_SIZE];
-
-	r->full_subvol_path = path_cat(r->root_path, path);
-	uuid_unparse(uuid, uuid_str);
-	uuid_unparse(parent_uuid, parent_uuid_str);
-
-	printf("snapshot\t%s\t%llu\t%s\t%llu\t%s\n", uuid_str,
-	       (unsigned long long)ctransid, parent_uuid_str,
-	       (unsigned long long)parent_ctransid, r->full_subvol_path);
-
-	return 0;
-}
-
-static int print_mkfile(const char *path, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("mkfile\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_mkdir(const char *path, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("mkdir\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_mknod(const char *path, u64 mode, u64 dev, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("mknod\t%llo\t0x%llx\t%s\n", (unsigned long long)mode,
-	       (unsigned long long)dev, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_mkfifo(const char *path, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("mkfifo\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_mksock(const char *path, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("mksock\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_symlink(const char *path, const char *lnk, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("symlink\t%s\t%s\n", lnk, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_rename(const char *from, const char *to, void *user)
-{
-	struct recv_args *r = user;
-	char *full_from = path_cat(r->full_subvol_path, from);
-	char *full_to = path_cat(r->full_subvol_path, to);
-
-	printf("rename\t%s\t%s\n", from, to);
-
-	free(full_from);
-	free(full_to);
-	return 0;
-}
-
-static int print_link(const char *path, const char *lnk, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("link\t%s\t%s\n", lnk, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_unlink(const char *path, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("unlink\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_rmdir(const char *path, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("rmdir\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_write(const char *path, const void *data, u64 offset,
-		       u64 len, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("write\t%llu\t%llu\t%s\n", (unsigned long long)offset,
-	       (unsigned long long)len, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_clone(const char *path, u64 offset, u64 len,
-		       const u8 *clone_uuid, u64 clone_ctransid,
-		       const char *clone_path, u64 clone_offset,
-		       void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("clone\t%s\t%s\n", full_path, clone_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_set_xattr(const char *path, const char *name,
-			   const void *data, int len, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("set_xattr\t%s\t%s\t%d\n", full_path,
-	       name, len);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_remove_xattr(const char *path, const char *name, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("remove_xattr\t%s\t%s\n", full_path, name);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_truncate(const char *path, u64 size, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("truncate\t%llu\t%s\n", (unsigned long long)size, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_chmod(const char *path, u64 mode, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("chmod\t%llo\t%s\n", (unsigned long long)mode, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_chown(const char *path, u64 uid, u64 gid, void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("chown\t%llu\t%llu\t%s\n", (unsigned long long)uid,
-	       (unsigned long long)gid, full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_utimes(const char *path, struct timespec *at,
-			struct timespec *mt, struct timespec *ct,
-			void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("utimes\t%s\n", full_path);
-
-	free(full_path);
-	return 0;
-}
-
-static int print_update_extent(const char *path, u64 offset, u64 len,
-			       void *user)
-{
-	struct recv_args *r = user;
-	char *full_path = path_cat(r->full_subvol_path, path);
-
-	printf("update_extent\t%s\t%llu\t%llu\n", full_path, offset, len);
-
-	free(full_path);
-	return 0;
-}
-
-static struct btrfs_send_ops send_ops_print = {
-	.subvol = print_subvol,
-	.snapshot = print_snapshot,
-	.mkfile = print_mkfile,
-	.mkdir = print_mkdir,
-	.mknod = print_mknod,
-	.mkfifo = print_mkfifo,
-	.mksock = print_mksock,
-	.symlink = print_symlink,
-	.rename = print_rename,
-	.link = print_link,
-	.unlink = print_unlink,
-	.rmdir = print_rmdir,
-	.write = print_write,
-	.clone = print_clone,
-	.set_xattr = print_set_xattr,
-	.remove_xattr = print_remove_xattr,
-	.truncate = print_truncate,
-	.chmod = print_chmod,
-	.chown = print_chown,
-	.utimes = print_utimes,
-	.update_extent = print_update_extent,
-};
-
-static void *process_thread(void *arg_)
-{
-	int ret;
-
-	while (1) {
-		ret = btrfs_read_and_process_send_stream(pipefd[0],
-							 &send_ops_print, arg_, 0);
-		if (ret)
-			break;
-	}
-
-	if (ret > 0)
-		ret = 0;
-
-	return ERR_PTR(ret);
-}
-
-int main(int argc, char **argv)
-{
-	int ret = 0;
-	int subvol_fd;
-	pthread_t t_read;
-	void *t_err = NULL;
-	struct recv_args r;
-
-	if (argc != 3)
-		usage(EINVAL);
-
-	root_path = realpath(argv[1], NULL);
-	if (!root_path) {
-		ret = errno;
-		usage(ret);
-	}
-
-	subvol_path = realpath(argv[2], NULL);
-	if (!subvol_path) {
-		ret = errno;
-		usage(ret);
-	}
-
-	r.full_subvol_path = subvol_path;
-	r.root_path = root_path;
-
-	subvol_fd = open(subvol_path, O_RDONLY|O_NOATIME);
-	if (subvol_fd < 0) {
-		ret = errno;
-		fprintf(stderr, "ERROR: Subvolume open failed. %s\n",
-			strerror(ret));
-		goto out;
-	}
-
-	ret = pipe(pipefd);
-	if (ret < 0) {
-		ret = errno;
-		fprintf(stderr, "ERROR: pipe failed. %s\n", strerror(ret));
-		goto out;
-	}
-
-	ret = pthread_create(&t_read, NULL, process_thread, &r);
-	if (ret < 0) {
-		ret = errno;
-		fprintf(stderr, "ERROR: pthread create failed. %s\n",
-			strerror(ret));
-		goto out;
-	}
-
-	io_send.send_fd = pipefd[1];
-	io_send.clone_sources_count = 0;
-	io_send.clone_sources = NULL;
-	io_send.parent_root = 0;
-	io_send.flags = BTRFS_SEND_FLAG_NO_FILE_DATA;
-
-	ret = ioctl(subvol_fd, BTRFS_IOC_SEND, &io_send);
-	if (ret < 0) {
-		ret = errno;
-		fprintf(stderr, "ERROR: send ioctl failed with %d: %s\n", ret,
-			strerror(ret));
-		goto out;
-	}
-
-	close(pipefd[1]);
-
-	ret = pthread_join(t_read, &t_err);
-	if (ret) {
-		fprintf(stderr, "ERROR: pthread_join failed: %s\n",
-			strerror(ret));
-		goto out;
-	}
-	if (t_err) {
-		ret = (long int)t_err;
-		fprintf(stderr, "ERROR: failed to process send stream, ret=%ld "
-			"(%s)\n", (long int)t_err, strerror(ret));
-		goto out;
-	}
-
-out:
-	return !!ret;
-}
-- 
2.9.3