* [PATCH 0/4] Userspace support for FSID change
@ 2018-08-29 8:35 Nikolay Borisov
2018-08-29 8:35 ` [PATCH 1/4] btrfs-progs: Add support for metadata_uuid field Nikolay Borisov
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Nikolay Borisov @ 2018-08-29 8:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Here is the userspace tooling support for utilising the new metadata_uuid field,
enabling the change of fsid without having to rewrite every metadata block. This
patchset consists of adding support for the new field to various tools and
files (Patch 1). The actual implementation of the new -m|-M options (which are
described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
exercises the new options and verifies certain invariants hold (these are also
described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
structures.
The intended usecase of this feature is to give the sysadmin the ability to
create copies of filesystesm, change their uuid quickly and mount them alongside
the original filesystem for, say, forensic purposes.
One thing which still hasn't been set in stone is whether the new options
will remain as -m|-M or whether they should subsume the current -u|-U - from
the point of view of users nothing should change. So this is something which
I'd like to hear from the community. Of course the alternative of rewriting
the metadata blocks will be assigne new options - perhaps -m|M ?
I've tested this with multiple xfstest runs with the new tools installed as
well as running btrfs-progs test and have observed no regressions.
Nikolay Borisov (4):
btrfs-progs: Add support for metadata_uuid field.
btrfstune: Add support for changing the user uuid
btrfs-progs: tests: Add tests for changing fsid feature
btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
btrfstune.c | 174 ++++++++++++++++++++++++-----
check/main.c | 2 +-
chunk-recover.c | 17 ++-
cmds-filesystem.c | 2 +
cmds-inspect-dump-super.c | 22 +++-
convert/common.c | 2 +
ctree.c | 15 +--
ctree.h | 8 +-
disk-io.c | 62 ++++++++--
image/main.c | 25 +++--
tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
volumes.c | 37 ++++--
volumes.h | 1 +
13 files changed, 431 insertions(+), 73 deletions(-)
create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] btrfs-progs: Add support for metadata_uuid field.
2018-08-29 8:35 [PATCH 0/4] Userspace support for FSID change Nikolay Borisov
@ 2018-08-29 8:35 ` Nikolay Borisov
2018-08-29 8:35 ` [PATCH 2/4] btrfstune: Add support for changing the user uuid Nikolay Borisov
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2018-08-29 8:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Add support for a new metadata_uuid field. This is just a preparatory
commit which switches all users of the fsid field for metdata comparison
purposes to utilize the new field. This more or less mirrors the
kernel patch, additionally:
* Update 'btrfs inspect-internal dump-super' to account for the new
field. This involes introducing the 'metadata_uuid' line to the
output and updating the logic for comparing the fs uuid to the
dev_item uuid.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
btrfstune.c | 3 +++
check/main.c | 2 +-
chunk-recover.c | 16 ++++++++++---
cmds-filesystem.c | 2 ++
cmds-inspect-dump-super.c | 22 ++++++++++++++---
convert/common.c | 2 ++
ctree.c | 14 +++++------
ctree.h | 8 +++++--
disk-io.c | 60 ++++++++++++++++++++++++++++++++++++++---------
image/main.c | 25 +++++++++++++-------
volumes.c | 34 +++++++++++++++++++++------
volumes.h | 1 +
12 files changed, 146 insertions(+), 43 deletions(-)
diff --git a/btrfstune.c b/btrfstune.c
index 889b931c55d8..ea04159a81ce 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -248,6 +248,9 @@ static int change_fsid_prepare(struct btrfs_fs_info *fs_info)
if (ret < 0)
return ret;
+ /* Also need to change the metadatauuid of the fs info */
+ memcpy(fs_info->metadata_uuid, fs_info->new_fsid, BTRFS_FSID_SIZE);
+
/* also restore new chunk_tree_id into tree_root for restore */
write_extent_buffer(tree_root->node, fs_info->new_chunk_tree_uuid,
btrfs_header_chunk_tree_uuid(tree_root->node),
diff --git a/check/main.c b/check/main.c
index bc2ee22f7943..0790264190f2 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8418,7 +8418,7 @@ static int btrfs_fsck_reinit_root(struct btrfs_trans_handle *trans,
btrfs_set_header_backref_rev(c, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(c, root->root_key.objectid);
- write_extent_buffer(c, root->fs_info->fsid,
+ write_extent_buffer(c, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(c, root->fs_info->chunk_tree_uuid,
diff --git a/chunk-recover.c b/chunk-recover.c
index 1d30db51d8ed..31325bfc54e0 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -759,7 +759,7 @@ static int scan_one_device(void *dev_scan_struct)
rc->nodesize)
break;
- if (memcmp_extent_buffer(buf, rc->fs_devices->fsid,
+ if (memcmp_extent_buffer(buf, rc->fs_devices->metadata_uuid,
btrfs_header_fsid(),
BTRFS_FSID_SIZE)) {
bytenr += rc->sectorsize;
@@ -1155,7 +1155,7 @@ static int __rebuild_chunk_root(struct btrfs_trans_handle *trans,
btrfs_set_header_level(cow, 0);
btrfs_set_header_backref_rev(cow, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(cow, BTRFS_CHUNK_TREE_OBJECTID);
- write_extent_buffer(cow, root->fs_info->fsid,
+ write_extent_buffer(cow, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(cow, root->fs_info->chunk_tree_uuid,
@@ -1192,7 +1192,8 @@ static int __rebuild_device_items(struct btrfs_trans_handle *trans,
btrfs_set_stack_device_io_width(dev_item, dev->io_width);
btrfs_set_stack_device_sector_size(dev_item, dev->sector_size);
memcpy(dev_item->uuid, dev->uuid, BTRFS_UUID_SIZE);
- memcpy(dev_item->fsid, dev->fs_devices->fsid, BTRFS_UUID_SIZE);
+ memcpy(dev_item->fsid, dev->fs_devices->metadata_uuid,
+ BTRFS_FSID_SIZE);
ret = btrfs_insert_item(trans, root, &key,
dev_item, sizeof(*dev_item));
@@ -1432,6 +1433,7 @@ open_ctree_with_broken_chunk(struct recover_control *rc)
struct btrfs_fs_info *fs_info;
struct btrfs_super_block *disk_super;
struct extent_buffer *eb;
+ u64 features;
int ret;
fs_info = btrfs_new_fs_info(1, BTRFS_SUPER_INFO_OFFSET);
@@ -1464,6 +1466,14 @@ open_ctree_with_broken_chunk(struct recover_control *rc)
if (ret)
goto out_devices;
+ features = btrfs_super_incompat_flags(disk_super);
+
+ if (features & BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
+ memcpy(fs_info->metadata_uuid, disk_super->metadata_uuid,
+ BTRFS_FSID_SIZE);
+ else
+ memcpy(fs_info->metadata_uuid, fs_info->fsid, BTRFS_FSID_SIZE);
+
btrfs_setup_root(fs_info->chunk_root, fs_info,
BTRFS_CHUNK_TREE_OBJECTID);
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 06c8311bdfe3..10946b31976f 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -172,6 +172,7 @@ static int match_search_item_kernel(u8 *fsid, char *mnt, char *label,
return 0;
}
+/* Search for user visible uuid 'search' in registered filesystems */
static int uuid_search(struct btrfs_fs_devices *fs_devices, const char *search)
{
char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
@@ -497,6 +498,7 @@ static int copy_fs_devices(struct btrfs_fs_devices *dst,
int ret = 0;
memcpy(dst->fsid, src->fsid, BTRFS_FSID_SIZE);
+ memcpy(dst->metadata_uuid, src->metadata_uuid, BTRFS_FSID_SIZE);
INIT_LIST_HEAD(&dst->devices);
dst->seed = NULL;
diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
index e965267c5d96..97e9624db8d7 100644
--- a/cmds-inspect-dump-super.c
+++ b/cmds-inspect-dump-super.c
@@ -228,7 +228,8 @@ static struct readable_flag_entry incompat_flags_array[] = {
DEF_INCOMPAT_FLAG_ENTRY(EXTENDED_IREF),
DEF_INCOMPAT_FLAG_ENTRY(RAID56),
DEF_INCOMPAT_FLAG_ENTRY(SKINNY_METADATA),
- DEF_INCOMPAT_FLAG_ENTRY(NO_HOLES)
+ DEF_INCOMPAT_FLAG_ENTRY(NO_HOLES),
+ DEF_INCOMPAT_FLAG_ENTRY(METADATA_UUID)
};
static const int incompat_flags_num = sizeof(incompat_flags_array) /
sizeof(struct readable_flag_entry);
@@ -319,6 +320,10 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
u8 *p;
u32 csum_size;
u16 csum_type;
+ bool metadata_uuid_present = (btrfs_super_incompat_flags(sb) &
+ BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
+ int cmp_res = 0;
+
csum_type = btrfs_super_csum_type(sb);
csum_size = BTRFS_CSUM_SIZE;
@@ -365,6 +370,12 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
uuid_unparse(sb->fsid, buf);
printf("fsid\t\t\t%s\n", buf);
+ if (metadata_uuid_present) {
+ uuid_unparse(sb->metadata_uuid, buf);
+ printf("metadata_uuid\t\t%s\n", buf);
+ } else {
+ printf("metadata_uuid\t\t%s\n", buf);
+ }
printf("label\t\t\t");
s = sb->label;
@@ -424,9 +435,14 @@ static void dump_superblock(struct btrfs_super_block *sb, int full)
printf("dev_item.uuid\t\t%s\n", buf);
uuid_unparse(sb->dev_item.fsid, buf);
+ if (metadata_uuid_present) {
+ cmp_res = !memcmp(sb->dev_item.fsid, sb->metadata_uuid,
+ BTRFS_FSID_SIZE);
+ } else {
+ cmp_res = !memcmp(sb->dev_item.fsid, sb->fsid, BTRFS_FSID_SIZE);
+ }
printf("dev_item.fsid\t\t%s %s\n", buf,
- !memcmp(sb->dev_item.fsid, sb->fsid, BTRFS_FSID_SIZE) ?
- "[match]" : "[DON'T MATCH]");
+ cmp_res ? "[match]" : "[DON'T MATCH]");
printf("dev_item.type\t\t%llu\n", (unsigned long long)
btrfs_stack_device_type(&sb->dev_item));
diff --git a/convert/common.c b/convert/common.c
index 6ddf4a46ce3c..77cbaf14f3c7 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -107,9 +107,11 @@ static int setup_temp_super(int fd, struct btrfs_mkfs_config *cfg,
ret = -EINVAL;
goto out;
}
+ uuid_copy(super->metadata_uuid, super->fsid);
} else {
uuid_generate(super->fsid);
uuid_unparse(super->fsid, cfg->fs_uuid);
+ uuid_copy(super->metadata_uuid, super->fsid);
}
uuid_generate(chunk_uuid);
uuid_unparse(chunk_uuid, cfg->chunk_uuid);
diff --git a/ctree.c b/ctree.c
index d8a6883aa85f..883c2ae1861d 100644
--- a/ctree.c
+++ b/ctree.c
@@ -134,7 +134,7 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
else
btrfs_set_header_owner(cow, new_root_objectid);
- write_extent_buffer(cow, root->fs_info->fsid,
+ write_extent_buffer(cow, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
WARN_ON(btrfs_header_generation(buf) > trans->transid);
@@ -308,7 +308,7 @@ int __btrfs_cow_block(struct btrfs_trans_handle *trans,
else
btrfs_set_header_owner(cow, root->root_key.objectid);
- write_extent_buffer(cow, root->fs_info->fsid,
+ write_extent_buffer(cow, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
WARN_ON(!(buf->flags & EXTENT_BAD_TRANSID) &&
@@ -1458,7 +1458,7 @@ static int noinline insert_new_root(struct btrfs_trans_handle *trans,
btrfs_node_key(lower, &lower_key, 0);
c = btrfs_alloc_free_block(trans, root, root->fs_info->nodesize,
- root->root_key.objectid, &lower_key,
+ root->root_key.objectid, &lower_key,
level, root->node->start, 0);
if (IS_ERR(c))
@@ -1474,7 +1474,7 @@ static int noinline insert_new_root(struct btrfs_trans_handle *trans,
root_add_used(root, root->fs_info->nodesize);
- write_extent_buffer(c, root->fs_info->fsid,
+ write_extent_buffer(c, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(c, root->fs_info->chunk_tree_uuid,
@@ -1595,7 +1595,7 @@ static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
btrfs_set_header_generation(split, trans->transid);
btrfs_set_header_backref_rev(split, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(split, root->root_key.objectid);
- write_extent_buffer(split, root->fs_info->fsid,
+ write_extent_buffer(split, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(split, root->fs_info->chunk_tree_uuid,
btrfs_header_chunk_tree_uuid(split),
@@ -2157,7 +2157,7 @@ static noinline int split_leaf(struct btrfs_trans_handle *trans,
}
}
}
-
+
if (split == 0)
btrfs_cpu_key_to_disk(&disk_key, ins_key);
else
@@ -2177,7 +2177,7 @@ static noinline int split_leaf(struct btrfs_trans_handle *trans,
btrfs_set_header_backref_rev(right, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(right, root->root_key.objectid);
btrfs_set_header_level(right, 0);
- write_extent_buffer(right, root->fs_info->fsid,
+ write_extent_buffer(right, root->fs_info->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(right, root->fs_info->chunk_tree_uuid,
diff --git a/ctree.h b/ctree.h
index 4719962df67d..c22d36b1871b 100644
--- a/ctree.h
+++ b/ctree.h
@@ -454,8 +454,9 @@ struct btrfs_super_block {
__le64 cache_generation;
__le64 uuid_tree_generation;
+ u8 metadata_uuid[BTRFS_FSID_SIZE];
/* future expansion */
- __le64 reserved[30];
+ __le64 reserved[28];
u8 sys_chunk_array[BTRFS_SYSTEM_CHUNK_ARRAY_SIZE];
struct btrfs_root_backup super_roots[BTRFS_NUM_BACKUP_ROOTS];
} __attribute__ ((__packed__));
@@ -489,6 +490,7 @@ struct btrfs_super_block {
#define BTRFS_FEATURE_INCOMPAT_RAID56 (1ULL << 7)
#define BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA (1ULL << 8)
#define BTRFS_FEATURE_INCOMPAT_NO_HOLES (1ULL << 9)
+#define BTRFS_FEATURE_INCOMPAT_METADATA_UUID (1ULL << 10)
#define BTRFS_FEATURE_COMPAT_SUPP 0ULL
@@ -509,7 +511,8 @@ struct btrfs_super_block {
BTRFS_FEATURE_INCOMPAT_RAID56 | \
BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS | \
BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA | \
- BTRFS_FEATURE_INCOMPAT_NO_HOLES)
+ BTRFS_FEATURE_INCOMPAT_NO_HOLES | \
+ BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
/*
* A leaf is full of items. offset and size tell us where to find
@@ -1077,6 +1080,7 @@ struct btrfs_device;
struct btrfs_fs_devices;
struct btrfs_fs_info {
u8 fsid[BTRFS_FSID_SIZE];
+ u8 metadata_uuid[BTRFS_FSID_SIZE];
u8 *new_fsid;
u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
u8 *new_chunk_tree_uuid;
diff --git a/disk-io.c b/disk-io.c
index 26e4f6e93ed6..c3be4a1017b7 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -55,8 +55,9 @@ static int check_tree_block(struct btrfs_fs_info *fs_info,
struct extent_buffer *buf)
{
- struct btrfs_fs_devices *fs_devices;
+ struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
u32 nodesize = fs_info->nodesize;
+ bool fsid_match = false;
int ret = BTRFS_BAD_FSID;
if (buf->start != btrfs_header_bytenr(buf))
@@ -72,12 +73,26 @@ static int check_tree_block(struct btrfs_fs_info *fs_info,
btrfs_header_level(buf) != 0)
return BTRFS_BAD_NRITEMS;
- fs_devices = fs_info->fs_devices;
while (fs_devices) {
- if (fs_info->ignore_fsid_mismatch ||
- !memcmp_extent_buffer(buf, fs_devices->fsid,
- btrfs_header_fsid(),
- BTRFS_FSID_SIZE)) {
+ /*
+ * Checking the incompat flag is only valid for the current
+ * fs. For seed devices it's forbidden to have their uuid
+ * changed so reading ->fsid in this case is fine
+ */
+ if (fs_devices == fs_info->fs_devices &&
+ btrfs_fs_incompat(fs_info, METADATA_UUID))
+ fsid_match = !memcmp_extent_buffer(buf,
+ fs_devices->metadata_uuid,
+ btrfs_header_fsid(),
+ BTRFS_FSID_SIZE);
+ else
+ fsid_match = !memcmp_extent_buffer(buf,
+ fs_devices->fsid,
+ btrfs_header_fsid(),
+ BTRFS_FSID_SIZE);
+
+
+ if (fs_info->ignore_fsid_mismatch || fsid_match) {
ret = 0;
break;
}
@@ -103,7 +118,7 @@ static void print_tree_block_error(struct btrfs_fs_info *fs_info,
read_extent_buffer(eb, buf, btrfs_header_fsid(),
BTRFS_UUID_SIZE);
uuid_unparse(buf, found_uuid);
- uuid_unparse(fs_info->fsid, fs_uuid);
+ uuid_unparse(fs_info->metadata_uuid, fs_uuid);
fprintf(stderr, "fsid mismatch, want=%s, have=%s\n",
fs_uuid, found_uuid);
break;
@@ -1169,6 +1184,12 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
}
memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
+ if (btrfs_fs_incompat(fs_info, METADATA_UUID)) {
+ memcpy(fs_info->metadata_uuid, disk_super->metadata_uuid,
+ BTRFS_FSID_SIZE);
+ } else {
+ memcpy(fs_info->metadata_uuid, fs_info->fsid, BTRFS_FSID_SIZE);
+ }
fs_info->sectorsize = btrfs_super_sectorsize(disk_super);
fs_info->nodesize = btrfs_super_nodesize(disk_super);
fs_info->stripesize = btrfs_super_stripesize(disk_super);
@@ -1289,6 +1310,7 @@ static int check_super(struct btrfs_super_block *sb, unsigned sbflags)
u32 crc;
u16 csum_type;
int csum_size;
+ u8 *metadata_uuid;
if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
if (btrfs_super_magic(sb) == BTRFS_MAGIC_TEMPORARY) {
@@ -1377,11 +1399,16 @@ static int check_super(struct btrfs_super_block *sb, unsigned sbflags)
goto error_out;
}
- if (memcmp(sb->fsid, sb->dev_item.fsid, BTRFS_UUID_SIZE) != 0) {
+ if (btrfs_super_incompat_flags(sb) & BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
+ metadata_uuid = sb->metadata_uuid;
+ else
+ metadata_uuid = sb->fsid;
+
+ if (memcmp(metadata_uuid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) {
char fsid[BTRFS_UUID_UNPARSED_SIZE];
char dev_fsid[BTRFS_UUID_UNPARSED_SIZE];
- uuid_unparse(sb->fsid, fsid);
+ uuid_unparse(sb->metadata_uuid, fsid);
uuid_unparse(sb->dev_item.fsid, dev_fsid);
error("dev_item UUID does not match fsid: %s != %s",
dev_fsid, fsid);
@@ -1448,6 +1475,7 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr,
unsigned sbflags)
{
u8 fsid[BTRFS_FSID_SIZE];
+ u8 metadata_uuid[BTRFS_FSID_SIZE];
int fsid_is_initialized = 0;
char tmp[BTRFS_SUPER_INFO_SIZE];
struct btrfs_super_block *buf = (struct btrfs_super_block *)tmp;
@@ -1455,6 +1483,7 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr,
int ret;
int max_super = sbflags & SBREAD_RECOVER ? BTRFS_SUPER_MIRROR_MAX : 1;
u64 transid = 0;
+ bool metadata_uuid_set = false;
u64 bytenr;
if (sb_bytenr != BTRFS_SUPER_INFO_OFFSET) {
@@ -1499,9 +1528,18 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr,
continue;
if (!fsid_is_initialized) {
+ if (btrfs_super_incompat_flags(buf) &
+ BTRFS_FEATURE_INCOMPAT_METADATA_UUID) {
+ metadata_uuid_set = true;
+ memcpy(metadata_uuid, buf->metadata_uuid,
+ sizeof(metadata_uuid));
+ }
memcpy(fsid, buf->fsid, sizeof(fsid));
fsid_is_initialized = 1;
- } else if (memcmp(fsid, buf->fsid, sizeof(fsid))) {
+ } else if (memcmp(fsid, buf->fsid, sizeof(fsid)) ||
+ (metadata_uuid_set && memcmp(metadata_uuid,
+ buf->metadata_uuid,
+ sizeof(metadata_uuid)))) {
/*
* the superblocks (the original one and
* its backups) contain data of different
@@ -1602,7 +1640,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
btrfs_set_stack_device_io_width(dev_item, dev->io_width);
btrfs_set_stack_device_sector_size(dev_item, dev->sector_size);
memcpy(dev_item->uuid, dev->uuid, BTRFS_UUID_SIZE);
- memcpy(dev_item->fsid, dev->fs_devices->fsid, BTRFS_UUID_SIZE);
+ memcpy(dev_item->fsid, fs_info->metadata_uuid, BTRFS_FSID_SIZE);
flags = btrfs_super_flags(sb);
btrfs_set_super_flags(sb, flags | BTRFS_HEADER_FLAG_WRITTEN);
diff --git a/image/main.c b/image/main.c
index 351c5a256938..efab3c44fb23 100644
--- a/image/main.c
+++ b/image/main.c
@@ -1401,7 +1401,7 @@ static void *restore_worker(void *data)
list_del_init(&async->list);
if (mdres->compress_method == COMPRESS_ZLIB) {
- size = compress_size;
+ size = compress_size;
pthread_mutex_unlock(&mdres->mutex);
ret = uncompress(buffer, (unsigned long *)&size,
async->buffer, async->bufsize);
@@ -1593,9 +1593,12 @@ static int fill_mdres_info(struct mdrestore_struct *mdres,
super = (struct btrfs_super_block *)outbuf;
mdres->nodesize = btrfs_super_nodesize(super);
- memcpy(mdres->fsid, super->fsid, BTRFS_FSID_SIZE);
- memcpy(mdres->uuid, super->dev_item.uuid,
- BTRFS_UUID_SIZE);
+ if (btrfs_super_incompat_flags(super) &
+ BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
+ memcpy(mdres->fsid, super->metadata_uuid, BTRFS_FSID_SIZE);
+ else
+ memcpy(mdres->fsid, super->fsid, BTRFS_FSID_SIZE);
+ memcpy(mdres->uuid, super->dev_item.uuid, BTRFS_UUID_SIZE);
mdres->devid = le64_to_cpu(super->dev_item.devid);
free(buffer);
return 0;
@@ -1722,7 +1725,7 @@ static int read_chunk_block(struct mdrestore_struct *mdres, u8 *buffer,
if (memcmp(mdres->fsid, eb->data + offsetof(struct btrfs_header, fsid),
BTRFS_FSID_SIZE)) {
- error("filesystem UUID of eb %llu does not match",
+ error("filesystem metadata UUID of eb %llu does not match",
(unsigned long long)bytenr);
ret = -EIO;
goto out;
@@ -2036,9 +2039,13 @@ static int build_chunk_tree(struct mdrestore_struct *mdres,
super = (struct btrfs_super_block *)buffer;
chunk_root_bytenr = btrfs_super_chunk_root(super);
mdres->nodesize = btrfs_super_nodesize(super);
- memcpy(mdres->fsid, super->fsid, BTRFS_FSID_SIZE);
- memcpy(mdres->uuid, super->dev_item.uuid,
- BTRFS_UUID_SIZE);
+ if (btrfs_super_incompat_flags(super) &
+ BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
+ memcpy(mdres->fsid, super->metadata_uuid, BTRFS_FSID_SIZE);
+ else
+ memcpy(mdres->fsid, super->fsid, BTRFS_FSID_SIZE);
+
+ memcpy(mdres->uuid, super->dev_item.uuid, BTRFS_UUID_SIZE);
mdres->devid = le64_to_cpu(super->dev_item.devid);
free(buffer);
pthread_mutex_unlock(&mdres->mutex);
@@ -2314,7 +2321,7 @@ static int update_disk_super_on_device(struct btrfs_fs_info *info,
key.offset = cur_devid;
btrfs_init_path(&path);
- ret = btrfs_search_slot(NULL, info->chunk_root, &key, &path, 0, 0);
+ ret = btrfs_search_slot(NULL, info->chunk_root, &key, &path, 0, 0);
if (ret) {
error("search key failed: %d", ret);
ret = -EIO;
diff --git a/volumes.c b/volumes.c
index d81b348eb14d..b8388194c38f 100644
--- a/volumes.c
+++ b/volumes.c
@@ -142,13 +142,19 @@ static struct btrfs_device *find_device(struct btrfs_fs_devices *fs_devices,
return NULL;
}
-static struct btrfs_fs_devices *find_fsid(u8 *fsid)
+static struct btrfs_fs_devices *find_fsid(u8 *fsid, u8 *metadata_uuid)
{
struct btrfs_fs_devices *fs_devices;
list_for_each_entry(fs_devices, &fs_uuids, list) {
- if (memcmp(fsid, fs_devices->fsid, BTRFS_FSID_SIZE) == 0)
+ if (metadata_uuid && (memcmp(fsid, fs_devices->fsid,
+ BTRFS_FSID_SIZE) == 0) &&
+ (memcmp(metadata_uuid, fs_devices->metadata_uuid,
+ BTRFS_FSID_SIZE) == 0)) {
return fs_devices;
+ } else if (memcmp(fsid, fs_devices->fsid, BTRFS_FSID_SIZE) == 0){
+ return fs_devices;
+ }
}
return NULL;
}
@@ -160,8 +166,15 @@ static int device_list_add(const char *path,
struct btrfs_device *device;
struct btrfs_fs_devices *fs_devices;
u64 found_transid = btrfs_super_generation(disk_super);
+ bool metadata_uuid = (btrfs_super_incompat_flags(disk_super) &
+ BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
+
+ if (metadata_uuid)
+ fs_devices = find_fsid(disk_super->fsid,
+ disk_super->metadata_uuid);
+ else
+ fs_devices = find_fsid(disk_super->fsid, NULL);
- fs_devices = find_fsid(disk_super->fsid);
if (!fs_devices) {
fs_devices = kzalloc(sizeof(*fs_devices), GFP_NOFS);
if (!fs_devices)
@@ -169,6 +182,13 @@ static int device_list_add(const char *path,
INIT_LIST_HEAD(&fs_devices->devices);
list_add(&fs_devices->list, &fs_uuids);
memcpy(fs_devices->fsid, disk_super->fsid, BTRFS_FSID_SIZE);
+ if (metadata_uuid)
+ memcpy(fs_devices->metadata_uuid,
+ disk_super->metadata_uuid, BTRFS_FSID_SIZE);
+ else
+ memcpy(fs_devices->metadata_uuid, fs_devices->fsid,
+ BTRFS_FSID_SIZE);
+
fs_devices->latest_devid = devid;
fs_devices->latest_trans = found_transid;
fs_devices->lowest_devid = (u64)-1;
@@ -712,7 +732,7 @@ int btrfs_add_device(struct btrfs_trans_handle *trans,
ptr = (unsigned long)btrfs_device_uuid(dev_item);
write_extent_buffer(leaf, device->uuid, ptr, BTRFS_UUID_SIZE);
ptr = (unsigned long)btrfs_device_fsid(dev_item);
- write_extent_buffer(leaf, fs_info->fsid, ptr, BTRFS_UUID_SIZE);
+ write_extent_buffer(leaf, fs_info->metadata_uuid, ptr, BTRFS_UUID_SIZE);
btrfs_mark_buffer_dirty(leaf);
ret = 0;
@@ -1685,7 +1705,7 @@ struct btrfs_device *btrfs_find_device(struct btrfs_fs_info *fs_info, u64 devid,
cur_devices = fs_info->fs_devices;
while (cur_devices) {
if (!fsid ||
- (!memcmp(cur_devices->fsid, fsid, BTRFS_UUID_SIZE) ||
+ (!memcmp(cur_devices->metadata_uuid, fsid, BTRFS_FSID_SIZE) ||
fs_info->ignore_fsid_mismatch)) {
device = find_device(cur_devices, devid, uuid);
if (device)
@@ -1966,7 +1986,7 @@ static int open_seed_devices(struct btrfs_fs_info *fs_info, u8 *fsid)
fs_devices = fs_devices->seed;
}
- fs_devices = find_fsid(fsid);
+ fs_devices = find_fsid(fsid, NULL);
if (!fs_devices) {
/* missing all seed devices */
fs_devices = kzalloc(sizeof(*fs_devices), GFP_NOFS);
@@ -2005,7 +2025,7 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
BTRFS_UUID_SIZE);
read_extent_buffer(leaf, fs_uuid,
(unsigned long)btrfs_device_fsid(dev_item),
- BTRFS_UUID_SIZE);
+ BTRFS_FSID_SIZE);
if (memcmp(fs_uuid, fs_info->fsid, BTRFS_UUID_SIZE)) {
ret = open_seed_devices(fs_info, fs_uuid);
diff --git a/volumes.h b/volumes.h
index b4ea93f0bec3..f5732f6a4c0f 100644
--- a/volumes.h
+++ b/volumes.h
@@ -71,6 +71,7 @@ struct btrfs_device {
struct btrfs_fs_devices {
u8 fsid[BTRFS_FSID_SIZE]; /* FS specific uuid */
+ u8 metadata_uuid[BTRFS_FSID_SIZE]; /* FS specific uuid */
/* the device with this id has the most recent copy of the super */
u64 latest_devid;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] btrfstune: Add support for changing the user uuid
2018-08-29 8:35 [PATCH 0/4] Userspace support for FSID change Nikolay Borisov
2018-08-29 8:35 ` [PATCH 1/4] btrfs-progs: Add support for metadata_uuid field Nikolay Borisov
@ 2018-08-29 8:35 ` Nikolay Borisov
2018-08-29 8:35 ` [PATCH 3/4] btrfs-progs: tests: Add tests for changing fsid feature Nikolay Borisov
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2018-08-29 8:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
This allows us to change the use-visible UUID on filesytems from
userspace if desired, by copying the existing UUID to the new location
for metadata comparisons. If this is done, an incompat flag must be
set to prevent older filesystems from mounting the filesystem, but
the original UUID can be restored, and the incompat flag removed.
This introduces the new -m|-M UUID options similar to current
-u|-U UUID ones with the difference that we don't rewrite the fsid but
just copy the old uuid and set a new one. Additionally running with
[-M old-uuid] clears the incompat flag and retains only fsid on-disk.
Additionally it's not allowed to intermix -m/-u/-U/-M options in a
single invocation of btrfstune, nor is it allowed to change the uuid
while there is a uuid rewrite in-progress. Also changing the uuid of a
seed device is not currently allowed (can change in the future).
Example:
btrfstune -m /dev/loop1
btrfs inspect-internal dump-super /dev/loop1
superblock: bytenr=65536, device=/dev/loop1
---------------------------------------------------------
csum_type 0 (crc32c)
csum_size 4
csum 0x4b7ea749 [match]
<ommitted for brevity>
fsid 0efc41d3-4451-49f3-8108-7b8bdbcf5ae8
metadata_uuid 352715e7-62cf-4ae0-92ee-85a574adc318
<ommitted for brevity>
incompat_flags 0x541
( MIXED_BACKREF |
EXTENDED_IREF |
SKINNY_METADATA |
METADATA_UUID )
<omitted for brevity>
dev_item.uuid 0610deee-dfc3-498b-9449-a06533cdec98
dev_item.fsid 352715e7-62cf-4ae0-92ee-85a574adc318 [match]
<ommitted for brevity>
mount /dev/loop1 btrfs-mnt/
btrfs fi show btrfs-mnt/
Label: none uuid: 0efc41d3-4451-49f3-8108-7b8bdbcf5ae8
Total devices 1 FS bytes used 128.00KiB
devid 1 size 5.00GiB used 536.00MiB path /dev/loop1
In this case a new btrfs filesystem was created and the original uuid
was 352715e7-62cf-4ae0-92ee-85a574adc318, then btrfstune was run which
copied that value over to metadata_uuid field and set the current fsid
to 0efc41d3-4451-49f3-8108-7b8bdbcf5ae8. And as far as userspace is
concerned this is the fsid of the fs.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
btrfstune.c | 168 +++++++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 143 insertions(+), 25 deletions(-)
diff --git a/btrfstune.c b/btrfstune.c
index ea04159a81ce..697285ef3bf6 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -73,6 +73,101 @@ static int update_seeding_flag(struct btrfs_root *root, int set_flag)
return ret;
}
+/*
+ * Return 0 for no unfinished fsid change.
+ * Return >0 for unfinished fsid change, and restore unfinished fsid/
+ * chunk_tree_id into fsid_ret/chunk_id_ret.
+ */
+static int check_unfinished_fsid_change(struct btrfs_fs_info *fs_info,
+ uuid_t fsid_ret, uuid_t chunk_id_ret)
+{
+ struct btrfs_root *tree_root = fs_info->tree_root;
+ u64 flags = btrfs_super_flags(fs_info->super_copy);
+
+ if (flags & BTRFS_SUPER_FLAG_CHANGING_FSID) {
+ memcpy(fsid_ret, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
+ read_extent_buffer(tree_root->node, chunk_id_ret,
+ btrfs_header_chunk_tree_uuid(tree_root->node),
+ BTRFS_UUID_SIZE);
+ return 1;
+ }
+ return 0;
+}
+
+static int set_metadata_uuid(struct btrfs_root *root, const char *uuid_string)
+{
+ struct btrfs_super_block *disk_super;
+ uuid_t new_fsid, unused1, unused2;
+ struct btrfs_trans_handle *trans;
+ bool new_uuid = true;
+ u64 incompat_flags;
+ bool uuid_changed;
+ u64 super_flags;
+ int ret;
+
+ disk_super = root->fs_info->super_copy;
+ super_flags = btrfs_super_flags(disk_super);
+ incompat_flags = btrfs_super_incompat_flags(disk_super);
+ uuid_changed = incompat_flags & BTRFS_FEATURE_INCOMPAT_METADATA_UUID;
+
+ if (super_flags & BTRFS_SUPER_FLAG_SEEDING) {
+ fprintf(stderr, "Cannot set metadata UUID on a seed device\n");
+ return 1;
+ }
+
+ if (check_unfinished_fsid_change(root->fs_info, unused1, unused2)) {
+ fprintf(stderr, "UUID rewrite in progress, cannot change "
+ "fsid\n");
+ return 1;
+ }
+
+ if (uuid_string)
+ uuid_parse(uuid_string, new_fsid);
+ else
+ uuid_generate(new_fsid);
+
+ new_uuid = (memcmp(new_fsid, disk_super->fsid, BTRFS_FSID_SIZE) != 0);
+
+ if (new_uuid && uuid_changed && memcmp(disk_super->metadata_uuid,
+ new_fsid, BTRFS_FSID_SIZE) == 0) {
+ /*
+ * Changing fsid to be the same as metadata uuid, so just
+ * disable the flag
+ */
+ memcpy(disk_super->fsid, &new_fsid, BTRFS_FSID_SIZE);
+ incompat_flags &= ~BTRFS_FEATURE_INCOMPAT_METADATA_UUID;
+ btrfs_set_super_incompat_flags(disk_super, incompat_flags);
+ memset(disk_super->metadata_uuid, 0, BTRFS_FSID_SIZE);
+ } else if (new_uuid && uuid_changed && memcmp(disk_super->metadata_uuid,
+ new_fsid, BTRFS_FSID_SIZE)) {
+ /*
+ * Changing fsid on an already changed FS, in this case we
+ * only change the fsid and don't touch metadata uuid as it
+ * has already the correct value
+ */
+ memcpy(disk_super->fsid, &new_fsid, BTRFS_FSID_SIZE);
+ } else if (new_uuid && !uuid_changed) {
+ /*
+ * First time changing the fsid, copy the fsid to
+ * metadata_uuid
+ */
+ incompat_flags |= BTRFS_FEATURE_INCOMPAT_METADATA_UUID;
+ btrfs_set_super_incompat_flags(disk_super, incompat_flags);
+ memcpy(disk_super->metadata_uuid, disk_super->fsid,
+ BTRFS_FSID_SIZE);
+ memcpy(disk_super->fsid, &new_fsid, BTRFS_FSID_SIZE);
+ } else {
+ /* Setting the same fsid as current NOOP */
+ return 0;
+ }
+
+ trans = btrfs_start_transaction(root, 1);
+ BUG_ON(IS_ERR(trans));
+ ret = btrfs_commit_transaction(trans, root);
+
+ return ret;
+}
+
static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
{
struct btrfs_trans_handle *trans;
@@ -268,26 +363,6 @@ static int change_fsid_done(struct btrfs_fs_info *fs_info)
return write_all_supers(fs_info);
}
-/*
- * Return 0 for no unfinished fsid change.
- * Return >0 for unfinished fsid change, and restore unfinished fsid/
- * chunk_tree_id into fsid_ret/chunk_id_ret.
- */
-static int check_unfinished_fsid_change(struct btrfs_fs_info *fs_info,
- uuid_t fsid_ret, uuid_t chunk_id_ret)
-{
- struct btrfs_root *tree_root = fs_info->tree_root;
- u64 flags = btrfs_super_flags(fs_info->super_copy);
-
- if (flags & BTRFS_SUPER_FLAG_CHANGING_FSID) {
- memcpy(fsid_ret, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
- read_extent_buffer(tree_root->node, chunk_id_ret,
- btrfs_header_chunk_tree_uuid(tree_root->node),
- BTRFS_UUID_SIZE);
- return 1;
- }
- return 0;
-}
/*
* Change fsid of a given fs.
@@ -381,8 +456,10 @@ static void print_usage(void)
printf("\t-x \t\tenable skinny metadata extent refs\n");
printf("\t-n \t\tenable no-holes feature (more efficient sparse file representation)\n");
printf("\t-f \t\tforce to do dangerous operation, make sure that you are aware of the dangers\n");
- printf("\t-u \t\tchange fsid, use a random one\n");
- printf("\t-U UUID\t\tchange fsid to UUID\n");
+ printf("\t-u \t\trewrite fsid, use a random one\n");
+ printf("\t-U UUID\t\trewrite fsid to UUID\n");
+ printf("\t-m \t\tChange only user-facing uuid (lighterweight than -u|-U\n");
+ printf("\t-M UUID\t\tchange fsid to UUID\n");
}
int main(int argc, char *argv[])
@@ -394,6 +471,7 @@ int main(int argc, char *argv[])
int seeding_flag = 0;
u64 seeding_value = 0;
int random_fsid = 0;
+ int change_metadata_uuid = 0;
char *new_fsid_str = NULL;
int ret;
u64 super_flags = 0;
@@ -404,7 +482,7 @@ int main(int argc, char *argv[])
{ "help", no_argument, NULL, GETOPT_VAL_HELP},
{ NULL, 0, NULL, 0 }
};
- int c = getopt_long(argc, argv, "S:rxfuU:n", long_options, NULL);
+ int c = getopt_long(argc, argv, "S:rxfuU:nmM:", long_options, NULL);
if (c < 0)
break;
@@ -433,6 +511,15 @@ int main(int argc, char *argv[])
ctree_flags |= OPEN_CTREE_IGNORE_FSID_MISMATCH;
random_fsid = 1;
break;
+ case 'M':
+ ctree_flags |= OPEN_CTREE_IGNORE_FSID_MISMATCH;
+ change_metadata_uuid = 1;
+ new_fsid_str = optarg;
+ break;
+ case 'm':
+ ctree_flags |= OPEN_CTREE_IGNORE_FSID_MISMATCH;
+ change_metadata_uuid = 1;
+ break;
case GETOPT_VAL_HELP:
default:
print_usage();
@@ -451,7 +538,8 @@ int main(int argc, char *argv[])
error("random fsid can't be used with specified fsid");
return 1;
}
- if (!super_flags && !seeding_flag && !(random_fsid || new_fsid_str)) {
+ if (!super_flags && !seeding_flag && !(random_fsid || new_fsid_str) &&
+ !change_metadata_uuid) {
error("at least one option should be specified");
print_usage();
return 1;
@@ -497,6 +585,12 @@ int main(int argc, char *argv[])
}
if (seeding_flag) {
+ if (btrfs_fs_incompat(root->fs_info, METADATA_UUID)) {
+ fprintf(stderr, "SEED flag cannot be changed on a metadata-uuid changed fs\n");
+ ret = 1;
+ goto out;
+ }
+
if (!seeding_value && !force) {
warning(
"this is dangerous, clearing the seeding flag may cause the derived device not to be mountable!");
@@ -521,7 +615,31 @@ int main(int argc, char *argv[])
total++;
}
- if (random_fsid || new_fsid_str) {
+ if (change_metadata_uuid) {
+ if (seeding_flag) {
+ fprintf(stderr, "Not allowed to set both seeding flag and uuid metadata\n");
+ ret = 1;
+ goto out;
+ }
+
+ if (new_fsid_str)
+ ret = set_metadata_uuid(root, new_fsid_str);
+ else
+ ret = set_metadata_uuid(root, NULL);
+
+ if (!ret)
+ success++;
+ total++;
+ }
+
+ if (random_fsid || (new_fsid_str && !change_metadata_uuid)) {
+ if (btrfs_fs_incompat(root->fs_info, METADATA_UUID)) {
+ fprintf(stderr, "Cannot rewrite fsid while METADATA_UUID flag is active."
+ "Ensure fsid and metadata_uuid match before retrying.\n");
+ ret = 1;
+ goto out;
+ }
+
if (!force) {
warning(
"it's highly recommended to run 'btrfs check' before this operation");
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] btrfs-progs: tests: Add tests for changing fsid feature
2018-08-29 8:35 [PATCH 0/4] Userspace support for FSID change Nikolay Borisov
2018-08-29 8:35 ` [PATCH 1/4] btrfs-progs: Add support for metadata_uuid field Nikolay Borisov
2018-08-29 8:35 ` [PATCH 2/4] btrfstune: Add support for changing the user uuid Nikolay Borisov
@ 2018-08-29 8:35 ` Nikolay Borisov
2018-08-29 8:35 ` [PATCH 4/4] btrfs-progs: Remove fsid/metdata_uuid fields from fs_info Nikolay Borisov
2018-08-29 12:09 ` [PATCH 0/4] Userspace support for FSID change Qu Wenruo
4 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2018-08-29 8:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Add a bunch of tests exercising the new btrfstune functionality. In
particular check that various restrictions are implemented correctly,
test that btrfs-image works as expected and also test the output of
btrfs inspect-internal dump-super is correct.
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++++++++
1 file changed, 137 insertions(+)
create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
diff --git a/tests/misc-tests/033-metadata-uuid/test.sh b/tests/misc-tests/033-metadata-uuid/test.sh
new file mode 100755
index 000000000000..b08220c257c8
--- /dev/null
+++ b/tests/misc-tests/033-metadata-uuid/test.sh
@@ -0,0 +1,137 @@
+#!/bin/bash
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+check_prereq btrfstune
+check_prereq btrfs-image
+
+setup_root_helper
+
+prepare_test_dev
+
+function read_fsid {
+ local dev="$1"
+ echo $(run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal \
+ dump-super "$dev" | awk '/fsid/ {print $2}' | head -n 1)
+}
+
+function read_metadata_uuid {
+ local dev="$1"
+ echo $(run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal \
+ dump-super "$dev" | awk '/metadata_uuid/ {print $2}')
+}
+
+function check_btrfstune {
+ local fsid
+ echo "Checking btrfstune logic" >> "$RESULTS"
+ #Test with random uuid
+ run_check $SUDO_HELPER "$TOP/btrfstune" -m "$TEST_DEV"
+
+ #check that specific uuid can set
+ run_check $SUDO_HELPER "$TOP/btrfstune" -M d88c8333-a652-4476-b225-2e9284eb59f1 "$TEST_DEV"
+
+ #test that having seed on already changed device doesn't work
+ run_mustfail "Managed to set seed on metadata uuid fs" \
+ $SUDO_HELPER "$TOP/btrfstune" -S 1 "$TEST_DEV"
+
+ #test that setting both seed and -m|M is forbidden
+ run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+ run_mustfail "Succeeded setting seed and changing fs uuid" \
+ $SUDO_HELPER "$TOP/btrfstune" -S 1 -m "$TEST_DEV"
+
+ #test that having -m|-M on seed device is forbidden
+ run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+ run_check $SUDO_HELPER "$TOP/btrfstune" -S 1 "$TEST_DEV"
+ run_mustfail "Succeded changing fsid on a seed device" $SUDO_HELPER "$TOP/btrfstune" -m "$TEST_DEV"
+
+ #test that using -U|-u on an fs with METADATA_UUID flag is forbidden
+ run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+ run_check $SUDO_HELPER "$TOP/btrfstune" -m "$TEST_DEV"
+ run_mustfail "Succeeded triggering FSID rewrite while METADATA_UUID is active" \
+ $SUDO_HELPER "$TOP/btrfstune" -u "$TEST_DEV"
+
+}
+
+function check_dump_super_output {
+ local fsid
+ local metadata_uuid
+ local dev_item_match
+ local old_metadata_uuid
+
+ echo "Checking dump-super output" >> "$RESULTS"
+ #Assert that metadata/fsid match on non-changed fs
+ fsid=$(read_fsid "$TEST_DEV")
+ metadata_uuid=$(read_metadata_uuid "$TEST_DEV")
+ [ "$fsid" = "$metadata_uuid" ] || _fail "fsid ("$fsid") doesn't match metadata_uuid ("$metadata_uuid")"
+
+ dev_item_match=$(run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super \
+ "$TEST_DEV" | awk '/dev_item.fsid/ {print $3}')
+
+ [ $dev_item_match = "[match]" ] || _fail "dev_item.fsid doesn't match on non-metadata uuid fs"
+
+
+ echo "Checking output after fsid change" >> "$RESULTS"
+ #change metadatauuid and ensure everything in the output is still correct
+ old_metadata_uuid=$metadata_uuid
+ run_check $SUDO_HELPER "$TOP/btrfstune" -M d88c8333-a652-4476-b225-2e9284eb59f1 "$TEST_DEV"
+ fsid=$(read_fsid "$TEST_DEV")
+ metadata_uuid=$(read_metadata_uuid "$TEST_DEV")
+ dev_item_match=$(run_check_stdout $SUDO_HELPER "$TOP/btrfs" \
+ inspect-internal dump-super "$TEST_DEV" | awk '/dev_item.fsid/ {print $3}')
+
+ [ "$dev_item_match" = "[match]" ] || _fail "dev_item.fsid doesn't match on metadata uuid fs"
+ [ "$fsid" = "d88c8333-a652-4476-b225-2e9284eb59f1" ] || _fail "btrfstune metadata UUID change failed"
+ [ "$old_metadata_uuid" = "$metadata_uuid" ] || _fail "Metadata uuid change unexpectedly"
+
+ echo "Checking for incompat textual representation" >> "$RESULTS"
+ #check for textual output of the new incompat feature
+ run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super \
+ "$TEST_DEV" | grep -q METADATA_UUID
+ [ $? -eq 0 ] || _fail "Didn't find textual representation of METADATA_UUID feature"
+
+ echo "Checking setting fsid back to original" >> "$RESULTS"
+ #ensure that setting the fsid back to the original works
+ run_check $SUDO_HELPER "$TOP/btrfstune" -M "$old_metadata_uuid" "$TEST_DEV"
+
+ fsid=$(read_fsid "$TEST_DEV")
+ metadata_uuid=$(read_metadata_uuid "$TEST_DEV")
+
+ [ "$fsid" = "$metadata_uuid" ] || _fail "FSID and METADATA_UUID don't match"
+ run_check_stdout $SUDO_HELPER "$TOP/btrfs" inspect-internal dump-super \
+ "$TEST_DEV" | grep -q METADATA_UUID
+ [ $? -eq 1 ] || _fail "METADATA_UUID feature still shown as enabled"
+}
+
+function check_image_restore {
+ local metadata_uuid
+ local fsid
+ local fsid_restored
+ local metadata_uuid_restored
+
+ echo "TESTING btrfs-image restore" >> "$RESULTS"
+ run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+ run_check $SUDO_HELPER "$TOP/btrfstune" -m "$TEST_DEV"
+ fsid=$(read_fsid "$TEST_DEV")
+ metadata_uuid=$(read_metadata_uuid "$TEST_DEV")
+ run_mayfail $SUDO_HELPER "$TOP/btrfs-image" "$TEST_DEV" /tmp/test-img.dump
+ # erase the fs by creating a new one, wipefs is not sufficient as it just
+ # deletes the fs magic string
+ run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+ run_check $SUDO_HELPER "$TOP/btrfs-image" -r /tmp/test-img.dump "$TEST_DEV"
+ fsid_restored=$(read_fsid "$TEST_DEV")
+ metadata_uuid_restored=$(read_metadata_uuid "$TEST_DEV")
+
+ [ "$fsid" = "$fsid_restored" ] || _faild "FSID don't match after restore"
+ [ "$metadata_uuid" = "$metadata_uuid_restored" ] || _faild "metadata uuids don't match after restore"
+}
+
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+check_btrfstune
+
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+check_dump_super_output
+
+run_check $SUDO_HELPER "$TOP/mkfs.btrfs" -f "$TEST_DEV"
+check_image_restore
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
2018-08-29 8:35 [PATCH 0/4] Userspace support for FSID change Nikolay Borisov
` (2 preceding siblings ...)
2018-08-29 8:35 ` [PATCH 3/4] btrfs-progs: tests: Add tests for changing fsid feature Nikolay Borisov
@ 2018-08-29 8:35 ` Nikolay Borisov
2018-08-29 12:09 ` [PATCH 0/4] Userspace support for FSID change Qu Wenruo
4 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2018-08-29 8:35 UTC (permalink / raw)
To: linux-btrfs; +Cc: Nikolay Borisov
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
btrfstune.c | 5 +++--
check/main.c | 2 +-
chunk-recover.c | 11 +++++------
ctree.c | 11 ++++++-----
ctree.h | 2 --
disk-io.c | 18 +++++++++---------
volumes.c | 5 +++--
7 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/btrfstune.c b/btrfstune.c
index 697285ef3bf6..98e366a3b3ce 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -344,7 +344,8 @@ static int change_fsid_prepare(struct btrfs_fs_info *fs_info)
return ret;
/* Also need to change the metadatauuid of the fs info */
- memcpy(fs_info->metadata_uuid, fs_info->new_fsid, BTRFS_FSID_SIZE);
+ memcpy(fs_info->fs_devices->metadata_uuid, fs_info->new_fsid,
+ BTRFS_FSID_SIZE);
/* also restore new chunk_tree_id into tree_root for restore */
write_extent_buffer(tree_root->node, fs_info->new_chunk_tree_uuid,
@@ -401,7 +402,7 @@ static int change_uuid(struct btrfs_fs_info *fs_info, const char *new_fsid_str)
fs_info->new_fsid = new_fsid;
fs_info->new_chunk_tree_uuid = new_chunk_id;
- memcpy(old_fsid, (const char*)fs_info->fsid, BTRFS_UUID_SIZE);
+ memcpy(old_fsid, (const char*)fs_info->fs_devices->fsid, BTRFS_UUID_SIZE);
uuid_unparse(old_fsid, uuid_buf);
printf("Current fsid: %s\n", uuid_buf);
diff --git a/check/main.c b/check/main.c
index 0790264190f2..0aede2742dcf 100644
--- a/check/main.c
+++ b/check/main.c
@@ -8418,7 +8418,7 @@ static int btrfs_fsck_reinit_root(struct btrfs_trans_handle *trans,
btrfs_set_header_backref_rev(c, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(c, root->root_key.objectid);
- write_extent_buffer(c, root->fs_info->metadata_uuid,
+ write_extent_buffer(c, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(c, root->fs_info->chunk_tree_uuid,
diff --git a/chunk-recover.c b/chunk-recover.c
index 31325bfc54e0..959c169f79a4 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -1155,7 +1155,7 @@ static int __rebuild_chunk_root(struct btrfs_trans_handle *trans,
btrfs_set_header_level(cow, 0);
btrfs_set_header_backref_rev(cow, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(cow, BTRFS_CHUNK_TREE_OBJECTID);
- write_extent_buffer(cow, root->fs_info->metadata_uuid,
+ write_extent_buffer(cow, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(cow, root->fs_info->chunk_tree_uuid,
@@ -1457,7 +1457,7 @@ open_ctree_with_broken_chunk(struct recover_control *rc)
goto out_devices;
}
- memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
+ ASSERT(!memcmp(disk_super->fsid, rc->fs_devices->fsid, BTRFS_FSID_SIZE));
fs_info->sectorsize = btrfs_super_sectorsize(disk_super);
fs_info->nodesize = btrfs_super_nodesize(disk_super);
fs_info->stripesize = btrfs_super_stripesize(disk_super);
@@ -1469,10 +1469,9 @@ open_ctree_with_broken_chunk(struct recover_control *rc)
features = btrfs_super_incompat_flags(disk_super);
if (features & BTRFS_FEATURE_INCOMPAT_METADATA_UUID)
- memcpy(fs_info->metadata_uuid, disk_super->metadata_uuid,
- BTRFS_FSID_SIZE);
- else
- memcpy(fs_info->metadata_uuid, fs_info->fsid, BTRFS_FSID_SIZE);
+ ASSERT(!memcmp(disk_super->metadata_uuid,
+ fs_info->fs_devices->metadata_uuid,
+ BTRFS_FSID_SIZE));
btrfs_setup_root(fs_info->chunk_root, fs_info,
BTRFS_CHUNK_TREE_OBJECTID);
diff --git a/ctree.c b/ctree.c
index 883c2ae1861d..c79837e0206f 100644
--- a/ctree.c
+++ b/ctree.c
@@ -23,6 +23,7 @@
#include "internal.h"
#include "sizes.h"
#include "messages.h"
+#include "volumes.h"
static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
*root, struct btrfs_path *path, int level);
@@ -134,7 +135,7 @@ int btrfs_copy_root(struct btrfs_trans_handle *trans,
else
btrfs_set_header_owner(cow, new_root_objectid);
- write_extent_buffer(cow, root->fs_info->metadata_uuid,
+ write_extent_buffer(cow, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
WARN_ON(btrfs_header_generation(buf) > trans->transid);
@@ -308,7 +309,7 @@ int __btrfs_cow_block(struct btrfs_trans_handle *trans,
else
btrfs_set_header_owner(cow, root->root_key.objectid);
- write_extent_buffer(cow, root->fs_info->metadata_uuid,
+ write_extent_buffer(cow, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
WARN_ON(!(buf->flags & EXTENT_BAD_TRANSID) &&
@@ -1474,7 +1475,7 @@ static int noinline insert_new_root(struct btrfs_trans_handle *trans,
root_add_used(root, root->fs_info->nodesize);
- write_extent_buffer(c, root->fs_info->metadata_uuid,
+ write_extent_buffer(c, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(c, root->fs_info->chunk_tree_uuid,
@@ -1595,7 +1596,7 @@ static int split_node(struct btrfs_trans_handle *trans, struct btrfs_root
btrfs_set_header_generation(split, trans->transid);
btrfs_set_header_backref_rev(split, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(split, root->root_key.objectid);
- write_extent_buffer(split, root->fs_info->metadata_uuid,
+ write_extent_buffer(split, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(split, root->fs_info->chunk_tree_uuid,
btrfs_header_chunk_tree_uuid(split),
@@ -2177,7 +2178,7 @@ static noinline int split_leaf(struct btrfs_trans_handle *trans,
btrfs_set_header_backref_rev(right, BTRFS_MIXED_BACKREF_REV);
btrfs_set_header_owner(right, root->root_key.objectid);
btrfs_set_header_level(right, 0);
- write_extent_buffer(right, root->fs_info->metadata_uuid,
+ write_extent_buffer(right, root->fs_info->fs_devices->metadata_uuid,
btrfs_header_fsid(), BTRFS_FSID_SIZE);
write_extent_buffer(right, root->fs_info->chunk_tree_uuid,
diff --git a/ctree.h b/ctree.h
index c22d36b1871b..751fc3f9f766 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1079,8 +1079,6 @@ struct btrfs_block_group_cache {
struct btrfs_device;
struct btrfs_fs_devices;
struct btrfs_fs_info {
- u8 fsid[BTRFS_FSID_SIZE];
- u8 metadata_uuid[BTRFS_FSID_SIZE];
u8 *new_fsid;
u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
u8 *new_chunk_tree_uuid;
diff --git a/disk-io.c b/disk-io.c
index c3be4a1017b7..e17fed13302b 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -118,7 +118,7 @@ static void print_tree_block_error(struct btrfs_fs_info *fs_info,
read_extent_buffer(eb, buf, btrfs_header_fsid(),
BTRFS_UUID_SIZE);
uuid_unparse(buf, found_uuid);
- uuid_unparse(fs_info->metadata_uuid, fs_uuid);
+ uuid_unparse(fs_info->fs_devices->metadata_uuid, fs_uuid);
fprintf(stderr, "fsid mismatch, want=%s, have=%s\n",
fs_uuid, found_uuid);
break;
@@ -1183,13 +1183,12 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
goto out_devices;
}
- memcpy(fs_info->fsid, &disk_super->fsid, BTRFS_FSID_SIZE);
- if (btrfs_fs_incompat(fs_info, METADATA_UUID)) {
- memcpy(fs_info->metadata_uuid, disk_super->metadata_uuid,
- BTRFS_FSID_SIZE);
- } else {
- memcpy(fs_info->metadata_uuid, fs_info->fsid, BTRFS_FSID_SIZE);
- }
+ ASSERT(!memcmp(disk_super->fsid, fs_devices->fsid, BTRFS_FSID_SIZE));
+ ASSERT(!memcmp(disk_super->fsid, fs_devices->fsid, BTRFS_FSID_SIZE));
+ if (btrfs_fs_incompat(fs_info, METADATA_UUID))
+ ASSERT(!memcmp(disk_super->metadata_uuid,
+ fs_devices->metadata_uuid, BTRFS_FSID_SIZE));
+
fs_info->sectorsize = btrfs_super_sectorsize(disk_super);
fs_info->nodesize = btrfs_super_nodesize(disk_super);
fs_info->stripesize = btrfs_super_stripesize(disk_super);
@@ -1640,7 +1639,8 @@ int write_all_supers(struct btrfs_fs_info *fs_info)
btrfs_set_stack_device_io_width(dev_item, dev->io_width);
btrfs_set_stack_device_sector_size(dev_item, dev->sector_size);
memcpy(dev_item->uuid, dev->uuid, BTRFS_UUID_SIZE);
- memcpy(dev_item->fsid, fs_info->metadata_uuid, BTRFS_FSID_SIZE);
+ memcpy(dev_item->fsid, fs_info->fs_devices->metadata_uuid,
+ BTRFS_FSID_SIZE);
flags = btrfs_super_flags(sb);
btrfs_set_super_flags(sb, flags | BTRFS_HEADER_FLAG_WRITTEN);
diff --git a/volumes.c b/volumes.c
index b8388194c38f..e13f120e2296 100644
--- a/volumes.c
+++ b/volumes.c
@@ -732,7 +732,8 @@ int btrfs_add_device(struct btrfs_trans_handle *trans,
ptr = (unsigned long)btrfs_device_uuid(dev_item);
write_extent_buffer(leaf, device->uuid, ptr, BTRFS_UUID_SIZE);
ptr = (unsigned long)btrfs_device_fsid(dev_item);
- write_extent_buffer(leaf, fs_info->metadata_uuid, ptr, BTRFS_UUID_SIZE);
+ write_extent_buffer(leaf, fs_info->fs_devices->metadata_uuid, ptr,
+ BTRFS_UUID_SIZE);
btrfs_mark_buffer_dirty(leaf);
ret = 0;
@@ -2027,7 +2028,7 @@ static int read_one_dev(struct btrfs_fs_info *fs_info,
(unsigned long)btrfs_device_fsid(dev_item),
BTRFS_FSID_SIZE);
- if (memcmp(fs_uuid, fs_info->fsid, BTRFS_UUID_SIZE)) {
+ if (memcmp(fs_uuid, fs_info->fs_devices->fsid, BTRFS_UUID_SIZE)) {
ret = open_seed_devices(fs_info, fs_uuid);
if (ret)
return ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Userspace support for FSID change
2018-08-29 8:35 [PATCH 0/4] Userspace support for FSID change Nikolay Borisov
` (3 preceding siblings ...)
2018-08-29 8:35 ` [PATCH 4/4] btrfs-progs: Remove fsid/metdata_uuid fields from fs_info Nikolay Borisov
@ 2018-08-29 12:09 ` Qu Wenruo
2018-08-29 12:33 ` Nikolay Borisov
4 siblings, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2018-08-29 12:09 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2018/8/29 下午4:35, Nikolay Borisov wrote:
> Here is the userspace tooling support for utilising the new metadata_uuid field,
> enabling the change of fsid without having to rewrite every metadata block. This
> patchset consists of adding support for the new field to various tools and
> files (Patch 1). The actual implementation of the new -m|-M options (which are
> described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
> exercises the new options and verifies certain invariants hold (these are also
> described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
> structures.
So to my understand, now we have another layer of UUID.
Before we have one fsid, both used in superblock and tree blocks.
Now we have 2 fsid, the one used in tree blocks are kept the same, but
changed its name to metadata_uuid in superblock.
And superblock::fsid will become a new field, and although they are the
same at mkfs time, they could change several times during its operation.
This indeed makes uuid change super fast, only needs to update all
superblocks of the fs, instead of all tree blocks.
However I have one nitpick of the design. Unlike XFS, btrfs supports
multiple devices.
If we have a raid10 fs with 4 devices, and it has already gone through
several UUID change (so its metadata uuid is already different from fsid).
And during another UUID change procedure, we lost power while only
updated 2 super blocks, what will happen for kernel device assembly?
(Although considering how fast the UUID change would happen, such case
should be super niche)
>
> The intended usecase of this feature is to give the sysadmin the ability to
> create copies of filesystesm, change their uuid quickly and mount them alongside
> the original filesystem for, say, forensic purposes.
>
> One thing which still hasn't been set in stone is whether the new options
> will remain as -m|-M or whether they should subsume the current -u|-U - from
> the point of view of users nothing should change.
Well, user would be surprised by how fast the new -m is, thus there is
still something changed :)
I prefer to subsume current -u/-U, and use the new one if the incompat
feature is already set. Or fall back to original behavior.
But I'm not a fan of using INCOMPAT flags as an indicator of changed
fsid/metadata uuid.
INCOMPAT feature should not change so easily nor acts as an indicator.
That's to say, the flag should only be set at mkfs time, and then never
change unlike the 2nd patch (I don't even like btrfstune to change
incompat flags).
E.g.
mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
change fsid without touching metadata uuid.
Or we could only use the old method.
Thanks,
Qu
> So this is something which
> I'd like to hear from the community. Of course the alternative of rewriting
> the metadata blocks will be assigne new options - perhaps -m|M ?
>
> I've tested this with multiple xfstest runs with the new tools installed as
> well as running btrfs-progs test and have observed no regressions.
>
> Nikolay Borisov (4):
> btrfs-progs: Add support for metadata_uuid field.
> btrfstune: Add support for changing the user uuid
> btrfs-progs: tests: Add tests for changing fsid feature
> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>
> btrfstune.c | 174 ++++++++++++++++++++++++-----
> check/main.c | 2 +-
> chunk-recover.c | 17 ++-
> cmds-filesystem.c | 2 +
> cmds-inspect-dump-super.c | 22 +++-
> convert/common.c | 2 +
> ctree.c | 15 +--
> ctree.h | 8 +-
> disk-io.c | 62 ++++++++--
> image/main.c | 25 +++--
> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
> volumes.c | 37 ++++--
> volumes.h | 1 +
> 13 files changed, 431 insertions(+), 73 deletions(-)
> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Userspace support for FSID change
2018-08-29 12:09 ` [PATCH 0/4] Userspace support for FSID change Qu Wenruo
@ 2018-08-29 12:33 ` Nikolay Borisov
2018-08-29 12:43 ` Austin S. Hemmelgarn
2018-08-29 12:47 ` Qu Wenruo
0 siblings, 2 replies; 11+ messages in thread
From: Nikolay Borisov @ 2018-08-29 12:33 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 29.08.2018 15:09, Qu Wenruo wrote:
>
>
> On 2018/8/29 下午4:35, Nikolay Borisov wrote:
>> Here is the userspace tooling support for utilising the new metadata_uuid field,
>> enabling the change of fsid without having to rewrite every metadata block. This
>> patchset consists of adding support for the new field to various tools and
>> files (Patch 1). The actual implementation of the new -m|-M options (which are
>> described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
>> exercises the new options and verifies certain invariants hold (these are also
>> described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
>> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
>> structures.
>
> So to my understand, now we have another layer of UUID.
>
> Before we have one fsid, both used in superblock and tree blocks.
>
> Now we have 2 fsid, the one used in tree blocks are kept the same, but
> changed its name to metadata_uuid in superblock.
> And superblock::fsid will become a new field, and although they are the
> same at mkfs time, they could change several times during its operation.
>
> This indeed makes uuid change super fast, only needs to update all
> superblocks of the fs, instead of all tree blocks.
>
> However I have one nitpick of the design. Unlike XFS, btrfs supports
> multiple devices.
> If we have a raid10 fs with 4 devices, and it has already gone through
> several UUID change (so its metadata uuid is already different from fsid).
>
> And during another UUID change procedure, we lost power while only
> updated 2 super blocks, what will happen for kernel device assembly?
>
> (Although considering how fast the UUID change would happen, such case
> should be super niche)
Then I guess you will be fucked. I'm all ears for suggestion how to
rectify this without skyrocketing the complexity. The current UUID
rewrite method sets a flag int he superblock that FSID change is in
progress and clears it once every metadatablock has been rewritten. I
can piggyback on this mechanism but I'm not sure it provides 100%
guarantee. Because by the some token you can set this flag, start
writing the super blocks then lose power and then only some of the
superblocks could have this flag set so we back at square 1.
>
>>
>> The intended usecase of this feature is to give the sysadmin the ability to
>> create copies of filesystesm, change their uuid quickly and mount them alongside
>> the original filesystem for, say, forensic purposes.
>>
>> One thing which still hasn't been set in stone is whether the new options
>> will remain as -m|-M or whether they should subsume the current -u|-U - from
>> the point of view of users nothing should change.
>
> Well, user would be surprised by how fast the new -m is, thus there is
> still something changed :)
>
> I prefer to subsume current -u/-U, and use the new one if the incompat
> feature is already set. Or fall back to original behavior.
>
> But I'm not a fan of using INCOMPAT flags as an indicator of changed
> fsid/metadata uuid.
> INCOMPAT feature should not change so easily nor acts as an indicator.
>
> That's to say, the flag should only be set at mkfs time, and then never
> change unlike the 2nd patch (I don't even like btrfstune to change
> incompat flags).
>
> E.g.
> mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
> change fsid without touching metadata uuid.
> Or we could only use the old method.
I disagree, I don't see any benefit in this but only added complexity.
Can you elaborate more ?
>
> Thanks,
> Qu
>
>> So this is something which
>> I'd like to hear from the community. Of course the alternative of rewriting
>> the metadata blocks will be assigne new options - perhaps -m|M ?
>>
>> I've tested this with multiple xfstest runs with the new tools installed as
>> well as running btrfs-progs test and have observed no regressions.
>>
>> Nikolay Borisov (4):
>> btrfs-progs: Add support for metadata_uuid field.
>> btrfstune: Add support for changing the user uuid
>> btrfs-progs: tests: Add tests for changing fsid feature
>> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>>
>> btrfstune.c | 174 ++++++++++++++++++++++++-----
>> check/main.c | 2 +-
>> chunk-recover.c | 17 ++-
>> cmds-filesystem.c | 2 +
>> cmds-inspect-dump-super.c | 22 +++-
>> convert/common.c | 2 +
>> ctree.c | 15 +--
>> ctree.h | 8 +-
>> disk-io.c | 62 ++++++++--
>> image/main.c | 25 +++--
>> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
>> volumes.c | 37 ++++--
>> volumes.h | 1 +
>> 13 files changed, 431 insertions(+), 73 deletions(-)
>> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Userspace support for FSID change
2018-08-29 12:33 ` Nikolay Borisov
@ 2018-08-29 12:43 ` Austin S. Hemmelgarn
2018-08-29 12:47 ` Qu Wenruo
1 sibling, 0 replies; 11+ messages in thread
From: Austin S. Hemmelgarn @ 2018-08-29 12:43 UTC (permalink / raw)
To: Nikolay Borisov, Qu Wenruo, linux-btrfs
On 2018-08-29 08:33, Nikolay Borisov wrote:
>
>
> On 29.08.2018 15:09, Qu Wenruo wrote:
>>
>>
>> On 2018/8/29 下午4:35, Nikolay Borisov wrote:
>>> Here is the userspace tooling support for utilising the new metadata_uuid field,
>>> enabling the change of fsid without having to rewrite every metadata block. This
>>> patchset consists of adding support for the new field to various tools and
>>> files (Patch 1). The actual implementation of the new -m|-M options (which are
>>> described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
>>> exercises the new options and verifies certain invariants hold (these are also
>>> described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
>>> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
>>> structures.
>>
>> So to my understand, now we have another layer of UUID.
>>
>> Before we have one fsid, both used in superblock and tree blocks.
>>
>> Now we have 2 fsid, the one used in tree blocks are kept the same, but
>> changed its name to metadata_uuid in superblock.
>> And superblock::fsid will become a new field, and although they are the
>> same at mkfs time, they could change several times during its operation.
>>
>> This indeed makes uuid change super fast, only needs to update all
>> superblocks of the fs, instead of all tree blocks.
>>
>> However I have one nitpick of the design. Unlike XFS, btrfs supports
>> multiple devices.
>> If we have a raid10 fs with 4 devices, and it has already gone through
>> several UUID change (so its metadata uuid is already different from fsid).
>>
>> And during another UUID change procedure, we lost power while only
>> updated 2 super blocks, what will happen for kernel device assembly?
>>
>> (Although considering how fast the UUID change would happen, such case
>> should be super niche)
>
> Then I guess you will be fucked. I'm all ears for suggestion how to
> rectify this without skyrocketing the complexity. The current UUID
> rewrite method sets a flag int he superblock that FSID change is in
> progress and clears it once every metadatablock has been rewritten. I
> can piggyback on this mechanism but I'm not sure it provides 100%
> guarantee. Because by the some token you can set this flag, start
> writing the super blocks then lose power and then only some of the
> superblocks could have this flag set so we back at square 1.
>
>>
>>>
>>> The intended usecase of this feature is to give the sysadmin the ability to
>>> create copies of filesystesm, change their uuid quickly and mount them alongside
>>> the original filesystem for, say, forensic purposes.
>>>
>>> One thing which still hasn't been set in stone is whether the new options
>>> will remain as -m|-M or whether they should subsume the current -u|-U - from
>>> the point of view of users nothing should change.
>>
>> Well, user would be surprised by how fast the new -m is, thus there is
>> still something changed :)
>>
>> I prefer to subsume current -u/-U, and use the new one if the incompat
>> feature is already set. Or fall back to original behavior.
>>
>> But I'm not a fan of using INCOMPAT flags as an indicator of changed
>> fsid/metadata uuid.
>> INCOMPAT feature should not change so easily nor acts as an indicator.
>>
>> That's to say, the flag should only be set at mkfs time, and then never
>> change unlike the 2nd patch (I don't even like btrfstune to change
>> incompat flags).
>>
>> E.g.
>> mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
>> change fsid without touching metadata uuid.
>> Or we could only use the old method.
>
> I disagree, I don't see any benefit in this but only added complexity.
> Can you elaborate more ?
Same here, I see essentially zero benefit to this, and one _big_
drawback, namely that you can't convert an existing volume to use this
approach if it's a feature that can only be set at mkfs time.
That one drawback means that this is effectively useless for all
existing BTRFS volumes, which is a pretty big limitation.
I also do think an INCOMPAT feature bit is appropriate here. Volumes
with this feature will potentially be enumerated with the wrong UUID on
older kernels, which is a pretty big behavioral issue (on the level of
completely breaking boot on some systems, keep in mind that almost all
major distros use volume UUID's to identify volumes in /etc/fstab).
>
>
>>
>> Thanks,
>> Qu
>>
>>> So this is something which
>>> I'd like to hear from the community. Of course the alternative of rewriting
>>> the metadata blocks will be assigne new options - perhaps -m|M ?
>>>
>>> I've tested this with multiple xfstest runs with the new tools installed as
>>> well as running btrfs-progs test and have observed no regressions.
>>>
>>> Nikolay Borisov (4):
>>> btrfs-progs: Add support for metadata_uuid field.
>>> btrfstune: Add support for changing the user uuid
>>> btrfs-progs: tests: Add tests for changing fsid feature
>>> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>>>
>>> btrfstune.c | 174 ++++++++++++++++++++++++-----
>>> check/main.c | 2 +-
>>> chunk-recover.c | 17 ++-
>>> cmds-filesystem.c | 2 +
>>> cmds-inspect-dump-super.c | 22 +++-
>>> convert/common.c | 2 +
>>> ctree.c | 15 +--
>>> ctree.h | 8 +-
>>> disk-io.c | 62 ++++++++--
>>> image/main.c | 25 +++--
>>> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
>>> volumes.c | 37 ++++--
>>> volumes.h | 1 +
>>> 13 files changed, 431 insertions(+), 73 deletions(-)
>>> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Userspace support for FSID change
2018-08-29 12:33 ` Nikolay Borisov
2018-08-29 12:43 ` Austin S. Hemmelgarn
@ 2018-08-29 12:47 ` Qu Wenruo
2018-09-02 9:56 ` Nikolay Borisov
1 sibling, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2018-08-29 12:47 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2018/8/29 下午8:33, Nikolay Borisov wrote:
>
>
> On 29.08.2018 15:09, Qu Wenruo wrote:
>>
>>
>> On 2018/8/29 下午4:35, Nikolay Borisov wrote:
>>> Here is the userspace tooling support for utilising the new metadata_uuid field,
>>> enabling the change of fsid without having to rewrite every metadata block. This
>>> patchset consists of adding support for the new field to various tools and
>>> files (Patch 1). The actual implementation of the new -m|-M options (which are
>>> described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
>>> exercises the new options and verifies certain invariants hold (these are also
>>> described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
>>> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
>>> structures.
>>
>> So to my understand, now we have another layer of UUID.
>>
>> Before we have one fsid, both used in superblock and tree blocks.
>>
>> Now we have 2 fsid, the one used in tree blocks are kept the same, but
>> changed its name to metadata_uuid in superblock.
>> And superblock::fsid will become a new field, and although they are the
>> same at mkfs time, they could change several times during its operation.
>>
>> This indeed makes uuid change super fast, only needs to update all
>> superblocks of the fs, instead of all tree blocks.
>>
>> However I have one nitpick of the design. Unlike XFS, btrfs supports
>> multiple devices.
>> If we have a raid10 fs with 4 devices, and it has already gone through
>> several UUID change (so its metadata uuid is already different from fsid).
>>
>> And during another UUID change procedure, we lost power while only
>> updated 2 super blocks, what will happen for kernel device assembly?
>>
>> (Although considering how fast the UUID change would happen, such case
>> should be super niche)
>
> Then I guess you will be fucked. I'm all ears for suggestion how to
> rectify this without skyrocketing the complexity. The current UUID
> rewrite method sets a flag int he superblock that FSID change is in
> progress and clears it once every metadatablock has been rewritten. I
> can piggyback on this mechanism but I'm not sure it provides 100%
> guarantee. Because by the some token you can set this flag, start
> writing the super blocks then lose power and then only some of the
> superblocks could have this flag set so we back at square 1.
Well, forget it, considering how fast the new method is, such case
should be really rare.
>
>>
>>>
>>> The intended usecase of this feature is to give the sysadmin the ability to
>>> create copies of filesystesm, change their uuid quickly and mount them alongside
>>> the original filesystem for, say, forensic purposes.
>>>
>>> One thing which still hasn't been set in stone is whether the new options
>>> will remain as -m|-M or whether they should subsume the current -u|-U - from
>>> the point of view of users nothing should change.
>>
>> Well, user would be surprised by how fast the new -m is, thus there is
>> still something changed :)
>>
>> I prefer to subsume current -u/-U, and use the new one if the incompat
>> feature is already set. Or fall back to original behavior.
>>
>> But I'm not a fan of using INCOMPAT flags as an indicator of changed
>> fsid/metadata uuid.
>> INCOMPAT feature should not change so easily nor acts as an indicator.
>>
>> That's to say, the flag should only be set at mkfs time, and then never
>> change unlike the 2nd patch (I don't even like btrfstune to change
>> incompat flags).
>>
>> E.g.
>> mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
>> change fsid without touching metadata uuid.
>> Or we could only use the old method.
>
> I disagree, I don't see any benefit in this but only added complexity.
> Can you elaborate more ?
Well, the incompat feature is really introducing some incompatible
on-disk format change.
So if you're introducing the new metadata_uuid field, no matter if it
differs from fsid or not, it's a new field, and the incompat flag should
be set.
To me, older kernel could recognize the new format when fsid matches
metadata uuid (since there in your current patchset, such case will not
have incompat flag set) is a little dangerous.
What will happen if such old kernel/btrfs-progs tries to change fsid?
Older btrfs-progs doesn't know there is a new field, it will not touch
the metadata uuid field, just changing the fsid field along with all
tree blocks.
This will cause a fs whose fsid doesn't match metadata uuid and has no
incompat flag set. This is definitely leading to compatibility problem.
So we need to follow the incompat flags rule strictly, if on-disk format
is changed, we need the new incompat flag.
Thanks,
Qu
>
>
>>
>> Thanks,
>> Qu
>>
>>> So this is something which
>>> I'd like to hear from the community. Of course the alternative of rewriting
>>> the metadata blocks will be assigne new options - perhaps -m|M ?
>>>
>>> I've tested this with multiple xfstest runs with the new tools installed as
>>> well as running btrfs-progs test and have observed no regressions.
>>>
>>> Nikolay Borisov (4):
>>> btrfs-progs: Add support for metadata_uuid field.
>>> btrfstune: Add support for changing the user uuid
>>> btrfs-progs: tests: Add tests for changing fsid feature
>>> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>>>
>>> btrfstune.c | 174 ++++++++++++++++++++++++-----
>>> check/main.c | 2 +-
>>> chunk-recover.c | 17 ++-
>>> cmds-filesystem.c | 2 +
>>> cmds-inspect-dump-super.c | 22 +++-
>>> convert/common.c | 2 +
>>> ctree.c | 15 +--
>>> ctree.h | 8 +-
>>> disk-io.c | 62 ++++++++--
>>> image/main.c | 25 +++--
>>> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
>>> volumes.c | 37 ++++--
>>> volumes.h | 1 +
>>> 13 files changed, 431 insertions(+), 73 deletions(-)
>>> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Userspace support for FSID change
2018-08-29 12:47 ` Qu Wenruo
@ 2018-09-02 9:56 ` Nikolay Borisov
2018-09-02 23:50 ` Qu Wenruo
0 siblings, 1 reply; 11+ messages in thread
From: Nikolay Borisov @ 2018-09-02 9:56 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 29.08.2018 15:47, Qu Wenruo wrote:
>
>
> On 2018/8/29 下午8:33, Nikolay Borisov wrote:
>>
>>
>> On 29.08.2018 15:09, Qu Wenruo wrote:
>>>
>>>
>>> On 2018/8/29 下午4:35, Nikolay Borisov wrote:
>>>> Here is the userspace tooling support for utilising the new metadata_uuid field,
>>>> enabling the change of fsid without having to rewrite every metadata block. This
>>>> patchset consists of adding support for the new field to various tools and
>>>> files (Patch 1). The actual implementation of the new -m|-M options (which are
>>>> described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
>>>> exercises the new options and verifies certain invariants hold (these are also
>>>> described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
>>>> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
>>>> structures.
>>>
>>> So to my understand, now we have another layer of UUID.
>>>
>>> Before we have one fsid, both used in superblock and tree blocks.
>>>
>>> Now we have 2 fsid, the one used in tree blocks are kept the same, but
>>> changed its name to metadata_uuid in superblock.
>>> And superblock::fsid will become a new field, and although they are the
>>> same at mkfs time, they could change several times during its operation.
>>>
>>> This indeed makes uuid change super fast, only needs to update all
>>> superblocks of the fs, instead of all tree blocks.
>>>
>>> However I have one nitpick of the design. Unlike XFS, btrfs supports
>>> multiple devices.
>>> If we have a raid10 fs with 4 devices, and it has already gone through
>>> several UUID change (so its metadata uuid is already different from fsid).
>>>
>>> And during another UUID change procedure, we lost power while only
>>> updated 2 super blocks, what will happen for kernel device assembly?
>>>
>>> (Although considering how fast the UUID change would happen, such case
>>> should be super niche)
>>
>> Then I guess you will be fucked. I'm all ears for suggestion how to
>> rectify this without skyrocketing the complexity. The current UUID
>> rewrite method sets a flag int he superblock that FSID change is in
>> progress and clears it once every metadatablock has been rewritten. I
>> can piggyback on this mechanism but I'm not sure it provides 100%
>> guarantee. Because by the some token you can set this flag, start
>> writing the super blocks then lose power and then only some of the
>> superblocks could have this flag set so we back at square 1.
>
> Well, forget it, considering how fast the new method is, such case
> should be really rare.
>
>>
>>>
>>>>
>>>> The intended usecase of this feature is to give the sysadmin the ability to
>>>> create copies of filesystesm, change their uuid quickly and mount them alongside
>>>> the original filesystem for, say, forensic purposes.
>>>>
>>>> One thing which still hasn't been set in stone is whether the new options
>>>> will remain as -m|-M or whether they should subsume the current -u|-U - from
>>>> the point of view of users nothing should change.
>>>
>>> Well, user would be surprised by how fast the new -m is, thus there is
>>> still something changed :)
>>>
>>> I prefer to subsume current -u/-U, and use the new one if the incompat
>>> feature is already set. Or fall back to original behavior.
>>>
>>> But I'm not a fan of using INCOMPAT flags as an indicator of changed
>>> fsid/metadata uuid.
>>> INCOMPAT feature should not change so easily nor acts as an indicator.
>>>
>>> That's to say, the flag should only be set at mkfs time, and then never
>>> change unlike the 2nd patch (I don't even like btrfstune to change
>>> incompat flags).
>>>
>>> E.g.
>>> mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
>>> change fsid without touching metadata uuid.
>>> Or we could only use the old method.
>>
>> I disagree, I don't see any benefit in this but only added complexity.
>> Can you elaborate more ?
>
> Well, the incompat feature is really introducing some incompatible
> on-disk format change.
> So if you're introducing the new metadata_uuid field, no matter if it
> differs from fsid or not, it's a new field, and the incompat flag should
> be set.
>
> To me, older kernel could recognize the new format when fsid matches
> metadata uuid (since there in your current patchset, such case will not
> have incompat flag set) is a little dangerous.
>
> What will happen if such old kernel/btrfs-progs tries to change fsid?
> Older btrfs-progs doesn't know there is a new field, it will not touch
> the metadata uuid field, just changing the fsid field along with all
> tree blocks.
>
> This will cause a fs whose fsid doesn't match metadata uuid and has no
> incompat flag set. This is definitely leading to compatibility problem.
Nope, when both fsid/metadata_uuid match and the INCOMPAT flag is not
set then on-disk we never save the metadata_uuid value and this value is
set only in-memory, the only thing on-disk is fsid. In this case if one
decides to rewrite the fsuid then this is fine - btrfs-progs disallow
using -m|-M when fsid rewrite is in progress. So when the fsid/metadata
is not set to old progs the kernel continue working as-is. When it's
changed then the incompat flags will prevent us from doing harmful
modifications.
>
> So we need to follow the incompat flags rule strictly, if on-disk format
> is changed, we need the new incompat flag.
>
> Thanks,
> Qu
>>
>>
>>>
>>> Thanks,
>>> Qu
>>>
>>>> So this is something which
>>>> I'd like to hear from the community. Of course the alternative of rewriting
>>>> the metadata blocks will be assigne new options - perhaps -m|M ?
>>>>
>>>> I've tested this with multiple xfstest runs with the new tools installed as
>>>> well as running btrfs-progs test and have observed no regressions.
>>>>
>>>> Nikolay Borisov (4):
>>>> btrfs-progs: Add support for metadata_uuid field.
>>>> btrfstune: Add support for changing the user uuid
>>>> btrfs-progs: tests: Add tests for changing fsid feature
>>>> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>>>>
>>>> btrfstune.c | 174 ++++++++++++++++++++++++-----
>>>> check/main.c | 2 +-
>>>> chunk-recover.c | 17 ++-
>>>> cmds-filesystem.c | 2 +
>>>> cmds-inspect-dump-super.c | 22 +++-
>>>> convert/common.c | 2 +
>>>> ctree.c | 15 +--
>>>> ctree.h | 8 +-
>>>> disk-io.c | 62 ++++++++--
>>>> image/main.c | 25 +++--
>>>> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
>>>> volumes.c | 37 ++++--
>>>> volumes.h | 1 +
>>>> 13 files changed, 431 insertions(+), 73 deletions(-)
>>>> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>>>>
>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] Userspace support for FSID change
2018-09-02 9:56 ` Nikolay Borisov
@ 2018-09-02 23:50 ` Qu Wenruo
0 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2018-09-02 23:50 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs
On 2018/9/2 下午5:56, Nikolay Borisov wrote:
>
>
> On 29.08.2018 15:47, Qu Wenruo wrote:
>>
>>
>> On 2018/8/29 下午8:33, Nikolay Borisov wrote:
>>>
>>>
>>> On 29.08.2018 15:09, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2018/8/29 下午4:35, Nikolay Borisov wrote:
>>>>> Here is the userspace tooling support for utilising the new metadata_uuid field,
>>>>> enabling the change of fsid without having to rewrite every metadata block. This
>>>>> patchset consists of adding support for the new field to various tools and
>>>>> files (Patch 1). The actual implementation of the new -m|-M options (which are
>>>>> described in more detail in Patch 2). A new misc-tests testcasei (Patch 3) which
>>>>> exercises the new options and verifies certain invariants hold (these are also
>>>>> described in Patch2). Patch 4 is more or less copy of the kernel conuterpart
>>>>> just reducing some duplication between btrfs_fs_info and btrfs_fs_devices
>>>>> structures.
>>>>
>>>> So to my understand, now we have another layer of UUID.
>>>>
>>>> Before we have one fsid, both used in superblock and tree blocks.
>>>>
>>>> Now we have 2 fsid, the one used in tree blocks are kept the same, but
>>>> changed its name to metadata_uuid in superblock.
>>>> And superblock::fsid will become a new field, and although they are the
>>>> same at mkfs time, they could change several times during its operation.
>>>>
>>>> This indeed makes uuid change super fast, only needs to update all
>>>> superblocks of the fs, instead of all tree blocks.
>>>>
>>>> However I have one nitpick of the design. Unlike XFS, btrfs supports
>>>> multiple devices.
>>>> If we have a raid10 fs with 4 devices, and it has already gone through
>>>> several UUID change (so its metadata uuid is already different from fsid).
>>>>
>>>> And during another UUID change procedure, we lost power while only
>>>> updated 2 super blocks, what will happen for kernel device assembly?
>>>>
>>>> (Although considering how fast the UUID change would happen, such case
>>>> should be super niche)
>>>
>>> Then I guess you will be fucked. I'm all ears for suggestion how to
>>> rectify this without skyrocketing the complexity. The current UUID
>>> rewrite method sets a flag int he superblock that FSID change is in
>>> progress and clears it once every metadatablock has been rewritten. I
>>> can piggyback on this mechanism but I'm not sure it provides 100%
>>> guarantee. Because by the some token you can set this flag, start
>>> writing the super blocks then lose power and then only some of the
>>> superblocks could have this flag set so we back at square 1.
>>
>> Well, forget it, considering how fast the new method is, such case
>> should be really rare.
>>
>>>
>>>>
>>>>>
>>>>> The intended usecase of this feature is to give the sysadmin the ability to
>>>>> create copies of filesystesm, change their uuid quickly and mount them alongside
>>>>> the original filesystem for, say, forensic purposes.
>>>>>
>>>>> One thing which still hasn't been set in stone is whether the new options
>>>>> will remain as -m|-M or whether they should subsume the current -u|-U - from
>>>>> the point of view of users nothing should change.
>>>>
>>>> Well, user would be surprised by how fast the new -m is, thus there is
>>>> still something changed :)
>>>>
>>>> I prefer to subsume current -u/-U, and use the new one if the incompat
>>>> feature is already set. Or fall back to original behavior.
>>>>
>>>> But I'm not a fan of using INCOMPAT flags as an indicator of changed
>>>> fsid/metadata uuid.
>>>> INCOMPAT feature should not change so easily nor acts as an indicator.
>>>>
>>>> That's to say, the flag should only be set at mkfs time, and then never
>>>> change unlike the 2nd patch (I don't even like btrfstune to change
>>>> incompat flags).
>>>>
>>>> E.g.
>>>> mkfs.btrfs -O metadata_uuid <device>, then we could use the new way to
>>>> change fsid without touching metadata uuid.
>>>> Or we could only use the old method.
>>>
>>> I disagree, I don't see any benefit in this but only added complexity.
>>> Can you elaborate more ?
>>
>> Well, the incompat feature is really introducing some incompatible
>> on-disk format change.
>> So if you're introducing the new metadata_uuid field, no matter if it
>> differs from fsid or not, it's a new field, and the incompat flag should
>> be set.
>>
>> To me, older kernel could recognize the new format when fsid matches
>> metadata uuid (since there in your current patchset, such case will not
>> have incompat flag set) is a little dangerous.
>>
>> What will happen if such old kernel/btrfs-progs tries to change fsid?
>> Older btrfs-progs doesn't know there is a new field, it will not touch
>> the metadata uuid field, just changing the fsid field along with all
>> tree blocks.
>>
>> This will cause a fs whose fsid doesn't match metadata uuid and has no
>> incompat flag set. This is definitely leading to compatibility problem.
>
> Nope, when both fsid/metadata_uuid match and the INCOMPAT flag is not
> set then on-disk we never save the metadata_uuid value and this value is
> set only in-memory, the only thing on-disk is fsid.
In my opinion, this is already more complex than forced new INCOMPAT
flag all the time.
You're already using 4 different branches to workaround the
compatibility, which already looks more complex than all time INCOMPAT flag.
And to me, I didn't really think using all time INCOMPAT flag is a
problem at all.
If someone really want to use it, btrfstune could be changed to add
METADATA_UUID flag easily for old fs.
And if the feature is well received we could just make this feature as
default later.
Thanks,
Qu
> In this case if one
> decides to rewrite the fsuid then this is fine - btrfs-progs disallow
> using -m|-M when fsid rewrite is in progress. So when the fsid/metadata
> is not set to old progs the kernel continue working as-is. When it's
> changed then the incompat flags will prevent us from doing harmful
> modifications.
>
>>
>> So we need to follow the incompat flags rule strictly, if on-disk format
>> is changed, we need the new incompat flag.
>>
>> Thanks,
>> Qu
>>>
>>>
>>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>> So this is something which
>>>>> I'd like to hear from the community. Of course the alternative of rewriting
>>>>> the metadata blocks will be assigne new options - perhaps -m|M ?
>>>>>
>>>>> I've tested this with multiple xfstest runs with the new tools installed as
>>>>> well as running btrfs-progs test and have observed no regressions.
>>>>>
>>>>> Nikolay Borisov (4):
>>>>> btrfs-progs: Add support for metadata_uuid field.
>>>>> btrfstune: Add support for changing the user uuid
>>>>> btrfs-progs: tests: Add tests for changing fsid feature
>>>>> btrfs-progs: Remove fsid/metdata_uuid fields from fs_info
>>>>>
>>>>> btrfstune.c | 174 ++++++++++++++++++++++++-----
>>>>> check/main.c | 2 +-
>>>>> chunk-recover.c | 17 ++-
>>>>> cmds-filesystem.c | 2 +
>>>>> cmds-inspect-dump-super.c | 22 +++-
>>>>> convert/common.c | 2 +
>>>>> ctree.c | 15 +--
>>>>> ctree.h | 8 +-
>>>>> disk-io.c | 62 ++++++++--
>>>>> image/main.c | 25 +++--
>>>>> tests/misc-tests/033-metadata-uuid/test.sh | 137 +++++++++++++++++++++++
>>>>> volumes.c | 37 ++++--
>>>>> volumes.h | 1 +
>>>>> 13 files changed, 431 insertions(+), 73 deletions(-)
>>>>> create mode 100755 tests/misc-tests/033-metadata-uuid/test.sh
>>>>>
>>>>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-03 4:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29 8:35 [PATCH 0/4] Userspace support for FSID change Nikolay Borisov
2018-08-29 8:35 ` [PATCH 1/4] btrfs-progs: Add support for metadata_uuid field Nikolay Borisov
2018-08-29 8:35 ` [PATCH 2/4] btrfstune: Add support for changing the user uuid Nikolay Borisov
2018-08-29 8:35 ` [PATCH 3/4] btrfs-progs: tests: Add tests for changing fsid feature Nikolay Borisov
2018-08-29 8:35 ` [PATCH 4/4] btrfs-progs: Remove fsid/metdata_uuid fields from fs_info Nikolay Borisov
2018-08-29 12:09 ` [PATCH 0/4] Userspace support for FSID change Qu Wenruo
2018-08-29 12:33 ` Nikolay Borisov
2018-08-29 12:43 ` Austin S. Hemmelgarn
2018-08-29 12:47 ` Qu Wenruo
2018-09-02 9:56 ` Nikolay Borisov
2018-09-02 23:50 ` Qu Wenruo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.