[PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Qu Wenruo]

This patchset will introduce offline fsid/chunk tree uuid change
function to btrfstune command, with "-u" and "-U" options respectively.

The implement is in-place btrfs_header modification, without transaction
protection. So the uuid change can be done on any valid image even it is
already full.

Since we don't use transaction to protect the convert, it's possible one
can stop the running convert progress, causing the fs in a inconsistent
status and unable to be mounted.
To avoid such problem, we introduce new open ctree flags
OPEN_CTREE_IGNORE_FSID, allowing btrfstune to finish the convert.

Qu Wenruo (8):
  btrfs-progs: Allow open_ctree to ignore fsid mismatch.
  btrfs-progs: Export write_tree_block().
  btrfs-progs: Introduce change_header_uuid() function.
  btrfs-progs: Introduce change_extents_uuid() function.
  btrfs-progs: Introduce function change_device_uuid().
  btrfs-progs: Introduce change_devices_uuid() function.
  btrfs-progs: Introduce change_uuid() function.
  btrfs-progs: btrfstune: Introduce new "-u" and "-U" options.

 Documentation/btrfstune.asciidoc |  12 +-
 btrfstune.c                      | 288 ++++++++++++++++++++++++++++++++++++++-
 ctree.h                          |   3 +
 disk-io.c                        |   9 +-
 disk-io.h                        |   6 +
 5 files changed, 309 insertions(+), 9 deletions(-)

-- 
2.3.7

[PATCH 1/8] btrfs-progs: Allow open_ctree to ignore fsid mismatch.

[Qu Wenruo]

This feature is used for later UUID change function.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 ctree.h   | 1 +
 disk-io.c | 5 ++++-
 disk-io.h | 3 +++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ctree.h b/ctree.h
index 10dc838..45fef3d 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1010,6 +1010,7 @@ struct btrfs_fs_info {
 	unsigned int is_chunk_recover:1;
 	unsigned int quota_enabled:1;
 	unsigned int suppress_check_block_errors:1;
+	unsigned int ignore_fsid:1;
 
 	int (*free_extent_hook)(struct btrfs_trans_handle *trans,
 				struct btrfs_root *root,
diff --git a/disk-io.c b/disk-io.c
index c1cf146..8a91345 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -49,7 +49,8 @@ static int check_tree_block(struct btrfs_root *root, struct extent_buffer *buf)
 
 	fs_devices = root->fs_info->fs_devices;
 	while (fs_devices) {
-		if (!memcmp_extent_buffer(buf, fs_devices->fsid,
+		if (root->fs_info->ignore_fsid ||
+		    !memcmp_extent_buffer(buf, fs_devices->fsid,
 					  btrfs_header_fsid(),
 					  BTRFS_FSID_SIZE)) {
 			ret = 0;
@@ -1149,6 +1150,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 		fs_info->on_restoring = 1;
 	if (flags & OPEN_CTREE_SUPPRESS_CHECK_BLOCK_ERRORS)
 		fs_info->suppress_check_block_errors = 1;
+	if (flags & OPEN_CTREE_IGNORE_FSID)
+		fs_info->ignore_fsid = 1;
 
 	ret = btrfs_scan_fs_devices(fp, path, &fs_devices, sb_bytenr,
 				    (flags & OPEN_CTREE_RECOVER_SUPER),
diff --git a/disk-io.h b/disk-io.h
index 4caebeb..83cbe47 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -49,6 +49,9 @@ enum btrfs_open_ctree_flags {
 	 * tree bits.
 	 * Like split PARTIAL into SKIP_CSUM/SKIP_EXTENT
 	 */
+
+	/* Currently, no codes check chunk_tree_uuid, so only ignore fsid */
+	OPEN_CTREE_IGNORE_FSID		= (1 << 10)
 };
 
 static inline u64 btrfs_sb_offset(int mirror)
-- 
2.3.7

[PATCH 2/8] btrfs-progs: Export write_tree_block().

[Qu Wenruo]

Export wirte_tree_block() function and allow it write extent without
transaction.

This provides the basis for later uuid change function.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 disk-io.c | 4 ++--
 disk-io.h | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index 8a91345..4f3d1de 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -372,7 +372,7 @@ int write_and_map_eb(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
-static int write_tree_block(struct btrfs_trans_handle *trans,
+int write_tree_block(struct btrfs_trans_handle *trans,
 		     struct btrfs_root *root,
 		     struct extent_buffer *eb)
 {
@@ -381,7 +381,7 @@ static int write_tree_block(struct btrfs_trans_handle *trans,
 		BUG();
 	}
 
-	if (!btrfs_buffer_uptodate(eb, trans->transid))
+	if (trans && !btrfs_buffer_uptodate(eb, trans->transid))
 		BUG();
 
 	btrfs_set_header_flag(eb, BTRFS_HEADER_FLAG_WRITTEN);
diff --git a/disk-io.h b/disk-io.h
index 83cbe47..dd5bb8a 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -127,6 +127,9 @@ int csum_tree_block_size(struct extent_buffer *buf, u16 csum_sectorsize,
 			 int verify);
 int verify_tree_block_csum_silent(struct extent_buffer *buf, u16 csum_size);
 int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid);
+int write_tree_block(struct btrfs_trans_handle *trans,
+		     struct btrfs_root *root,
+		     struct extent_buffer *eb);
 int write_and_map_eb(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		     struct extent_buffer *eb);
 
-- 
2.3.7

[PATCH 3/8] btrfs-progs: Introduce change_header_uuid() function.

[Qu Wenruo]

This function is used to change fsid/chunk_tree_uuid of a node/leaf.
The function does it without transaction protect.

This is the basis of offline uuid change.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfstune.c | 31 +++++++++++++++++++++++++++++++
 ctree.h     |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/btrfstune.c b/btrfstune.c
index 808466f..f039d84 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -98,6 +98,37 @@ static int enable_skinny_metadata(struct btrfs_root *root)
 	return 0;
 }
 
+static int change_header_uuid(struct btrfs_root *root, struct extent_buffer *eb)
+{
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	int same_fsid = 1;
+	int same_chunk_tree_uuid = 1;
+	int ret = 0;
+
+	/* Check for whether we need to change fs/chunk id */
+	if (!fs_info->new_fsid && !fs_info->new_chunk_tree_uuid)
+		return 0;
+	if (fs_info->new_fsid)
+		same_fsid = !memcmp_extent_buffer(eb, fs_info->new_fsid,
+					  btrfs_header_fsid(), BTRFS_FSID_SIZE);
+	if (fs_info->new_chunk_tree_uuid)
+		same_chunk_tree_uuid =
+			!memcmp_extent_buffer(eb, fs_info->new_chunk_tree_uuid,
+					      btrfs_header_chunk_tree_uuid(eb),
+					      BTRFS_UUID_SIZE);
+	if (same_fsid && same_chunk_tree_uuid)
+		return 0;
+	if (!same_fsid)
+		write_extent_buffer(eb, fs_info->new_fsid, btrfs_header_fsid(),
+				    BTRFS_FSID_SIZE);
+	if (!same_chunk_tree_uuid)
+		write_extent_buffer(eb, fs_info->new_chunk_tree_uuid,
+				    btrfs_header_chunk_tree_uuid(eb),
+				    BTRFS_UUID_SIZE);
+	ret = write_tree_block(NULL, root, eb);
+	return ret;
+}
+
 static void print_usage(void)
 {
 	fprintf(stderr, "usage: btrfstune [options] device\n");
diff --git a/ctree.h b/ctree.h
index 45fef3d..6f882aa 100644
--- a/ctree.h
+++ b/ctree.h
@@ -954,7 +954,9 @@ struct btrfs_device;
 struct btrfs_fs_devices;
 struct btrfs_fs_info {
 	u8 fsid[BTRFS_FSID_SIZE];
+	u8 *new_fsid;
 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
+	u8 *new_chunk_tree_uuid;
 	struct btrfs_root *fs_root;
 	struct btrfs_root *extent_root;
 	struct btrfs_root *tree_root;
-- 
2.3.7

[PATCH 4/8] btrfs-progs: Introduce change_extents_uuid() function.

[Qu Wenruo]

This is the function which iterates all metadata extent and change their
fsid.

This function also does it without transaction.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfstune.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/btrfstune.c b/btrfstune.c
index f039d84..9c1b7aa 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -20,6 +20,7 @@
 #include <stdlib.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <uuid/uuid.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <dirent.h>
@@ -129,6 +130,74 @@ static int change_header_uuid(struct btrfs_root *root, struct extent_buffer *eb)
 	return ret;
 }
 
+static int change_extents_uuid(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_root *root = fs_info->extent_root;
+	struct btrfs_path *path;
+	struct btrfs_key key = {0, 0, 0};
+	int ret = 0;
+
+	if (!fs_info->new_fsid && !fs_info->new_chunk_tree_uuid)
+		return 0;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	/*
+	 * Here we don't use transaction as it will takes a lot of reserve
+	 * space, and that will make a near-full btrfs unable to change uuid
+	 */
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret < 0)
+		goto out;
+
+	while (1) {
+		struct btrfs_extent_item *ei;
+		struct extent_buffer *eb;
+		u64 flags;
+		u64 bytenr;
+
+		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+		if (key.type != BTRFS_EXTENT_ITEM_KEY &&
+		    key.type != BTRFS_METADATA_ITEM_KEY)
+			goto next;
+		ei = btrfs_item_ptr(path->nodes[0], path->slots[0],
+				    struct btrfs_extent_item);
+		flags = btrfs_extent_flags(path->nodes[0], ei);
+		if (!(flags & BTRFS_EXTENT_FLAG_TREE_BLOCK))
+			goto next;
+
+		bytenr = key.objectid;
+		eb = read_tree_block(root, bytenr, root->nodesize, 0);
+		if (IS_ERR(eb)) {
+			fprintf(stderr, "Failed to read tree block: %llu\n",
+				bytenr);
+			ret = PTR_ERR(eb);
+			goto out;
+		}
+		ret = change_header_uuid(root, eb);
+		free_extent_buffer(eb);
+		if (ret < 0) {
+			fprintf(stderr, "Failed to change uuid of tree block: %llu\n",
+				bytenr);
+			goto out;
+		}
+next:
+		ret = btrfs_next_item(root, path);
+		if (ret < 0)
+			goto out;
+		if (ret > 0) {
+			ret = 0;
+			goto out;
+		}
+	}
+
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 static void print_usage(void)
 {
 	fprintf(stderr, "usage: btrfstune [options] device\n");
-- 
2.3.7

[PATCH 5/8] btrfs-progs: Introduce function change_device_uuid().

[Qu Wenruo]

This function just change a device item's uuid.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfstune.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/btrfstune.c b/btrfstune.c
index 9c1b7aa..6706cd7 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -198,6 +198,27 @@ out:
 	return ret;
 }
 
+static int change_device_uuid(struct btrfs_root *root, struct extent_buffer *eb,
+			      int slot)
+{
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_dev_item *di;
+	int ret = 0;
+
+	di = btrfs_item_ptr(eb, slot, struct btrfs_dev_item);
+	if (fs_info->new_fsid) {
+		if (!memcmp_extent_buffer(eb, fs_info->new_fsid,
+					  (unsigned long)btrfs_device_fsid(di),
+					  BTRFS_FSID_SIZE))
+			return ret;
+		write_extent_buffer(eb, fs_info->new_fsid,
+				    (unsigned long)btrfs_device_fsid(di),
+				    BTRFS_FSID_SIZE);
+		ret = write_tree_block(NULL, root, eb);
+	}
+	return ret;
+}
+
 static void print_usage(void)
 {
 	fprintf(stderr, "usage: btrfstune [options] device\n");
-- 
2.3.7

[PATCH 6/8] btrfs-progs: Introduce change_devices_uuid() function.

[Qu Wenruo]

This function will change all dev items' fsid.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfstune.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/btrfstune.c b/btrfstune.c
index 6706cd7..8a5b3c2 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -219,6 +219,49 @@ static int change_device_uuid(struct btrfs_root *root, struct extent_buffer *eb,
 	return ret;
 }
 
+static int change_devices_uuid(struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_root *root = fs_info->chunk_root;
+	struct btrfs_path *path;
+	struct btrfs_key key = {0, 0, 0};
+	int ret = 0;
+
+	/*
+	 * Unlike change_extents_uuid, we only need to change fsid in dev_item
+	 */
+	if (!fs_info->new_fsid)
+		return 0;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+	/* No transaction again */
+	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+	if (ret < 0)
+		goto out;
+
+	while (1) {
+		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+		if (key.type != BTRFS_DEV_ITEM_KEY ||
+		    key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
+			goto next;
+		ret = change_device_uuid(root, path->nodes[0], path->slots[0]);
+		if (ret < 0)
+			goto out;
+next:
+		ret = btrfs_next_item(root, path);
+		if (ret < 0)
+			goto out;
+		if (ret > 0) {
+			ret = 0;
+			goto out;
+		}
+	}
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
 static void print_usage(void)
 {
 	fprintf(stderr, "usage: btrfstune [options] device\n");
-- 
2.3.7

[PATCH 7/8] btrfs-progs: Introduce change_uuid() function.

[Qu Wenruo]

This function does all the needed things for changing uuid.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfstune.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/btrfstune.c b/btrfstune.c
index 8a5b3c2..3e308b7 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -29,6 +29,7 @@
 #include "disk-io.h"
 #include "transaction.h"
 #include "utils.h"
+#include "volumes.h"
 
 static char *device;
 static int force = 0;
@@ -262,6 +263,62 @@ out:
 	return ret;
 }
 
+static int change_uuid(struct btrfs_fs_info *fs_info, char *new_fsid,
+		       char *new_chunk_uuid)
+{
+	int ret = 0;
+
+	/* caller should do extra check on passed uuid */
+	if (new_fsid) {
+		/* allocated mem will be freed at close_ctree() */
+		fs_info->new_fsid = malloc(BTRFS_FSID_SIZE);
+		if (!fs_info->new_fsid) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		ret = uuid_parse(new_fsid, fs_info->new_fsid);
+		if (ret < 0)
+			goto out;
+	}
+
+	if (new_chunk_uuid) {
+		/* allocated mem will be freed at close_ctree() */
+		fs_info->new_chunk_tree_uuid = malloc(BTRFS_UUID_SIZE);
+		if (!fs_info->new_chunk_tree_uuid) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		ret = uuid_parse(new_chunk_uuid, fs_info->new_chunk_tree_uuid);
+		if (ret < 0)
+			goto out;
+	}
+
+	/* Change extents first */
+	ret = change_extents_uuid(fs_info);
+	if (ret < 0) {
+		fprintf(stderr, "Failed to change UUID of metadata\n");
+		goto out;
+	}
+
+	/* Then devices */
+	ret = change_devices_uuid(fs_info);
+	if (ret < 0) {
+		fprintf(stderr, "Failed to change UUID of devices\n");
+		goto out;
+	}
+
+	/* Last, change fsid in super, only fsid change needs this */
+	if (new_fsid) {
+		memcpy(fs_info->fs_devices->fsid, fs_info->new_fsid,
+		       BTRFS_FSID_SIZE);
+		memcpy(fs_info->super_copy->fsid, fs_info->new_fsid,
+		       BTRFS_FSID_SIZE);
+		ret = write_all_supers(fs_info->tree_root);
+	}
+out:
+	return ret;
+}
+
 static void print_usage(void)
 {
 	fprintf(stderr, "usage: btrfstune [options] device\n");
-- 
2.3.7

[PATCH 8/8] btrfs-progs: btrfstune: Introduce new "-u" and "-U" options.

[Qu Wenruo]

These new options will change fsid and chunk tree uuid respectively.

This feature is useful for things like virt-clone, which needs to change
UUID of a filesystem to avoid conflicts.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 Documentation/btrfstune.asciidoc | 12 +++++--
 btrfstune.c                      | 67 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/Documentation/btrfstune.asciidoc b/Documentation/btrfstune.asciidoc
index 9620221..efd5fc8 100644
--- a/Documentation/btrfstune.asciidoc
+++ b/Documentation/btrfstune.asciidoc
@@ -25,8 +25,16 @@ Enable extended inode refs.
 -x::
 Enable skinny metadata extent refs.
 -f::
-Allow dangerous changes, e.g. clear the seeding flag. Make sure that you are
-aware of the dangers.
+Allow dangerous changes, e.g. clear the seeding flag or changing UUID.
+Make sure that you are aware of the dangers.
+-u <uuid>::
+Change fsid to <uuid>.
+-U <uuid>::
+Change chunk tree uuid to <uuid>.
+
+WARNING: If canceling a running fsid changing process, the fs will not be
+mountable due to mismatch fsid. When that happens, use *btrfstune* to change
+fsid again, and it should fix the inconsistence.
 
 When mounting the new device, btrfs will check whether the seeding flag is set
 when try to open seeding device.  If the user clears the seeding flag of the
diff --git a/btrfstune.c b/btrfstune.c
index 3e308b7..ef69a31 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -325,7 +325,9 @@ static void print_usage(void)
 	fprintf(stderr, "\t-S value\tpositive value will enable seeding, zero to disable, negative is not allowed\n");
 	fprintf(stderr, "\t-r \t\tenable extended inode refs\n");
 	fprintf(stderr, "\t-x \t\tenable skinny metadata extent refs\n");
-	fprintf(stderr, "\t-f \t\tforce to set or clear flags, make sure that you are aware of the dangers\n");
+	fprintf(stderr, "\t-f \t\tforce to do dangerous operation, make sure that you are aware of the dangers\n");
+	fprintf(stderr, "\t-u uuid\tchange fsid\n");
+	fprintf(stderr, "\t-U uuid\tchange chunk tree uuid\n");
 }
 
 int main(int argc, char *argv[])
@@ -336,12 +338,15 @@ int main(int argc, char *argv[])
 	int extrefs_flag = 0;
 	int seeding_flag = 0;
 	u64 seeding_value = 0;
+	char *new_fsid = NULL;
+	char *new_chunk_uuid = NULL;
 	int skinny_flag = 0;
+	enum btrfs_open_ctree_flags open_flags = OPEN_CTREE_WRITES;
 	int ret;
 
 	optind = 1;
 	while(1) {
-		int c = getopt(argc, argv, "S:rxf");
+		int c = getopt(argc, argv, "S:rxfu:U:");
 		if (c < 0)
 			break;
 		switch(c) {
@@ -358,6 +363,13 @@ int main(int argc, char *argv[])
 		case 'f':
 			force = 1;
 			break;
+		case 'u':
+			new_fsid = optarg;
+			open_flags |= OPEN_CTREE_IGNORE_FSID;
+			break;
+		case 'U':
+			new_chunk_uuid = optarg;
+			break;
 		default:
 			print_usage();
 			return 1;
@@ -372,13 +384,42 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	if (!(seeding_flag + extrefs_flag + skinny_flag)) {
+	if (!(seeding_flag + extrefs_flag + skinny_flag) &&
+	    !(new_fsid || new_chunk_uuid)) {
 		fprintf(stderr,
 			"ERROR: At least one option should be assigned.\n");
 		print_usage();
 		return 1;
 	}
 
+	if (new_fsid) {
+		uuid_t dummy_uuid;
+
+		if (uuid_parse(new_fsid, dummy_uuid) != 0) {
+			fprintf(stderr, "could not parse UUID: %s\n", new_fsid);
+			return 1;
+		}
+		if (!test_uuid_unique(new_fsid)) {
+			fprintf(stderr, "non-unique UUID: %s\n", new_fsid);
+			return 1;
+		}
+	}
+
+	if (new_chunk_uuid) {
+		uuid_t dummy_uuid;
+
+		if (uuid_parse(new_chunk_uuid, dummy_uuid) != 0) {
+			fprintf(stderr, "could not parse UUID: %s\n",
+				new_chunk_uuid);
+			return 1;
+		}
+		if (!test_uuid_unique(new_chunk_uuid)) {
+			fprintf(stderr, "non-unique UUID: %s\n",
+				new_chunk_uuid);
+			return 1;
+		}
+	}
+
 	ret = check_mounted(device);
 	if (ret < 0) {
 		fprintf(stderr, "Could not check mount status: %s\n",
@@ -389,7 +430,7 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	root = open_ctree(device, 0, OPEN_CTREE_WRITES);
+	root = open_ctree(device, 0, open_flags);
 
 	if (!root) {
 		fprintf(stderr, "Open ctree failed\n");
@@ -424,6 +465,24 @@ int main(int argc, char *argv[])
 		total++;
 	}
 
+	if (new_fsid || new_chunk_uuid) {
+		if (!force) {
+			fprintf(stderr,
+				"Warning: It's highly recommended to run 'btrfs check' before changing UUID. \n");
+			fprintf(stderr,
+				"Also canceling running UUID change progress will cause corruption\n");
+			ret = ask_user("Are you sure to process?");
+			if (!ret) {
+				fprintf(stderr, "UUID change canceled\n");
+				return 1;
+			}
+		}
+		ret = change_uuid(root->fs_info, new_fsid, new_chunk_uuid);
+		if (!ret)
+			success++;
+		total++;
+	}
+
 	if (success == total) {
 		ret = 0;
 	} else {
-- 
2.3.7

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[David Sterba]

On Tue, May 05, 2015 at 02:16:38PM +0800, Qu Wenruo wrote:
> This patchset will introduce offline fsid/chunk tree uuid change
> function to btrfstune command, with "-u" and "-U" options respectively.
> 
> The implement is in-place btrfs_header modification, without transaction
> protection. So the uuid change can be done on any valid image even it is
> already full.
> 
> Since we don't use transaction to protect the convert, it's possible one
> can stop the running convert progress, causing the fs in a inconsistent
> status and unable to be mounted.
> To avoid such problem, we introduce new open ctree flags
> OPEN_CTREE_IGNORE_FSID, allowing btrfstune to finish the convert.

Code looks good to me. I'm not sure whether it should land in btrfstune
or in 'btrfs' proper.

* btrfstune should go away at some point, so I'd rather not add new
  features there

* 'btrfs property set -t filesystem uuid' seems best to me at this moment

A minor suggestion: set BTRFS_SUPER_FLAG_ERROR on the filesystem during
the conversion and drop it after it finishes successfully. It's merely a
hint that there's something wrong with the filesytem, the mount will
fail because filesystem uuid would not match the device id.

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Qu Wenruo]

-------- Original Message  --------
Subject: Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change 
for btrfstune.
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2015年05月05日 23:20

> On Tue, May 05, 2015 at 02:16:38PM +0800, Qu Wenruo wrote:
>> This patchset will introduce offline fsid/chunk tree uuid change
>> function to btrfstune command, with "-u" and "-U" options respectively.
>>
>> The implement is in-place btrfs_header modification, without transaction
>> protection. So the uuid change can be done on any valid image even it is
>> already full.
>>
>> Since we don't use transaction to protect the convert, it's possible one
>> can stop the running convert progress, causing the fs in a inconsistent
>> status and unable to be mounted.
>> To avoid such problem, we introduce new open ctree flags
>> OPEN_CTREE_IGNORE_FSID, allowing btrfstune to finish the convert.
>
> Code looks good to me. I'm not sure whether it should land in btrfstune
> or in 'btrfs' proper.
>
> * btrfstune should go away at some point, so I'd rather not add new
>    features there
>
> * 'btrfs property set -t filesystem uuid' seems best to me at this moment
Good idea.

I'll change it to property in next version.
>
> A minor suggestion: set BTRFS_SUPER_FLAG_ERROR on the filesystem during
> the conversion and drop it after it finishes successfully. It's merely a
> hint that there's something wrong with the filesytem, the mount will
> fail because filesystem uuid would not match the device id.
>
Nice idea too.
But I'd like to use more specific flags like
BTRFS_SUPER_FLAG_UUID_CHANGING to give better info if kernel can't mount it.

Thanks for all these advice!
Qu

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[David Sterba]

On Wed, May 06, 2015 at 08:49:33AM +0800, Qu Wenruo wrote:
> > Code looks good to me. I'm not sure whether it should land in btrfstune
> > or in 'btrfs' proper.
> >
> > * btrfstune should go away at some point, so I'd rather not add new
> >    features there
> >
> > * 'btrfs property set -t filesystem uuid' seems best to me at this moment
> Good idea.
> 
> I'll change it to property in next version.

I've realized this later on.  The original purpose of the properties is
to manipulate objects on a mounted filesystem, so this might need some
adjustnments on the progs side. I'd really want to avoid another
separate tool, so we should make sure that the interface is sane even if
it could mix online/offline filesystem changes.

Respecting the current syntax, the way to set the uuid is:

 btrfs prop set -t filesytem /dev/sdx uuid ...UUID...

In this special case, the property handler should check if the device is
unmounted (and is a btrfs filesystem) and then do the rest.

> > A minor suggestion: set BTRFS_SUPER_FLAG_ERROR on the filesystem during
> > the conversion and drop it after it finishes successfully. It's merely a
> > hint that there's something wrong with the filesytem, the mount will
> > fail because filesystem uuid would not match the device id.
> >
> Nice idea too.
> But I'd like to use more specific flags like
> BTRFS_SUPER_FLAG_UUID_CHANGING to give better info if kernel can't mount it.

Yeah that would be better and we'd use that for the on-line uuid change
as well. I've outlined the idea here
https://btrfs.wiki.kernel.org/index.php/Project_ideas#Filesystem_UUID_change_-_on-line

The code should really support only 2 UUIDs, so we should check if the
UUIDs specified by user matches one of them if the superblock flag is
set. Ie. a partial uuid conversion must be finished before another
UUID change (though it's a rare case, we have to be ready for that).

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Qu Wenruo]

-------- Original Message  --------
Subject: Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change 
for btrfstune.
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2015年05月06日 23:43

> On Wed, May 06, 2015 at 08:49:33AM +0800, Qu Wenruo wrote:
>>> Code looks good to me. I'm not sure whether it should land in btrfstune
>>> or in 'btrfs' proper.
>>>
>>> * btrfstune should go away at some point, so I'd rather not add new
>>>     features there
>>>
>>> * 'btrfs property set -t filesystem uuid' seems best to me at this moment
>> Good idea.
>>
>> I'll change it to property in next version.
>
> I've realized this later on.  The original purpose of the properties is
> to manipulate objects on a mounted filesystem, so this might need some
> adjustnments on the progs side. I'd really want to avoid another
> separate tool, so we should make sure that the interface is sane even if
> it could mix online/offline filesystem changes.
That's the original reason why I put these codes into btrfstune.

But that should not be a big problem. (See below)
>
> Respecting the current syntax, the way to set the uuid is:
>
>   btrfs prop set -t filesytem /dev/sdx uuid ...UUID...
>
> In this special case, the property handler should check if the device is
> unmounted (and is a btrfs filesystem) and then do the rest.
This is one good idea, but I'm afraid it may still be confusing.
And things may get even worse when we try to move everything into 
btrfs-prop,
E.g. if we auto move seeding flag to prop, we must give a quite detailed 
explain on which property is offline.

So I prefer to add a new type called offline, and put uuid change with 
other btrfstune function into this type.
IMHO this should provide a good explain on which is online and which is 
offline.

How do you think about this method?

Thanks,
Qu
>
>>> A minor suggestion: set BTRFS_SUPER_FLAG_ERROR on the filesystem during
>>> the conversion and drop it after it finishes successfully. It's merely a
>>> hint that there's something wrong with the filesytem, the mount will
>>> fail because filesystem uuid would not match the device id.
>>>
>> Nice idea too.
>> But I'd like to use more specific flags like
>> BTRFS_SUPER_FLAG_UUID_CHANGING to give better info if kernel can't mount it.
>
> Yeah that would be better and we'd use that for the on-line uuid change
> as well. I've outlined the idea here
> https://btrfs.wiki.kernel.org/index.php/Project_ideas#Filesystem_UUID_change_-_on-line
>
> The code should really support only 2 UUIDs, so we should check if the
> UUIDs specified by user matches one of them if the superblock flag is
> set. Ie. a partial uuid conversion must be finished before another
> UUID change (though it's a rare case, we have to be ready for that).
>

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[David Sterba]

On Fri, May 08, 2015 at 04:57:14PM +0800, Qu Wenruo wrote:
> > Respecting the current syntax, the way to set the uuid is:
> >
> >   btrfs prop set -t filesytem /dev/sdx uuid ...UUID...
> >
> > In this special case, the property handler should check if the device is
> > unmounted (and is a btrfs filesystem) and then do the rest.
> This is one good idea, but I'm afraid it may still be confusing.
> And things may get even worse when we try to move everything into 
> btrfs-prop,
> E.g. if we auto move seeding flag to prop, we must give a quite detailed 
> explain on which property is offline.
> 
> So I prefer to add a new type called offline, and put uuid change with 
> other btrfstune function into this type.
> IMHO this should provide a good explain on which is online and which is 
> offline.
> 
> How do you think about this method?

I don't think that 'offline' is a good choice. The type should denote
the modified object, ie device, inode, filesystem. What's 'offline' in
this context?

The property interface can be used to modify other features both for
mounted and unmounted filesystem, I agree that this should be done in a
way that's not confusing. There are operations that are allowed on an
unmounted fs (seeding), mounted + unmounted (extref feature) etc. so I
think we could add options like --check-unmounted that would not allow
to act on a mounted filesystem.

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Qu Wenruo]

-------- Original Message  --------
Subject: Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change 
for btrfstune.
From: David Sterba <dsterba@suse.cz>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>
Date: 2015年05月12日 00:24

> On Fri, May 08, 2015 at 04:57:14PM +0800, Qu Wenruo wrote:
>>> Respecting the current syntax, the way to set the uuid is:
>>>
>>>    btrfs prop set -t filesytem /dev/sdx uuid ...UUID...
>>>
>>> In this special case, the property handler should check if the device is
>>> unmounted (and is a btrfs filesystem) and then do the rest.
>> This is one good idea, but I'm afraid it may still be confusing.
>> And things may get even worse when we try to move everything into
>> btrfs-prop,
>> E.g. if we auto move seeding flag to prop, we must give a quite detailed
>> explain on which property is offline.
>>
>> So I prefer to add a new type called offline, and put uuid change with
>> other btrfstune function into this type.
>> IMHO this should provide a good explain on which is online and which is
>> offline.
>>
>> How do you think about this method?
>
> I don't think that 'offline' is a good choice. The type should denote
> the modified object, ie device, inode, filesystem. What's 'offline' in
> this context?
Right, 'offline' type here is just a easy trick for me to code, not a 
meaningful type.
>
> The property interface can be used to modify other features both for
> mounted and unmounted filesystem, I agree that this should be done in a
> way that's not confusing. There are operations that are allowed on an
> unmounted fs (seeding), mounted + unmounted (extref feature) etc. so I
> think we could add options like --check-unmounted that would not allow
> to act on a mounted filesystem.

Nice idea, but i prefer a option like "--offline" to enable the usage of 
offline operations.
E.g, for normal online operations, nothing is changed.

And for "btrfs prop set <DEV> uuid <UUID>", "--offline" must be 
specified to work.
Or a message will be output to info user.

What about this one?

Thanks,
Qu

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Anand Jain]

On 05/12/2015 10:09 AM, Qu Wenruo wrote:
>
>
> -------- Original Message  --------
> Subject: Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change
> for btrfstune.
> From: David Sterba <dsterba@suse.cz>
> To: Qu Wenruo <quwenruo@cn.fujitsu.com>
> Date: 2015年05月12日 00:24
>
>> On Fri, May 08, 2015 at 04:57:14PM +0800, Qu Wenruo wrote:
>>>> Respecting the current syntax, the way to set the uuid is:
>>>>
>>>>    btrfs prop set -t filesytem /dev/sdx uuid ...UUID...
>>>>
>>>> In this special case, the property handler should check if the
>>>> device is
>>>> unmounted (and is a btrfs filesystem) and then do the rest.
>>> This is one good idea, but I'm afraid it may still be confusing.
>>> And things may get even worse when we try to move everything into
>>> btrfs-prop,
>>> E.g. if we auto move seeding flag to prop, we must give a quite detailed
>>> explain on which property is offline.
>>>
>>> So I prefer to add a new type called offline, and put uuid change with
>>> other btrfstune function into this type.
>>> IMHO this should provide a good explain on which is online and which is
>>> offline.
>>>
>>> How do you think about this method?
>>
>> I don't think that 'offline' is a good choice. The type should denote
>> the modified object, ie device, inode, filesystem. What's 'offline' in
>> this context?
> Right, 'offline' type here is just a easy trick for me to code, not a
> meaningful type.
>>
>> The property interface can be used to modify other features both for
>> mounted and unmounted filesystem, I agree that this should be done in a
>> way that's not confusing. There are operations that are allowed on an
>> unmounted fs (seeding), mounted + unmounted (extref feature) etc. so I
>> think we could add options like --check-unmounted that would not allow
>> to act on a mounted filesystem.
>
> Nice idea, but i prefer a option like "--offline" to enable the usage of
> offline operations.
> E.g, for normal online operations, nothing is changed.
>
> And for "btrfs prop set <DEV> uuid <UUID>", "--offline" must be
> specified to work.
> Or a message will be output to info user.
>
> What about this one?


Qu, Thanks for the feature.

Qu/David,

  I strongly recommend this feature to be part of btrfstune as
  of now, as originally planned by Qu. When its time to move
  to btrfs cli we could. If we do it wrongly now it would stay
  there forever.

  We could take better call, when all the features of progs are
  in place.

Thanks, Anand

> 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 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Qu Wenruo]

-------- Original Message  --------
Subject: Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change 
for btrfstune.
From: Anand Jain <Anand.Jain@oracle.com>
To: Qu Wenruo <quwenruo@cn.fujitsu.com>, <dsterba@suse.cz>
Date: 2015年05月12日 12:00

>
>
> On 05/12/2015 10:09 AM, Qu Wenruo wrote:
>>
>>
>> -------- Original Message  --------
>> Subject: Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change
>> for btrfstune.
>> From: David Sterba <dsterba@suse.cz>
>> To: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> Date: 2015年05月12日 00:24
>>
>>> On Fri, May 08, 2015 at 04:57:14PM +0800, Qu Wenruo wrote:
>>>>> Respecting the current syntax, the way to set the uuid is:
>>>>>
>>>>>    btrfs prop set -t filesytem /dev/sdx uuid ...UUID...
>>>>>
>>>>> In this special case, the property handler should check if the
>>>>> device is
>>>>> unmounted (and is a btrfs filesystem) and then do the rest.
>>>> This is one good idea, but I'm afraid it may still be confusing.
>>>> And things may get even worse when we try to move everything into
>>>> btrfs-prop,
>>>> E.g. if we auto move seeding flag to prop, we must give a quite
>>>> detailed
>>>> explain on which property is offline.
>>>>
>>>> So I prefer to add a new type called offline, and put uuid change with
>>>> other btrfstune function into this type.
>>>> IMHO this should provide a good explain on which is online and which is
>>>> offline.
>>>>
>>>> How do you think about this method?
>>>
>>> I don't think that 'offline' is a good choice. The type should denote
>>> the modified object, ie device, inode, filesystem. What's 'offline' in
>>> this context?
>> Right, 'offline' type here is just a easy trick for me to code, not a
>> meaningful type.
>>>
>>> The property interface can be used to modify other features both for
>>> mounted and unmounted filesystem, I agree that this should be done in a
>>> way that's not confusing. There are operations that are allowed on an
>>> unmounted fs (seeding), mounted + unmounted (extref feature) etc. so I
>>> think we could add options like --check-unmounted that would not allow
>>> to act on a mounted filesystem.
>>
>> Nice idea, but i prefer a option like "--offline" to enable the usage of
>> offline operations.
>> E.g, for normal online operations, nothing is changed.
>>
>> And for "btrfs prop set <DEV> uuid <UUID>", "--offline" must be
>> specified to work.
>> Or a message will be output to info user.
>>
>> What about this one?
>
>
> Qu, Thanks for the feature.
>
> Qu/David,
>
>   I strongly recommend this feature to be part of btrfstune as
>   of now, as originally planned by Qu. When its time to move
>   to btrfs cli we could. If we do it wrongly now it would stay
>   there forever.
>
>   We could take better call, when all the features of progs are
>   in place.
>
> Thanks, Anand
This is much easier for me, no need to consider the format of new 
online-offline mixed parameter for btrfs progs.

But the decision is on David.

Thanks,
Qu
>
>> 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 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[David Sterba]

On Tue, May 12, 2015 at 12:00:28PM +0800, Anand Jain wrote:
>   I strongly recommend this feature to be part of btrfstune as
>   of now, as originally planned by Qu. When its time to move
>   to btrfs cli we could. If we do it wrongly now it would stay
>   there forever.

I agree with that now. The code to change the uuid is straightforward,
we'd got stuck on getting the interface right. At the moment it's more
likely to get it wrong because the properties are not finalized.  A
standalone tool for the uuid change is IMO not the right way either,
btrfstune is usually packaged already.

I'll go through the patches and take the core changes + the btrfstune
interface from v1.

Thank you both for the feedback.

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Anand Jain]

On 05/13/2015 09:43 PM, David Sterba wrote:
> On Tue, May 12, 2015 at 12:00:28PM +0800, Anand Jain wrote:
>>    I strongly recommend this feature to be part of btrfstune as
>>    of now, as originally planned by Qu. When its time to move
>>    to btrfs cli we could. If we do it wrongly now it would stay
>>    there forever.
>
> I agree with that now. The code to change the uuid is straightforward,
> we'd got stuck on getting the interface right. At the moment it's more
> likely to get it wrong because the properties are not finalized.  A
> standalone tool for the uuid change is IMO not the right way either,
> btrfstune is usually packaged already.

  How about placing the new sub-cli like below, until it finds its
  final home.

  btrfs unstable <sub-cli>
  or
  btrfs experimental <sub-cli>

thanks, Anand

> I'll go through the patches and take the core changes + the btrfstune
> interface from v1.
>
> Thank you both for the feedback.
> --
> 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 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[David Sterba]

On Fri, May 15, 2015 at 11:42:57PM +0800, Anand Jain wrote:
> 
> 
> On 05/13/2015 09:43 PM, David Sterba wrote:
> > On Tue, May 12, 2015 at 12:00:28PM +0800, Anand Jain wrote:
> >>    I strongly recommend this feature to be part of btrfstune as
> >>    of now, as originally planned by Qu. When its time to move
> >>    to btrfs cli we could. If we do it wrongly now it would stay
> >>    there forever.
> >
> > I agree with that now. The code to change the uuid is straightforward,
> > we'd got stuck on getting the interface right. At the moment it's more
> > likely to get it wrong because the properties are not finalized.  A
> > standalone tool for the uuid change is IMO not the right way either,
> > btrfstune is usually packaged already.
> 
>   How about placing the new sub-cli like below, until it finds its
>   final home.
> 
>   btrfs unstable <sub-cli>
>   or
>   btrfs experimental <sub-cli>

You probably remember this

http://thread.gmane.org/gmane.comp.file-systems.btrfs/27396

I'll revive the integration branches and merge the unfinished features
there.

Re: [PATCH 0/8] Introduce offline fsid/chunk tree uuid change for btrfstune.

[Anand Jain]

On 05/22/2015 12:30 AM, David Sterba wrote:
> On Fri, May 15, 2015 at 11:42:57PM +0800, Anand Jain wrote:
>>
>>
>> On 05/13/2015 09:43 PM, David Sterba wrote:
>>> On Tue, May 12, 2015 at 12:00:28PM +0800, Anand Jain wrote:
>>>>     I strongly recommend this feature to be part of btrfstune as
>>>>     of now, as originally planned by Qu. When its time to move
>>>>     to btrfs cli we could. If we do it wrongly now it would stay
>>>>     there forever.
>>>
>>> I agree with that now. The code to change the uuid is straightforward,
>>> we'd got stuck on getting the interface right. At the moment it's more
>>> likely to get it wrong because the properties are not finalized.  A
>>> standalone tool for the uuid change is IMO not the right way either,
>>> btrfstune is usually packaged already.
>>
>>    How about placing the new sub-cli like below, until it finds its
>>    final home.
>>
>>    btrfs unstable <sub-cli>
>>    or
>>    btrfs experimental <sub-cli>
>
> You probably remember this
>
> http://thread.gmane.org/gmane.comp.file-systems.btrfs/27396

  yes I remember it vaguely. thanks for pulling it out.
  I didn't anticipate this context of uncertainty in naming
  of sub cli before. you were right this is useful.

Thanks, Anand


> I'll revive the integration branches and merge the unfinished features
> there.
> --
> 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
>