[PATCH 0/4] Convert regression fix with print-tree enhancement

[PATCH 0/4] Convert regression fix with print-tree enhancement

[Qu Wenruo]

This patchset fixes one convert regression, adds regression test.
Then with 2 print-tree enhancement, which is quite handy to find how inode
and subvolume READONLY flag is set.

Qu Wenruo (4):
  btrfs-progs: convert: Fix a regression that ext2_save/image is not
    readonly
  btrfs-progs: convert-test: Check if the ext2_save/image is read only
  btrfs-progs: print-tree: Print human readable inode flags
  btrfs-progs: print-tree: Print hex and human readable root flags

 btrfs-convert.c                                |  9 ++++-
 ctree.h                                        |  2 ++
 inode.c                                        | 36 +++++++++++++++++++
 print-tree.c                                   | 49 ++++++++++++++++++++++++--
 tests/convert-tests/008-readonly-image/test.sh | 26 ++++++++++++++
 5 files changed, 119 insertions(+), 3 deletions(-)
 create mode 100755 tests/convert-tests/008-readonly-image/test.sh

-- 
2.9.3

[PATCH 1/4] btrfs-progs: convert: Fix a regression that ext2_save/image is not readonly

[Qu Wenruo]

The new convert treats the convert image as a normal file, without any
special flags and permissions.

This is different from original code:
1) Permission changed from 0400 to 0600
2) Inode lacks READONLY flag

This makes we can read-write mount the ext2 image and cause rollback
failure.

Follow old code behavior, use 0400 permission and add back READONLY
flag to fix it.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 btrfs-convert.c |  9 ++++++++-
 ctree.h         |  2 ++
 inode.c         | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/btrfs-convert.c b/btrfs-convert.c
index b18de59..a8852d6 100644
--- a/btrfs-convert.c
+++ b/btrfs-convert.c
@@ -1527,8 +1527,12 @@ static int create_image(struct btrfs_root *root,
 	struct cache_tree used_tmp;
 	u64 cur;
 	u64 ino;
+	u64 flags = BTRFS_INODE_READONLY;
 	int ret;
 
+	if (!datacsum)
+		flags |= BTRFS_INODE_NODATASUM;
+
 	trans = btrfs_start_transaction(root, 1);
 	if (!trans)
 		return -ENOMEM;
@@ -1539,7 +1543,10 @@ static int create_image(struct btrfs_root *root,
 				       &ino);
 	if (ret < 0)
 		goto out;
-	ret = btrfs_new_inode(trans, root, ino, 0600 | S_IFREG);
+	ret = btrfs_new_inode(trans, root, ino, 0400 | S_IFREG);
+	if (ret < 0)
+		goto out;
+	ret = btrfs_change_inode_flags(trans, root, ino, flags);
 	if (ret < 0)
 		goto out;
 	ret = btrfs_add_link(trans, root, ino, BTRFS_FIRST_FREE_OBJECTID, name,
diff --git a/ctree.h b/ctree.h
index dda72d0..b9fb732 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2578,6 +2578,8 @@ int check_dir_conflict(struct btrfs_root *root, char *name, int namelen,
 		u64 dir, u64 index);
 int btrfs_new_inode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		u64 ino, u32 mode);
+int btrfs_change_inode_flags(struct btrfs_trans_handle *trans,
+			     struct btrfs_root *root, u64 ino, u64 flags);
 int btrfs_add_link(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 		   u64 ino, u64 parent_ino, char *name, int namelen,
 		   u8 type, u64 *index, int add_backref);
diff --git a/inode.c b/inode.c
index ed6d529..991b8dd 100644
--- a/inode.c
+++ b/inode.c
@@ -472,6 +472,42 @@ int btrfs_new_inode(struct btrfs_trans_handle *trans, struct btrfs_root *root,
 }
 
 /*
+ * Change inode flags to given value
+ */
+int btrfs_change_inode_flags(struct btrfs_trans_handle *trans,
+			     struct btrfs_root *root, u64 ino, u64 flags)
+{
+	struct btrfs_inode_item *item;
+	struct btrfs_path *path;
+	struct btrfs_key key;
+	int ret;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	key.objectid = ino;
+	key.type = BTRFS_INODE_ITEM_KEY;
+	key.offset = 0;
+
+	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+	if (ret > 0) {
+		ret = -ENOENT;
+		goto out;
+	}
+	if (ret < 0)
+		goto out;
+
+	item = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			      struct btrfs_inode_item);
+	btrfs_set_inode_flags(path->nodes[0], item, flags);
+	btrfs_mark_buffer_dirty(path->nodes[0]);
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
+/*
  * Make a dir under the parent inode 'parent_ino' with 'name'
  * and 'mode', The owner will be root/root.
  */
-- 
2.9.3

[PATCH 2/4] btrfs-progs: convert-test: Check if the ext2_save/image is read only

[Qu Wenruo]

Old convert codes uses both 0400 permission and INODE_READONLY flag to
make the converted ext2 image readonly.

While new convert treat the inode just as normal inode, with no special
inode flag and uses 0600 permission.
This makes user able to modify converted image unintentionally and make
rollback fails.

This test case will test the regression.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 tests/convert-tests/008-readonly-image/test.sh | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100755 tests/convert-tests/008-readonly-image/test.sh

diff --git a/tests/convert-tests/008-readonly-image/test.sh b/tests/convert-tests/008-readonly-image/test.sh
new file mode 100755
index 0000000..4e42237
--- /dev/null
+++ b/tests/convert-tests/008-readonly-image/test.sh
@@ -0,0 +1,26 @@
+#!/bin/bash
+# Check if the converted ext2 image is readonly
+
+source $TOP/tests/common
+source $TOP/tests/common.convert
+
+setup_root_helper
+prepare_test_dev 512M
+check_prereq btrfs-convert
+
+default_mke2fs="mke2fs -t ext4 -b 4096"
+convert_test_preamble '' 'readonly image test' 16k "$default_mke2fs"
+convert_test_prep_fs $default_mke2fs
+run_check_umount_test_dev
+convert_test_do_convert
+run_check_mount_test_dev
+
+# It's expected to fail
+$SUDO_HELPER dd if=/dev/zero of=$TEST_MNT/ext2_save/image bs=1M count=1 \
+	&> /dev/null
+if [ $? -ne 1 ]; then
+	echo "after convert ext2_save/image is not read-only"
+	exit 1
+fi
+run_check_umount_test_dev
+convert_test_post_rollback
-- 
2.9.3

[PATCH 3/4] btrfs-progs: print-tree: Print human readable inode flags

[Qu Wenruo]

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
 print-tree.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/print-tree.c b/print-tree.c
index 9f9e11e..f33ddad 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -835,6 +835,33 @@ static void print_uuid_item(struct extent_buffer *l, unsigned long offset,
 	}
 }
 
+/* Caller should ensure sizeof(*ret) >= 29 "NODATASUM|NODATACOW|READONLY" */
+static void inode_flags_to_str(u64 flags, char *ret)
+{
+	int empty = 1;
+
+	if (flags & BTRFS_INODE_NODATASUM) {
+		empty = 0;
+		strcpy(ret, "NODATASUM");
+	}
+	if (flags & BTRFS_INODE_NODATACOW) {
+		if (!empty) {
+			empty = 0;
+			strcat(ret, "|");
+		}
+		strcat(ret, "NODATACOW");
+	}
+	if (flags & BTRFS_INODE_READONLY) {
+		if (!empty) {
+			empty = 0;
+			strcat(ret, "|");
+		}
+		strcat(ret, "READONLY");
+	}
+	if (empty)
+		strcat(ret, "none");
+}
+
 void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l)
 {
 	int i;
@@ -884,10 +911,12 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l)
 
 		switch (type) {
 		case BTRFS_INODE_ITEM_KEY:
+			memset(flags_str, 0, sizeof(flags_str));
 			ii = btrfs_item_ptr(l, i, struct btrfs_inode_item);
+			inode_flags_to_str(btrfs_inode_flags(l, ii), flags_str);
 			printf("\t\tinode generation %llu transid %llu size %llu nbytes %llu\n"
 			       "\t\tblock group %llu mode %o links %u uid %u gid %u\n"
-			       "\t\trdev %llu flags 0x%llx\n",
+			       "\t\trdev %llu flags 0x%llx(%s)\n",
 			       (unsigned long long)btrfs_inode_generation(l, ii),
 			       (unsigned long long)btrfs_inode_transid(l, ii),
 			       (unsigned long long)btrfs_inode_size(l, ii),
@@ -898,7 +927,8 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *l)
 			       btrfs_inode_uid(l, ii),
 			       btrfs_inode_gid(l, ii),
 			       (unsigned long long)btrfs_inode_rdev(l,ii),
-			       (unsigned long long)btrfs_inode_flags(l,ii));
+			       (unsigned long long)btrfs_inode_flags(l,ii),
+			       flags_str);
 			break;
 		case BTRFS_INODE_REF_KEY:
 			iref = btrfs_item_ptr(l, i, struct btrfs_inode_ref);
-- 
2.9.3

[PATCH 4/4] btrfs-progs: print-tree: Print hex and human readable root flags

[Qu Wenruo]

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

diff --git a/print-tree.c b/print-tree.c
index f33ddad..81ab81f 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -496,18 +496,31 @@ static int count_bytes(void *buf, int len, char b)
 	return cnt;
 }
 
+/*
+ * Caller must ensure sizeof(*ret) >= 7 "RDONLY"
+ */
+static void root_flags_to_str(u64 flags, char *ret)
+{
+	if (flags & BTRFS_ROOT_SUBVOL_RDONLY)
+		strcat(ret, "RDONLY");
+	else
+		strcat(ret, "none");
+}
+
 static void print_root(struct extent_buffer *leaf, int slot)
 {
 	struct btrfs_root_item *ri;
 	struct btrfs_root_item root_item;
 	int len;
 	char uuid_str[BTRFS_UUID_UNPARSED_SIZE];
+	char flags_str[32] = {0};
 
 	ri = btrfs_item_ptr(leaf, slot, struct btrfs_root_item);
 	len = btrfs_item_size_nr(leaf, slot);
 
 	memset(&root_item, 0, sizeof(root_item));
 	read_extent_buffer(leaf, &root_item, (unsigned long)ri, len);
+	root_flags_to_str(btrfs_root_flags(&root_item), flags_str);
 
 	printf("\t\troot data bytenr %llu level %d dirid %llu refs %u gen %llu lastsnap %llu\n",
 		(unsigned long long)btrfs_root_bytenr(&root_item),
@@ -516,6 +529,8 @@ static void print_root(struct extent_buffer *leaf, int slot)
 		btrfs_root_refs(&root_item),
 		(unsigned long long)btrfs_root_generation(&root_item),
 		(unsigned long long)btrfs_root_last_snapshot(&root_item));
+	printf("\t\tflags 0x%llx(%s)\n", btrfs_root_flags(&root_item),
+	       flags_str);
 
 	if (root_item.generation == root_item.generation_v2) {
 		uuid_unparse(root_item.uuid, uuid_str);
-- 
2.9.3

Re: [PATCH 0/4] Convert regression fix with print-tree enhancement

[David Sterba]

On Fri, Aug 19, 2016 at 04:13:04PM +0800, Qu Wenruo wrote:
> This patchset fixes one convert regression, adds regression test.
> Then with 2 print-tree enhancement, which is quite handy to find how inode
> and subvolume READONLY flag is set.
> 
> Qu Wenruo (4):
>   btrfs-progs: convert: Fix a regression that ext2_save/image is not
>     readonly
>   btrfs-progs: convert-test: Check if the ext2_save/image is read only
>   btrfs-progs: print-tree: Print human readable inode flags
>   btrfs-progs: print-tree: Print hex and human readable root flags

All applied, thanks.