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

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

[Qu Wenruo]

The branch can be fetched from github:
https://github.com/adam900710/btrfs-progs/tree/send_dump

The branch doesn't contain the fake test case.

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.
v3:
  Add output for ctime/mtime/atime
  (Human readable local time like "1111-11-01 11:11:11")
  Rearrange the output from "key1: value1, key2: value2" to
  "key1=value1 key2=value2". Suggested by David
  Rename macro path_cat_or_error() to PATH_CAT_OR_RET(). Suggested by David
  Change pointer to array to avoid memory allocation error. Suggested by David
  Add the 5th patch to add a fake test case to show how the new output looks
  like.

Qu Wenruo (4):
  btrfs-progs: utils: Introduce function to escape characters
  btrfs-progs: introduce new send-dump object
  btrfs-progs: receive: introduce option to dump send stream
  btrfs-progs: remove send-test tool

 Documentation/btrfs-receive.asciidoc |  15 +-
 Makefile.in                          |   8 +-
 cmds-receive.c                       |  35 ++-
 send-dump.c                          | 299 +++++++++++++++++++++++
 send-dump.h                          |  29 +++
 send-test.c                          | 447 -----------------------------------
 utils.c                              |  17 ++
 utils.h                              |   2 +
 8 files changed, 394 insertions(+), 458 deletions(-)
 create mode 100644 send-dump.c
 create mode 100644 send-dump.h
 delete mode 100644 send-test.c

-- 
2.10.1

[PATCH v3 1/4] btrfs-progs: utils: Introduce function to escape characters

[Qu Wenruo]

Introduce new function, escape_string_inplace(), to escape specified
characters in place.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 utils.c | 17 +++++++++++++++++
 utils.h |  2 ++
 2 files changed, 19 insertions(+)

diff --git a/utils.c b/utils.c
index 3f54245..69faae1 100644
--- a/utils.c
+++ b/utils.c
@@ -4251,3 +4251,20 @@ unsigned int rand_range(unsigned int upper)
 	 */
 	return (unsigned int)(jrand48(rand_seed) % upper);
 }
+
+void string_escape_inplace(char *restrict string, char *restrict escape_chars)
+{
+	int i, j;
+
+	for (i = 0; i < strlen(escape_chars); i++) {
+		j = 0;
+		while (j < strlen(string)) {
+			if (string[j] == escape_chars[i]) {
+				memmove(string + j, string + j + 1,
+					strlen(string) -j);
+				continue;
+			}
+			j++;
+		}
+	}
+}
diff --git a/utils.h b/utils.h
index 1a2dbcd..b36d411 100644
--- a/utils.h
+++ b/utils.h
@@ -457,4 +457,6 @@ unsigned int rand_range(unsigned int upper);
 /* Also allow setting the seed manually */
 void init_rand_seed(u64 seed);
 
+void string_escape_inplace(char *restrict string, char *restrict escape_chars);
+
 #endif
-- 
2.10.1

[PATCH v3 2/4] 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>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 Makefile.in |   2 +-
 send-dump.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 send-dump.h |  29 ++++++
 3 files changed, 329 insertions(+), 1 deletion(-)
 create mode 100644 send-dump.c
 create mode 100644 send-dump.h

diff --git a/Makefile.in b/Makefile.in
index b53cf2c..c535c19 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -93,7 +93,7 @@ objects = ctree.o disk-io.o kernel-lib/radix-tree.o extent-tree.o print-tree.o \
 	  extent-cache.o extent_io.o volumes.o utils.o repair.o \
 	  qgroup.o raid56.o free-space-cache.o kernel-lib/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..b4d8a19
--- /dev/null
+++ b/send-dump.c
@@ -0,0 +1,299 @@
+/*
+ * 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 <time.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_OR_RET(function_name, outpath, path1, path2, ret)	\
+({									\
+	ret = path_cat_out(outpath, path1, path2);			\
+	if (ret < 0) {							\
+		error("%s: path invalid: %s\n", function_name, path2);	\
+		return ret;						\
+	}								\
+})
+
+#define TITLE_WIDTH	16
+#define PATH_WIDTH	32
+
+/*
+ * Underlying PRINT_DUMP, the only difference is how we handle
+ * the full path.
+ */
+static int __print_dump(int subvol, void *user, const char *path,
+			const char *title, const char *fmt, ...)
+{
+	struct btrfs_dump_send_args *r = user;
+	char real_title[TITLE_WIDTH + 1] = { 0 };
+	char full_path[PATH_MAX] = {0};
+	char *out_path;
+	va_list args;
+	int ret;
+
+	if (subvol) {
+		PATH_CAT_OR_RET(title, r->full_subvol_path, r->root_path, path, ret);
+		out_path = r->full_subvol_path;
+	} else {
+		PATH_CAT_OR_RET(title, full_path, r->full_subvol_path, path, ret);
+		out_path = full_path;
+	}
+	string_escape_inplace(out_path, " \n\t\\");
+
+	/* Append ':' to title */
+	strncpy(real_title, title, TITLE_WIDTH - 1);
+	strncat(real_title, ":", TITLE_WIDTH);
+
+	/* Unified header, */
+	printf("%-*s%-*s", TITLE_WIDTH, real_title, PATH_WIDTH, out_path);
+	va_start(args, fmt);
+	/* Operation specified ones */
+	vprintf(fmt, args);
+	va_end(args);
+	printf("\n");
+	return 0;
+}
+
+/* For subvolume/snapshot operation only */
+#define PRINT_DUMP_SUBVOL(user, path, title, fmt, ...) \
+	__print_dump(1, user, path, title, fmt, ##__VA_ARGS__)
+
+/* For other operations */
+#define PRINT_DUMP(user, path, title, fmt, ...) \
+	__print_dump(0, user, path, title, fmt, ##__VA_ARGS__)
+
+static int print_subvol(const char *path, const u8 *uuid, u64 ctransid,
+			void *user)
+{
+	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+
+	uuid_unparse(uuid, uuid_str);
+
+	return PRINT_DUMP_SUBVOL(user, path, "subvol", "uuid=%s transid=%llu",
+				 uuid_str, ctransid);
+}
+
+static int print_snapshot(const char *path, const u8 *uuid, u64 ctransid,
+			  const u8 *parent_uuid, u64 parent_ctransid,
+			  void *user)
+{
+	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+	char parent_uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+	int ret;
+
+	uuid_unparse(uuid, uuid_str);
+	uuid_unparse(parent_uuid, parent_uuid_str);
+
+	ret = PRINT_DUMP_SUBVOL(user, path, "snapshot",
+		"uuid=%s transid=%llu parent_uuid=%s parent_transid= %llu",
+				uuid_str, ctransid, parent_uuid_str,
+				parent_ctransid);
+	return ret;
+}
+
+static int print_mkfile(const char *path, void *user)
+{
+	return PRINT_DUMP(user, path, "mkfile", "");
+}
+
+static int print_mkdir(const char *path, void *user)
+{
+	return PRINT_DUMP(user, path, "mkdir", "");
+}
+
+static int print_mknod(const char *path, u64 mode, u64 dev, void *user)
+{
+	return PRINT_DUMP(user, path, "mknod", "mode=%llo dev=0x%llx", mode,
+			  dev);
+}
+
+static int print_mkfifo(const char *path, void *user)
+{
+	return PRINT_DUMP(user, path, "mkfifo", "");
+}
+
+static int print_mksock(const char *path, void *user)
+{
+	return PRINT_DUMP(user, path, "mksock", "");
+}
+
+static int print_symlink(const char *path, const char *lnk, void *user)
+{
+	return PRINT_DUMP(user, path, "symlink", "dest=%s", lnk);
+}
+
+static int print_rename(const char *from, const char *to, void *user)
+{
+	struct btrfs_dump_send_args *r = user;
+	char full_to[PATH_MAX];
+	int ret;
+
+	PATH_CAT_OR_RET("rename", full_to, r->full_subvol_path, to, ret);
+	return PRINT_DUMP(user, from, "rename", "dest=%s", full_to);
+}
+
+static int print_link(const char *path, const char *lnk, void *user)
+{
+	return PRINT_DUMP(user, path, "link", "dest=%s", lnk);
+}
+
+static int print_unlink(const char *path, void *user)
+{
+	return PRINT_DUMP(user, path, "unlink", "");
+}
+
+static int print_rmdir(const char *path, void *user)
+{
+	return PRINT_DUMP(user, path, "rmdir", "");
+}
+
+static int print_write(const char *path, const void *data, u64 offset,
+		       u64 len, void *user)
+{
+	return PRINT_DUMP(user, path, "write", "offset=%llu len=%llu",
+			  offset, len);
+}
+
+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_OR_RET("clone", full_path, r->full_subvol_path, clone_path,
+			ret);
+	return PRINT_DUMP(user, path, "clone",
+			  "offset=%llu len=%llu from=%s clone_offset=%llu",
+			  offset, len, full_path, clone_offset);
+}
+
+static int print_set_xattr(const char *path, const char *name,
+			   const void *data, int len, void *user)
+{
+	return PRINT_DUMP(user, path, "set_xattr", "name=%s, len=%d",
+			  name, len);
+}
+
+static int print_remove_xattr(const char *path, const char *name, void *user)
+{
+
+	return PRINT_DUMP(user, path, "remove_xattr", "name=%s", name);
+}
+
+static int print_truncate(const char *path, u64 size, void *user)
+{
+	return PRINT_DUMP(user, path, "truncate", "size=%llu", size);
+}
+
+static int print_chmod(const char *path, u64 mode, void *user)
+{
+	return PRINT_DUMP(user, path, "chmod", "mode=%llo", mode);
+}
+
+static int print_chown(const char *path, u64 uid, u64 gid, void *user)
+{
+	return PRINT_DUMP(user, path, "chown", "gid=%llu uid=%llu", gid, uid);
+}
+
+static int sprint_timespec(struct timespec *ts, char *restrict dest, int max_size)
+{
+	struct tm *tm;
+	int ret;
+
+	tm = localtime(&ts->tv_sec);
+	if (!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, "%Y-%m-%d %H:%M:%S", 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;
+}
+
+#define TIME_STRING_MAX	64
+static int print_utimes(const char *path, struct timespec *at,
+			struct timespec *mt, struct timespec *ct,
+			void *user)
+{
+	char at_str[TIME_STRING_MAX];
+	char mt_str[TIME_STRING_MAX];
+	char ct_str[TIME_STRING_MAX];
+
+	if (sprint_timespec(at, at_str, TIME_STRING_MAX - 1) < 0 ||
+	    sprint_timespec(mt, mt_str, TIME_STRING_MAX - 1) < 0 ||
+	    sprint_timespec(ct, ct_str, TIME_STRING_MAX - 1) < 0)
+		return -EINVAL;
+	return PRINT_DUMP(user, path, "utimes", "atime=%s mtime=%s ctime=%s",
+			  at_str, mt_str, ct_str);
+}
+
+static int print_update_extent(const char *path, u64 offset, u64 len,
+			       void *user)
+{
+	return PRINT_DUMP(user, path, "update_extent", "offset=%llu len=%llu",
+			  offset, len);
+}
+
+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..06a6108
--- /dev/null
+++ b/send-dump.h
@@ -0,0 +1,29 @@
+/*
+ * 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__
+
+#include <linux/limits.h>
+
+struct btrfs_dump_send_args {
+	char full_subvol_path[PATH_MAX];
+	char root_path[PATH_MAX];
+};
+
+extern struct btrfs_send_ops btrfs_print_send_ops;
+
+#endif
-- 
2.10.1

[PATCH v3 3/4] btrfs-progs: receive: introduce option to dump send stream

[Qu Wenruo]

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

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

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 Documentation/btrfs-receive.asciidoc | 15 ++++++++++++++-
 cmds-receive.c                       | 35 +++++++++++++++++++++++++++++++----
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/btrfs-receive.asciidoc b/Documentation/btrfs-receive.asciidoc
index e246603..f12e949 100644
--- a/Documentation/btrfs-receive.asciidoc
+++ b/Documentation/btrfs-receive.asciidoc
@@ -9,12 +9,19 @@ 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 do the validation of
+the stream, and print the stream metadata.
 
 *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::
+print the stream metadata
++
+Does not accept the '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 d0525bf..1dcdb1a 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;
 
@@ -1214,6 +1215,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));
@@ -1226,9 +1228,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 }
 		};
 
@@ -1265,6 +1269,9 @@ int cmd_receive(int argc, char **argv)
 				goto out;
 			}
 			break;
+		case GETOPT_VAL_DUMP:
+			dump = 1;
+			break;
 		case '?':
 		default:
 			error("receive args invalid");
@@ -1272,7 +1279,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];
@@ -1285,17 +1294,33 @@ 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[0] = '.';
+		dump_args.root_path[1] = '\0';
+		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>",
+	"btrfs receive [options] <mount>",
+	"or",
+	"btrfs receive --dump [options]",
 	"Receive subvolumes from stdin.",
 	"Receives one or more subvolumes that were previously",
 	"sent with btrfs send. The received subvolumes are stored",
@@ -1322,5 +1347,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.10.1

[PATCH v3 4/4] 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>
Signed-off-by: David Sterba <dsterba@suse.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 c535c19..9295d32 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -428,10 +428,6 @@ test-ioctl: ioctl-test ioctl-test-32 ioctl-test-64
 	$(Q)./ioctl-test-32 > ioctl-test-32.log
 	$(Q)./ioctl-test-64 > ioctl-test-64.log
 
-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
@@ -464,7 +460,7 @@ clean: $(CLEANDIRS)
 	@echo "Cleaning"
 	$(Q)$(RM) -f -- $(progs) cscope.out *.o *.o.d \
 		kernel-lib/*.o kernel-lib/*.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.10.1

[PATCH 5/5] btrfs-progs: misc-test: Add send stream dump test

[Qu Wenruo]

* PLEASE DON'T MERGE THIS PATCH*

With 2 send stream which contains all operations for user to check the
output.

This is just used for checking output format and find any possible
deflects or ugly layout.

It doesn't really checks any thing and doesn't follow normal test output
redirection.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 .../016-send-dump-output/creation.stream.xz        | Bin 0 -> 984 bytes
 .../016-send-dump-output/deletion.stream.xz        | Bin 0 -> 408 bytes
 tests/misc-tests/016-send-dump-output/test.sh      |  23 +++++++++++++++++++++
 3 files changed, 23 insertions(+)
 create mode 100644 tests/misc-tests/016-send-dump-output/creation.stream.xz
 create mode 100644 tests/misc-tests/016-send-dump-output/deletion.stream.xz
 create mode 100755 tests/misc-tests/016-send-dump-output/test.sh

diff --git a/tests/misc-tests/016-send-dump-output/creation.stream.xz b/tests/misc-tests/016-send-dump-output/creation.stream.xz
new file mode 100644
index 0000000000000000000000000000000000000000..521c5d97170e7ad41ac8b3305461df09e2cb4c86
GIT binary patch
literal 984
zcmV;}11J3bH+ooF000E$*0e?f03iVu0001VFXf})8bkw?T>vp13XGw}P}F=IbdLR(
z>U`@9XCIGr1@6OEPWlaQgT5hCS#G{B*Zx$SeQxx(I;AlUUp>USnJ^DpJDxOMlV57u
zelh2}f67+r{m^CB^YzYr3tjQ|k)GN6FCPlJzZl~%dVK~u^utMa*9rwMg9+N$g{SND
zcZ-NRsrbN=(OpirB^JtZ_@@|0ko&m=lrZK&kwT4$K~N%4wfsccFuJN7=oxM?(C{$t
zrJ3+}UkQ}_E*UQZWqzP7z377g)WK7w_xN6@JI=Ju(YvI$Mt_bx^7=ASE}(b8_B}4i
zasV`w=9w+*M6KB1PI0epqZRs16o(yJ_TJRHtLJs)H=r2l{lUm#HgtEO+R509mJ9Dp
zH53E2E>5wavINe=AaNq|VGT6MOCuv!R7O%jU@r%Yrdpfo?l6$2s-G=tmJ&#ZJK-XI
zc3_*HY85LsZo<$!ckHazSUr=utE9f?X_~CA>t03#bx~F<mvHfn1i6m>T8lXi$?P|(
zyUn-J`1NVoq>_vLA+8)in6aeaTk1B53fFNk)2uRv5OTLy;afVi?wl>cMS@Y<e`g+$
z)FDH?VwD`thACCobJJ`9&&4{|D;D*QKv^^C62Df^&_92eS=-x!-ep`F>{dRkH;AHw
zS^qftW|-zRq~4aB8Kg{kG*5vYWv*suPO<y5Q^;aW!(Yj=Hu&r#^TMCoPGHD*H)E^i
zM1Rfahee#ZtD&<OX=NhM5bA>OIx5<3%t>J~EcplJGDQ@?EpA?a!WZ^tuQB7Lid{7l
zWLuhRJZ3uiQ(-7c!+d4j2#bJ4GD4P%lv6G(cG-A7%R}d<d3ww6(SZAaI{&z*os!5`
z_dICt&b0qrjTKF#JwD6cZxb5#i<C&PRelC?HsCnm(QSt7J?u9h=3PGA%pOjpB?w&m
zaWP2tKuRNZWk^fRs(@XTvMM?`@u;I#xyaK6QpF_?HT5vCyi{f#&&QWKka&xKVzAKy
zKIczYjr<qtBza6d>${N+_2>{&W|9ejC}b5lc$A}M$5<VKgm@@_Ms(wEq2fml0muq*
zs@|Fzh`C>e(E4qn&EfR`1?U4<n!5gnJBcB8j<A2|T4yd*VW?{Qmirh113jpbc=_Xg
zBjrV$8hCJ}zK}x2`ldZ%Z&cdv6XkVg+P=RSz*ul-9_l=EEEw$=eR#*j$0g`BC+Byr
zV1;8DLJMNr19q7I^jAH0qrkub0002<S#w0x6j%%Z0kH?gGyni)n-apY#Ao{g00000
G1X)@v?&B%|

literal 0
HcmV?d00001

diff --git a/tests/misc-tests/016-send-dump-output/deletion.stream.xz b/tests/misc-tests/016-send-dump-output/deletion.stream.xz
new file mode 100644
index 0000000000000000000000000000000000000000..d04c0bbbdf1b5c8631d565ddd5ab88b654b9ad35
GIT binary patch
literal 408
zcmV;J0cZaGH+ooF000E$*0e?f03iVu0001VFXf})0+|6<T>vp13XGw}P}F=IbdLR(
z>U`@R#K~!;K6&PD9_8<Tm+X$wWFUg4g!Xb}hz}pqVOA+vFBLg$tLtyfgcU7SwZ;gJ
z#Vn}s)6N#00Av};DKO;W8$cbQ0S$_{rCv;a7e?J%*LA!l(#pIaz5ggmmk(u=u?v<d
z$-GvlqyAkeq#3pnOVEEL){E#%a2@LW4#}&`9iG!ttJW!X-6IzWiPh|Y;1dM^qOz!F
z^?Ep*e~M7yrqGBuY$?9-q*hE8{iK~2BgU;feTX`4io}b)o<$54{}3kuOQ}vn))YSD
zYnkhIV4U0N?b(W=l%Cb@_Mq`In_D@IYAq&~lA-m@Ys&NfwGkG|Fe7|n_=a1edxrHf
zFRd2#+}@y_1&v360k(wP?bAH#Ss8U`2+^E%>7`}Ysb&w5Tjy)cDTA~&eSS_FzE%Qx
z1=H-qw6D#$620}uFzMY^0002pjh8@1RLQ^q0rLWy1pojcFQxvm#Ao{g000001X)_e
CV8dwu

literal 0
HcmV?d00001

diff --git a/tests/misc-tests/016-send-dump-output/test.sh b/tests/misc-tests/016-send-dump-output/test.sh
new file mode 100755
index 0000000..cc8586c
--- /dev/null
+++ b/tests/misc-tests/016-send-dump-output/test.sh
@@ -0,0 +1,23 @@
+#!/bin/bash
+#
+# output receive --dump output for further tunning
+
+source $TOP/tests/common
+
+check_prereq btrfs
+
+# send streams are in *.stream.xz format, can't reuse check_all_images()
+
+rm *.stream -f
+
+for image in $(find . -iname '*.stream.xz'); do
+	xz --decompress --keep "$image" || \
+		_fail "failed to decompress $image" >&2
+	image=${image%%.xz}
+
+	echo "###### $image: ######"
+	# We call btrfs directly to output result without redirection
+	$TOP/btrfs receive --dump -f $image || \
+		_fail "failed to exam send stream"
+	rm -f $image
+done
-- 
2.10.1

Re: [PATCH v3 1/4] btrfs-progs: utils: Introduce function to escape characters

[David Sterba]

On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote:
> Introduce new function, escape_string_inplace(), to escape specified
> characters in place.

Sorry, the pointer to seq_path was misleading. The actual escape
function is mangle_path and it copies one string to another. As we just
print the path, we can simply switch and call putchar.

Re: [PATCH v3 2/4] btrfs-progs: introduce new send-dump object

[David Sterba]

On Tue, Nov 01, 2016 at 04:01:44PM +0800, Qu Wenruo wrote:
> +/*
> + * Underlying PRINT_DUMP, the only difference is how we handle
> + * the full path.
> + */
> +static int __print_dump(int subvol, void *user, const char *path,
> +			const char *title, const char *fmt, ...)

printf-like the __attribute__ ((format (printf, ...)))

> +{
> +	struct btrfs_dump_send_args *r = user;
> +	char real_title[TITLE_WIDTH + 1] = { 0 };
> +	char full_path[PATH_MAX] = {0};
> +	char *out_path;
> +	va_list args;
> +	int ret;
> +
> +	if (subvol) {
> +		PATH_CAT_OR_RET(title, r->full_subvol_path, r->root_path, path, ret);
> +		out_path = r->full_subvol_path;
> +	} else {
> +		PATH_CAT_OR_RET(title, full_path, r->full_subvol_path, path, ret);
> +		out_path = full_path;
> +	}
> +	string_escape_inplace(out_path, " \n\t\\");
> +
> +	/* Append ':' to title */
> +	strncpy(real_title, title, TITLE_WIDTH - 1);
> +	strncat(real_title, ":", TITLE_WIDTH);

I'd rather avoid such string operations, ':', just print everything.

> +
> +	/* Unified header, */
> +	printf("%-*s%-*s", TITLE_WIDTH, real_title, PATH_WIDTH, out_path);

PATH_WIDTH is used only here, please hardcode it into the format string.

The rest of the patch looks good. I think I've seen some artifacts in
the output, but we can tune this later.

> +	ret = strftime(dest, max_size, "%Y-%m-%d %H:%M:%S", tm);

We should use the RFC 3339 format, as it's standardized and also is a
string without whitespace.

Re: [PATCH v3 2/4] btrfs-progs: introduce new send-dump object

[Qu Wenruo]

At 11/01/2016 06:22 PM, David Sterba wrote:
> On Tue, Nov 01, 2016 at 04:01:44PM +0800, Qu Wenruo wrote:
>> +/*
>> + * Underlying PRINT_DUMP, the only difference is how we handle
>> + * the full path.
>> + */
>> +static int __print_dump(int subvol, void *user, const char *path,
>> +			const char *title, const char *fmt, ...)
>
> printf-like the __attribute__ ((format (printf, ...)))
>
>> +{
>> +	struct btrfs_dump_send_args *r = user;
>> +	char real_title[TITLE_WIDTH + 1] = { 0 };
>> +	char full_path[PATH_MAX] = {0};
>> +	char *out_path;
>> +	va_list args;
>> +	int ret;
>> +
>> +	if (subvol) {
>> +		PATH_CAT_OR_RET(title, r->full_subvol_path, r->root_path, path, ret);
>> +		out_path = r->full_subvol_path;
>> +	} else {
>> +		PATH_CAT_OR_RET(title, full_path, r->full_subvol_path, path, ret);
>> +		out_path = full_path;
>> +	}
>> +	string_escape_inplace(out_path, " \n\t\\");
>> +
>> +	/* Append ':' to title */
>> +	strncpy(real_title, title, TITLE_WIDTH - 1);
>> +	strncat(real_title, ":", TITLE_WIDTH);
>
> I'd rather avoid such string operations, ':', just print everything.

I'm completely OK to remove the ':'.

>
>> +
>> +	/* Unified header, */
>> +	printf("%-*s%-*s", TITLE_WIDTH, real_title, PATH_WIDTH, out_path);
>
> PATH_WIDTH is used only here, please hardcode it into the format string.
>
> The rest of the patch looks good. I think I've seen some artifacts in
> the output, but we can tune this later.

Would you like to share the artifacts so I can handle them in next update?

>
>> +	ret = strftime(dest, max_size, "%Y-%m-%d %H:%M:%S", tm);
>
> We should use the RFC 3339 format, as it's standardized and also is a
> string without whitespace.

Will use RFC 3339 in next version.

Thanks,
Qu

Re: [PATCH v3 1/4] btrfs-progs: utils: Introduce function to escape characters

[Qu Wenruo]

At 11/01/2016 06:08 PM, David Sterba wrote:
> On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote:
>> Introduce new function, escape_string_inplace(), to escape specified
>> characters in place.
>
> Sorry, the pointer to seq_path was misleading. The actual escape
> function is mangle_path and it copies one string to another. As we just
> print the path, we can simply switch and call putchar.
>

Putchar() method is indeed much easier to implement.

But it makes us hard to do further formatting, like aligning the path to 
given width. (At least we are still using 32 chars alignment for path)

So I still prefer the current full function string escaping and still 
use %-32s for formatting.


And the idea of implementing escape_string_inplace() as a pure string 
manipulation function can make it more agile for later use.
For example, we can reuse it for print-tree.

Although the current esacpe_string_inplace() can be further fine-tuned 
to avoid calling memmove() in a loop, which I can update it in next version.

Thanks,
Qu

Re: [PATCH v3 2/4] btrfs-progs: introduce new send-dump object

[David Sterba]

On Wed, Nov 02, 2016 at 08:37:20AM +0800, Qu Wenruo wrote:
> > PATH_WIDTH is used only here, please hardcode it into the format string.
> >
> > The rest of the patch looks good. I think I've seen some artifacts in
> > the output, but we can tune this later.
> 
> Would you like to share the artifacts so I can handle them in next update?

>From the output of misc test 016:

snapshot: [...] parent_transid= 14
                               ^ extra space

set_xattr:      ./snap_creation/xattr_file      name=user.test, len=1
                                                              ^ comma

also set xattr does not print the attribute data.

Re: [PATCH v3 1/4] btrfs-progs: utils: Introduce function to escape characters

[David Sterba]

On Wed, Nov 02, 2016 at 09:19:10AM +0800, Qu Wenruo wrote:
> 
> 
> At 11/01/2016 06:08 PM, David Sterba wrote:
> > On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote:
> >> Introduce new function, escape_string_inplace(), to escape specified
> >> characters in place.
> >
> > Sorry, the pointer to seq_path was misleading. The actual escape
> > function is mangle_path and it copies one string to another. As we just
> > print the path, we can simply switch and call putchar.
> >
> 
> Putchar() method is indeed much easier to implement.
> 
> But it makes us hard to do further formatting, like aligning the path to 
> given width. (At least we are still using 32 chars alignment for path)
> 
> So I still prefer the current full function string escaping and still 
> use %-32s for formatting.
> 
> 
> And the idea of implementing escape_string_inplace() as a pure string 
> manipulation function can make it more agile for later use.
> For example, we can reuse it for print-tree.

Reusing is fine, but I really don't like that the function modifies the
argument. What if the function is called twice on the same string? Also,
in the print-tree, this would mean the extent buffer would be modified,
potentially overwriting other items.

Re: [PATCH v3 4/4] btrfs-progs: remove send-test tool

[David Sterba]

On Tue, Nov 01, 2016 at 04:01:46PM +0800, Qu Wenruo wrote:
> 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>
> Signed-off-by: David Sterba <dsterba@suse.com>

As this patch is independet, applied now, no need to resend with the
rest.

Re: [PATCH v3 2/4] btrfs-progs: introduce new send-dump object

[Qu Wenruo]

At 11/02/2016 06:52 PM, David Sterba wrote:
> On Wed, Nov 02, 2016 at 08:37:20AM +0800, Qu Wenruo wrote:
>>> PATH_WIDTH is used only here, please hardcode it into the format string.
>>>
>>> The rest of the patch looks good. I think I've seen some artifacts in
>>> the output, but we can tune this later.
>>
>> Would you like to share the artifacts so I can handle them in next update?
>
> From the output of misc test 016:
>
> snapshot: [...] parent_transid= 14
>                                ^ extra space
>
> set_xattr:      ./snap_creation/xattr_file      name=user.test, len=1
>                                                               ^ comma
>
> also set xattr does not print the attribute data.

Thanks, I'll fix them.

Thanks,
Qu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

Re: [PATCH v3 1/4] btrfs-progs: utils: Introduce function to escape characters

[Qu Wenruo]

At 11/02/2016 06:55 PM, David Sterba wrote:
> On Wed, Nov 02, 2016 at 09:19:10AM +0800, Qu Wenruo wrote:
>>
>>
>> At 11/01/2016 06:08 PM, David Sterba wrote:
>>> On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote:
>>>> Introduce new function, escape_string_inplace(), to escape specified
>>>> characters in place.
>>>
>>> Sorry, the pointer to seq_path was misleading. The actual escape
>>> function is mangle_path and it copies one string to another. As we just
>>> print the path, we can simply switch and call putchar.
>>>
>>
>> Putchar() method is indeed much easier to implement.
>>
>> But it makes us hard to do further formatting, like aligning the path to
>> given width. (At least we are still using 32 chars alignment for path)
>>
>> So I still prefer the current full function string escaping and still
>> use %-32s for formatting.
>>
>>
>> And the idea of implementing escape_string_inplace() as a pure string
>> manipulation function can make it more agile for later use.
>> For example, we can reuse it for print-tree.
>
> Reusing is fine, but I really don't like that the function modifies the
> argument. What if the function is called twice on the same string? Also,
> in the print-tree, this would mean the extent buffer would be modified,
> potentially overwriting other items.
>
>
Then just encapsulate the in-place version into memory allocation version.

In-place version can be encapsulated very easily, while it's not 
possible vice verse.

So there will be two functions, escape_string_inplace() and 
escape_string() for caller to choose.

Would this be OK?

Thanks,
Qu

Re: [PATCH v3 1/4] btrfs-progs: utils: Introduce function to escape characters

[Qu Wenruo]

At 11/02/2016 06:55 PM, David Sterba wrote:
> On Wed, Nov 02, 2016 at 09:19:10AM +0800, Qu Wenruo wrote:
>>
>>
>> At 11/01/2016 06:08 PM, David Sterba wrote:
>>> On Tue, Nov 01, 2016 at 04:01:43PM +0800, Qu Wenruo wrote:
>>>> Introduce new function, escape_string_inplace(), to escape specified
>>>> characters in place.
>>>
>>> Sorry, the pointer to seq_path was misleading. The actual escape
>>> function is mangle_path and it copies one string to another. As we just
>>> print the path, we can simply switch and call putchar.
>>>
>>
>> Putchar() method is indeed much easier to implement.
>>
>> But it makes us hard to do further formatting, like aligning the path to
>> given width. (At least we are still using 32 chars alignment for path)
>>
>> So I still prefer the current full function string escaping and still
>> use %-32s for formatting.
>>
>>
>> And the idea of implementing escape_string_inplace() as a pure string
>> manipulation function can make it more agile for later use.
>> For example, we can reuse it for print-tree.
>
> Reusing is fine, but I really don't like that the function modifies the
> argument. What if the function is called twice on the same string?
Forgot to ask, what's the problem calling the function twice on the same 
string?

escape_string_inplace(dest, " ");
escape_string_inplace(dest, "\n");

is just the same as

escape_string_inplace(dest, " \n");

So I see no problem here.

Thanks,
Qu