[PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Qu Wenruo]

mkfs.btrfs --rootdir provides user a method to generate btrfs with
pre-written content while without the need of root privilege.

However the code is quite old and doesn't get much review or test.
This makes some strange behavior, from customized chunk allocation
(which uses the reserved 0~1M device space) to lack of special file
handler (Fixed in previous 2 patches).

The reworked --rootdir will be based on traditional mkfs, everything is
processed after traditional mkfs, so nothing is customized.

The result will be an equivalent of mkfs, mount, cp, umount.
(If btrfs-progs chunk/extent allocator acts as the same as kernel)

And, add extra explanation for --rootdir, since the old implement
introduced a confusing behavior to limit the filesystem size.

The 1st patch fixes a bug that causes any write after
cleanup_temp_chunks() to trigger a NULL pointer dereference.

The 2nd patch changes the work flow of --rootdir, and slightly changed
the behavior, which will not shrink filesystem size by its own.

The 3rd patch will shrink the fs, to keep the behavior the same as old
--rootdir.
If anyone doesn't like the behavior, it can be removed easily.
(Which is the main part of the new code, I'd like to get rid of this)

The 4th patch enhance the verbose output to reflect the real chunk
allocation of --rootdir.

The final patch add extra documentary explanation.

Qu Wenruo (5):
  btrfs-progs: Fix one-byte overlap bug in free_block_group_cache
  btrfs-progs: mkfs: Rework rootdir option to avoid custom chunk layout
  btrfs-progs: mkfs: Shrink the image for rootdir to minimal size
  btrfs-progs: mkfs: Update allocation info before verbose output
  btrfs-progs: Doc/mkfs: Add explanation for rootdir parameter

 Documentation/mkfs.btrfs.asciidoc |   3 +
 extent-tree.c                     |   2 +-
 mkfs/main.c                       | 342 ++++++++++++++++++--------------------
 3 files changed, 164 insertions(+), 183 deletions(-)

-- 
2.14.1

[PATCH 1/5] btrfs-progs: Fix one-byte overlap bug in free_block_group_cache

[Qu Wenruo]

free_block_group_cache() calls clear_extent_bits() with wrong end, which
is one byte larger than the correct range.

This will cause the next adjacent cache state be split.
And due to the split, private pointer (which points to block group
cache) will be reset to NULL.

This is very hard to detect as this function only gets called in
cleanup_temp_chunks() which is just before mkfs finishes.
This bug only get exposed when reworking --rootdir option.

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 extent-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extent-tree.c b/extent-tree.c
index 46b8a05c..3039d254 100644
--- a/extent-tree.c
+++ b/extent-tree.c
@@ -3726,7 +3726,7 @@ static int free_block_group_cache(struct btrfs_trans_handle *trans,
 		btrfs_remove_free_space_cache(cache);
 		kfree(cache->free_space_ctl);
 	}
-	clear_extent_bits(&fs_info->block_group_cache, bytenr, bytenr + len,
+	clear_extent_bits(&fs_info->block_group_cache, bytenr, bytenr + len - 1,
 			  (unsigned int)-1);
 	ret = free_space_info(fs_info, flags, len, 0, NULL);
 	if (ret < 0)
-- 
2.14.1

[PATCH 2/5] btrfs-progs: mkfs: Rework rootdir option to avoid custom chunk layout

[Qu Wenruo]

mkfs.btrfs --rootdir uses its own custom chunk layout.
This provides the possibility to limit the filesystem to a minimal size.

However this custom chunk allocation has several problems.
The most obvious problem is that it will allocate chunk from device offset
0.
Both kernel and normal mkfs will reserve 0~1M range for each device.

This rework will remove all related custom chunk allocation and size
calculation.
Less code to maintain is always a good thing, especially for minor or
less maintained code.

So all --rootdir operation will result the same result as mkfs.btrfs +
mount + cp. (Same result as mkfs.ext4 -d)

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 mkfs/main.c | 218 ++++++++----------------------------------------------------
 1 file changed, 28 insertions(+), 190 deletions(-)

diff --git a/mkfs/main.c b/mkfs/main.c
index 84ff300b..bca836b5 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -990,53 +990,6 @@ fail_no_dir:
 	goto out;
 }
 
-static int create_chunks(struct btrfs_trans_handle *trans,
-			 struct btrfs_root *root, u64 num_of_meta_chunks,
-			 u64 size_of_data,
-			 struct mkfs_allocation *allocation)
-{
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	u64 chunk_start;
-	u64 chunk_size;
-	u64 meta_type = BTRFS_BLOCK_GROUP_METADATA;
-	u64 data_type = BTRFS_BLOCK_GROUP_DATA;
-	u64 minimum_data_chunk_size = 8 * 1024 * 1024;
-	u64 i;
-	int ret;
-
-	for (i = 0; i < num_of_meta_chunks; i++) {
-		ret = btrfs_alloc_chunk(trans, fs_info,
-					&chunk_start, &chunk_size, meta_type);
-		if (ret)
-			return ret;
-		ret = btrfs_make_block_group(trans, fs_info, 0,
-					     meta_type, BTRFS_FIRST_CHUNK_TREE_OBJECTID,
-					     chunk_start, chunk_size);
-		allocation->metadata += chunk_size;
-		if (ret)
-			return ret;
-		set_extent_dirty(&root->fs_info->free_space_cache,
-				 chunk_start, chunk_start + chunk_size - 1);
-	}
-
-	if (size_of_data < minimum_data_chunk_size)
-		size_of_data = minimum_data_chunk_size;
-
-	ret = btrfs_alloc_data_chunk(trans, fs_info,
-				     &chunk_start, size_of_data, data_type, 0);
-	if (ret)
-		return ret;
-	ret = btrfs_make_block_group(trans, fs_info, 0,
-				     data_type, BTRFS_FIRST_CHUNK_TREE_OBJECTID,
-				     chunk_start, size_of_data);
-	allocation->data += size_of_data;
-	if (ret)
-		return ret;
-	set_extent_dirty(&root->fs_info->free_space_cache,
-			 chunk_start, chunk_start + size_of_data - 1);
-	return ret;
-}
-
 static int make_image(const char *source_dir, struct btrfs_root *root)
 {
 	int ret;
@@ -1080,86 +1033,6 @@ out:
 	return ret;
 }
 
-/*
- * This ignores symlinks with unreadable targets and subdirs that can't
- * be read.  It's a best-effort to give a rough estimate of the size of
- * a subdir.  It doesn't guarantee that prepopulating btrfs from this
- * tree won't still run out of space.
- */
-static u64 global_total_size;
-static u64 fs_block_size;
-static int ftw_add_entry_size(const char *fpath, const struct stat *st,
-			      int type)
-{
-	if (type == FTW_F || type == FTW_D)
-		global_total_size += round_up(st->st_size, fs_block_size);
-
-	return 0;
-}
-
-static u64 size_sourcedir(const char *dir_name, u64 sectorsize,
-			  u64 *num_of_meta_chunks_ret, u64 *size_of_data_ret)
-{
-	u64 dir_size = 0;
-	u64 total_size = 0;
-	int ret;
-	u64 default_chunk_size = 8 * 1024 * 1024;	/* 8MB */
-	u64 allocated_meta_size = 8 * 1024 * 1024;	/* 8MB */
-	u64 allocated_total_size = 20 * 1024 * 1024;	/* 20MB */
-	u64 num_of_meta_chunks = 0;
-	u64 num_of_data_chunks = 0;
-	u64 num_of_allocated_meta_chunks =
-			allocated_meta_size / default_chunk_size;
-
-	global_total_size = 0;
-	fs_block_size = sectorsize;
-	ret = ftw(dir_name, ftw_add_entry_size, 10);
-	dir_size = global_total_size;
-	if (ret < 0) {
-		error("ftw subdir walk of %s failed: %s", dir_name,
-			strerror(errno));
-		exit(1);
-	}
-
-	num_of_data_chunks = (dir_size + default_chunk_size - 1) /
-		default_chunk_size;
-
-	num_of_meta_chunks = (dir_size / 2) / default_chunk_size;
-	if (((dir_size / 2) % default_chunk_size) != 0)
-		num_of_meta_chunks++;
-	if (num_of_meta_chunks <= num_of_allocated_meta_chunks)
-		num_of_meta_chunks = 0;
-	else
-		num_of_meta_chunks -= num_of_allocated_meta_chunks;
-
-	total_size = allocated_total_size +
-		     (num_of_data_chunks * default_chunk_size) +
-		     (num_of_meta_chunks * default_chunk_size);
-
-	*num_of_meta_chunks_ret = num_of_meta_chunks;
-	*size_of_data_ret = num_of_data_chunks * default_chunk_size;
-	return total_size;
-}
-
-static int zero_output_file(int out_fd, u64 size)
-{
-	int loop_num;
-	u64 location = 0;
-	char buf[4096];
-	int ret = 0, i;
-	ssize_t written;
-
-	memset(buf, 0, 4096);
-	loop_num = size / 4096;
-	for (i = 0; i < loop_num; i++) {
-		written = pwrite64(out_fd, buf, 4096, location);
-		if (written != 4096)
-			ret = -EIO;
-		location += 4096;
-	}
-	return ret;
-}
-
 static int is_ssd(const char *file)
 {
 	blkid_probe probe;
@@ -1429,9 +1302,6 @@ int main(int argc, char **argv)
 	int force_overwrite = 0;
 	char *source_dir = NULL;
 	int source_dir_set = 0;
-	u64 num_of_meta_chunks = 0;
-	u64 size_of_data = 0;
-	u64 source_dir_size = 0;
 	int dev_cnt = 0;
 	int saved_optind;
 	char fs_uuid[BTRFS_UUID_UNPARSED_SIZE] = { 0 };
@@ -1682,44 +1552,27 @@ int main(int argc, char **argv)
 		 * (btrfs_register_one_device() fails if O_EXCL is on)
 		 */
 		fd = open(file, O_RDWR);
-		if (fd < 0) {
-			error("unable to open %s: %s", file, strerror(errno));
-			goto error;
-		}
-		ret = btrfs_prepare_device(fd, file, &dev_block_count,
-				block_count,
-				(zero_end ? PREP_DEVICE_ZERO_END : 0) |
-				(discard ? PREP_DEVICE_DISCARD : 0) |
-				(verbose ? PREP_DEVICE_VERBOSE : 0));
-		if (ret) {
-			goto error;
-		}
-		if (block_count && block_count > dev_block_count) {
-			error("%s is smaller than requested size, expected %llu, found %llu",
-					file,
-					(unsigned long long)block_count,
-					(unsigned long long)dev_block_count);
-			goto error;
-		}
 	} else {
 		fd = open(file, O_CREAT | O_RDWR,
 				S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH);
-		if (fd < 0) {
-			error("unable to open %s: %s", file, strerror(errno));
-			goto error;
-		}
+	}
+	if (fd < 0) {
+		error("unable to open %s: %s", file, strerror(errno));
+		goto error;
+	}
+	ret = btrfs_prepare_device(fd, file, &dev_block_count,
+			block_count,
+			(zero_end ? PREP_DEVICE_ZERO_END : 0) |
+			(discard ? PREP_DEVICE_DISCARD : 0) |
+			(verbose ? PREP_DEVICE_VERBOSE : 0));
+	if (ret)
+		goto error;
 
-		source_dir_size = size_sourcedir(source_dir, sectorsize,
-					     &num_of_meta_chunks, &size_of_data);
-		if(block_count < source_dir_size)
-			block_count = source_dir_size;
-		ret = zero_output_file(fd, block_count);
-		if (ret) {
-			error("unable to zero the output file");
-			goto error;
-		}
-		/* our "device" is the new image file */
-		dev_block_count = block_count;
+	if (block_count && block_count > dev_block_count) {
+		error("%s is smaller than requested size, expected %llu, found %llu",
+			file, (unsigned long long)block_count,
+			(unsigned long long)dev_block_count);
+		goto error;
 	}
 
 	/* To create the first block group and chunk 0 in make_btrfs */
@@ -1845,13 +1698,11 @@ int main(int argc, char **argv)
 	}
 
 raid_groups:
-	if (!source_dir_set) {
-		ret = create_raid_groups(trans, root, data_profile,
+	ret = create_raid_groups(trans, root, data_profile,
 				 metadata_profile, mixed, &allocation);
-		if (ret) {
-			error("unable to create raid groups: %d", ret);
-			goto out;
-		}
+	if (ret) {
+		error("unable to create raid groups: %d", ret);
+		goto out;
 	}
 
 	ret = create_data_reloc_tree(trans, root);
@@ -1866,33 +1717,20 @@ raid_groups:
 		goto out;
 	}
 
-	if (source_dir_set) {
-		trans = btrfs_start_transaction(root, 1);
-		ret = create_chunks(trans, root,
-				    num_of_meta_chunks, size_of_data,
-				    &allocation);
-		if (ret) {
-			error("unable to create chunks: %d", ret);
-			goto out;
-		}
-		ret = btrfs_commit_transaction(trans, root);
-		if (ret) {
-			error("transaction commit failed: %d", ret);
-			goto out;
-		}
+	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
+				  metadata_profile, metadata_profile);
+	if (ret < 0) {
+		error("failed to cleanup temporary chunks: %d", ret);
+		goto out;
+	}
 
+	if (source_dir_set) {
 		ret = make_image(source_dir, root);
 		if (ret) {
 			error("error wihle filling filesystem: %d", ret);
 			goto out;
 		}
 	}
-	ret = cleanup_temp_chunks(fs_info, &allocation, data_profile,
-				  metadata_profile, metadata_profile);
-	if (ret < 0) {
-		error("failed to cleanup temporary chunks: %d", ret);
-		goto out;
-	}
 
 	if (verbose) {
 		char features_buf[64];
-- 
2.14.1

[PATCH 3/5] btrfs-progs: mkfs: Shrink the image for rootdir to minimal size

[Qu Wenruo]

Follow the original rootdir behavior to shrink the device size to
minimal.

The shrink itself is very simple, since dev extent is allocated on
demand, we just need to shrink the device size to the device extent end
position.

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 mkfs/main.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)

diff --git a/mkfs/main.c b/mkfs/main.c
index bca836b5..c43ce18d 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -990,6 +990,107 @@ fail_no_dir:
 	goto out;
 }
 
+/*
+ * Shrink the image to its minimal size.
+ *
+ * The work is quite easy, as btrfs only allocates dev extent/chunk on demand.
+ * So we only need to do:
+ * 1) Determine the minimal image size
+ *    By checking the device extent end
+ * 2) Modify in-memory device size
+ * 3) Modify device item
+ * 4) Modify superblock
+ */
+static int shrink_image(struct btrfs_trans_handle *trans,
+			struct btrfs_fs_info *fs_info)
+{
+	struct btrfs_root *dev_root = fs_info->dev_root;
+	struct btrfs_root *chunk_root = fs_info->chunk_root;
+	struct btrfs_path path;
+	struct btrfs_key key;
+	struct btrfs_dev_extent *de;
+	struct btrfs_dev_item *di;
+	struct btrfs_device *dev;
+	u64 min_size;
+	int ret = 0;
+
+	/*
+	 * New rootdir can support multi device easily, but since only one
+	 * device is supported, devid is fixed to 1.
+	 */
+	key.objectid = 1;
+	key.type = BTRFS_DEV_EXTENT_KEY;
+	key.offset = (u64)-1;
+	btrfs_init_path(&path);
+
+	/*
+	 * Determine minimal fs size by find the end position of last dev extent
+	 */
+	ret = btrfs_search_slot(NULL, dev_root, &key, &path, 0, 0);
+	if (ret == 0) {
+		WARN_ON(1);
+		ret = -EIO;
+		goto out_release;
+	}
+	if (ret < 0)
+		goto out_release;
+
+	ret = btrfs_previous_item(dev_root, &path, key.objectid,
+				  BTRFS_DEV_EXTENT_KEY);
+	if (ret < 0)
+		goto out_release;
+	if (ret > 0) {
+		ret = -ENOENT;
+		goto out_release;
+	}
+	btrfs_item_key_to_cpu(path.nodes[0], &key, path.slots[0]);
+	de = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_dev_extent);
+	min_size = key.offset + btrfs_dev_extent_length(path.nodes[0], de);
+	btrfs_release_path(&path);
+
+	/*
+	 * Modify btrfs_device size
+	 */
+	dev = list_entry(fs_info->fs_devices->devices.next,
+			 struct btrfs_device, dev_list);
+	if (!dev) {
+		WARN_ON(1);
+		ret = -ENOENT;
+		goto out;
+	}
+	dev->total_bytes = min_size;
+
+	/*
+	 * Modify dev item size
+	 */
+	key.objectid = BTRFS_DEV_ITEMS_OBJECTID;
+	key.type = BTRFS_DEV_ITEM_KEY;
+	key.offset = 1;
+
+	ret = btrfs_search_slot(trans, chunk_root, &key, &path, 0, 1);
+	if (ret > 0) {
+		ret = -ENOENT;
+		goto out_release;
+	}
+	if (ret < 0)
+		goto out_release;
+	di = btrfs_item_ptr(path.nodes[0], path.slots[0],
+			    struct btrfs_dev_item);
+	btrfs_set_device_total_bytes(path.nodes[0], di, min_size);
+	btrfs_release_path(&path);
+
+	/*
+	 * Modify superblock size
+	 */
+	btrfs_set_super_total_bytes(fs_info->super_copy, min_size);
+
+out_release:
+	btrfs_release_path(&path);
+out:
+	return ret;
+}
+
 static int make_image(const char *source_dir, struct btrfs_root *root)
 {
 	int ret;
@@ -1013,6 +1114,12 @@ static int make_image(const char *source_dir, struct btrfs_root *root)
 		error("unable to traverse directory %s: %d", source_dir, ret);
 		goto fail;
 	}
+
+	ret = shrink_image(trans, root->fs_info);
+	if (ret < 0) {
+		error("failed to shrink image: %d", ret);
+		goto out;
+	}
 	ret = btrfs_commit_transaction(trans, root);
 	if (ret) {
 		error("transaction commit failed: %d", ret);
-- 
2.14.1

[PATCH 4/5] btrfs-progs: mkfs: Update allocation info before verbose output

[Qu Wenruo]

Since new --rootdir can allocate chunk, it will modify the chunk
allocation result.

This patch will update allocation info before verbose output to reflect
such info.

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 mkfs/main.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/mkfs/main.c b/mkfs/main.c
index c43ce18d..873aec83 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -1380,6 +1380,38 @@ out:
 	return ret;
 }
 
+/*
+ * Just update chunk allocation info, since --rootdir may allocate new
+ * chunks which is not updated in @allocation structure.
+ */
+static void update_chunk_allocation(struct btrfs_fs_info *fs_info,
+				    struct mkfs_allocation *allocation)
+{
+	struct btrfs_block_group_cache *bg_cache;
+	u64 mixed_flag = BTRFS_BLOCK_GROUP_DATA | BTRFS_BLOCK_GROUP_METADATA;
+	u64 search_start = 0;
+
+	allocation->mixed = 0;
+	allocation->data = 0;
+	allocation->metadata = 0;
+	allocation->system = 0;
+	while (1) {
+		bg_cache = btrfs_lookup_first_block_group(fs_info,
+							  search_start);
+		if (!bg_cache)
+			break;
+		if ((bg_cache->flags & mixed_flag) == mixed_flag)
+			allocation->mixed += bg_cache->key.offset;
+		else if (bg_cache->flags & BTRFS_BLOCK_GROUP_DATA)
+			allocation->data += bg_cache->key.offset;
+		else if (bg_cache->flags & BTRFS_BLOCK_GROUP_METADATA)
+			allocation->metadata += bg_cache->key.offset;
+		else
+			allocation->system += bg_cache->key.offset;
+		search_start = bg_cache->key.objectid + bg_cache->key.offset;
+	}
+}
+
 int main(int argc, char **argv)
 {
 	char *file;
@@ -1842,6 +1874,7 @@ raid_groups:
 	if (verbose) {
 		char features_buf[64];
 
+		update_chunk_allocation(fs_info, &allocation);
 		printf("Label:              %s\n", label);
 		printf("UUID:               %s\n", mkfs_cfg.fs_uuid);
 		printf("Node size:          %u\n", nodesize);
-- 
2.14.1

[PATCH 5/5] btrfs-progs: Doc/mkfs: Add explanation for rootdir parameter

[Qu Wenruo]

Original --rootdir parameter will shrink the filesystem to its minimal
size, which is quite confusing for some users.

Add extra note for this behavior.

Reported-by: Goffredo Baroncelli <kreijack@libero.it>
Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 Documentation/mkfs.btrfs.asciidoc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/mkfs.btrfs.asciidoc b/Documentation/mkfs.btrfs.asciidoc
index d53d9e26..8977761f 100644
--- a/Documentation/mkfs.btrfs.asciidoc
+++ b/Documentation/mkfs.btrfs.asciidoc
@@ -106,6 +106,9 @@ Please see the mount option 'discard' for that in `btrfs`(5).
 *-r|--rootdir <rootdir>*::
 Populate the toplevel subvolume with files from 'rootdir'.  This does not
 require root permissions and does not mount the filesystem.
++
+NOTE: Result btrfs will be shrink to it minimal size, exceeding space will not
+be accessible unless resized.
 
 *-O|--features <feature1>[,<feature2>...]*::
 A list of filesystem features turned on at mkfs time. Not all features are
-- 
2.14.1

Re: [PATCH 5/5] btrfs-progs: Doc/mkfs: Add explanation for rootdir parameter

[Duncan]

Qu Wenruo posted on Mon, 04 Sep 2017 15:41:10 +0900 as excerpted:

> +NOTE: Result btrfs will be shrink to it minimal size, exceeding space will not
> +be accessible unless resized.

s/exceeding/additional/

(Exceeding could be used if reworded, but the results I came up with were
much less concise than the above with the single substitution, so...)

-- 
Duncan - List replies preferred.   No HTML msgs.
"Every nonfree program has a lord, a master --
and if you use the program, he is your master."  Richard Stallman

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[David Sterba]

On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
> mkfs.btrfs --rootdir provides user a method to generate btrfs with
> pre-written content while without the need of root privilege.
> 
> However the code is quite old and doesn't get much review or test.
> This makes some strange behavior, from customized chunk allocation
> (which uses the reserved 0~1M device space) to lack of special file
> handler (Fixed in previous 2 patches).

The cleanup in this area is most welcome. The patches look good after a
quick look, I'll do another review round.

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Qu Wenruo]

On 2017年09月05日 02:08, David Sterba wrote:
> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>> pre-written content while without the need of root privilege.
>>
>> However the code is quite old and doesn't get much review or test.
>> This makes some strange behavior, from customized chunk allocation
>> (which uses the reserved 0~1M device space) to lack of special file
>> handler (Fixed in previous 2 patches).
> 
> The cleanup in this area is most welcome. The patches look good after a
> quick look, I'll do another review round.

To save you some time, I found that my rework can't create new image 
which old --rootdir can do. So it's still not completely the same behavior.
I can fix it by creating a large sparse file first and then truncate it 
using current method easily.

But this really concerns me, do we need to shrink the fs?

I had a discussion with Austin about this, thread named "[btrfs-progs] 
Bug in mkfs.btrfs -r".
The only equivalent I found is "mkfs.ext4 -d", which can only create new 
file if size is given and will not shrink fs.
(Genext2fs shrinks the fs, but is no longer in e2fsprogs)

If we follow that behavior, the 3rd and 5th patches are not needed, 
which I'm pretty happy with.

Functionally, both behavior can be implemented with current method, but 
I hope to make sure which is the designed behavior so I can stick to it.

I hope you could make the final decision on this so I can update the 
patchset.

Thanks,
Qu

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

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Goffredo Baroncelli]

On 09/05/2017 10:19 AM, Qu Wenruo wrote:
> 
> 
> On 2017年09月05日 02:08, David Sterba wrote:
>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>> pre-written content while without the need of root privilege.
>>>
>>> However the code is quite old and doesn't get much review or test.
>>> This makes some strange behavior, from customized chunk allocation
>>> (which uses the reserved 0~1M device space) to lack of special file
>>> handler (Fixed in previous 2 patches).
>>
>> The cleanup in this area is most welcome. The patches look good after a
>> quick look, I'll do another review round.
> 
> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
> I can fix it by creating a large sparse file first and then truncate it using current method easily.
> 
> But this really concerns me, do we need to shrink the fs?

I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar...."; 

in the first case I have to do
a1) mkfs.btrfs --root-dir....  (create the archive)
a2) dd  (copy and truncate the image and store it in the archive)
a3) dd  (take the archived image, and restore it)
a4) btrfs fi resize (expand the image)

in the second case I have to 
b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
b2) mkfs,btrfs (create the filesystem with the final size)
b3) tar xf ... (take the archived image and restore it)


However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.

mkfs.btrfs --root ...                --> populate the filesystem
mkfs.btrfs --shrink --root           --> populate and shrink the filesystem 

however I find this useful only if it is possible to creating the filesystem in a file; ie.

mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>

where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after 
a) <file-to-be-created> contains the image
b) <file-to-be-created> is the smallest possible size.

Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).

BTW I compiled successfully the patches, and these seems to work. 

PS: I tried to cross-compile mkfs.btrfs ton arm, but mkfs.btrfs was unable to work:

$ uname -a
Linux bananapi 4.4.66-bananian #2 SMP Sat May 6 19:26:50 UTC 2017 armv7l GNU/Linux
$ sudo ./mkfs.btrfs /dev/loop0
btrfs-progs v4.12.1-5-g3c9451cd
See http://btrfs.wiki.kernel.org for more information.

ERROR: superblock magic doesn't match
Performing full device TRIM /dev/loop0 (10.00GiB) ...
ERROR: open ctree failed

However this problem exists even with a plain v4.12.1. The first error seems to suggest that there is some endian-ness issue

BR
G.Baroncelli

> 
> I had a discussion with Austin about this, thread named "[btrfs-progs] Bug in mkfs.btrfs -r".
> The only equivalent I found is "mkfs.ext4 -d", which can only create new file if size is given and will not shrink fs.
> (Genext2fs shrinks the fs, but is no longer in e2fsprogs)
> 
> If we follow that behavior, the 3rd and 5th patches are not needed, which I'm pretty happy with.
> 
> Functionally, both behavior can be implemented with current method, but I hope to make sure which is the designed behavior so I can stick to it.
> 
> I hope you could make the final decision on this so I can update the patchset.
> 
> Thanks,
> Qu
> 
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Qu Wenruo]

On 2017年09月06日 03:05, Goffredo Baroncelli wrote:
> On 09/05/2017 10:19 AM, Qu Wenruo wrote:
>>
>>
>> On 2017年09月05日 02:08, David Sterba wrote:
>>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>>> pre-written content while without the need of root privilege.
>>>>
>>>> However the code is quite old and doesn't get much review or test.
>>>> This makes some strange behavior, from customized chunk allocation
>>>> (which uses the reserved 0~1M device space) to lack of special file
>>>> handler (Fixed in previous 2 patches).
>>>
>>> The cleanup in this area is most welcome. The patches look good after a
>>> quick look, I'll do another review round.
>>
>> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
>> I can fix it by creating a large sparse file first and then truncate it using current method easily.
>>
>> But this really concerns me, do we need to shrink the fs?
> 
> I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar....";
> 
> in the first case I have to do
> a1) mkfs.btrfs --root-dir....  (create the archive)
> a2) dd  (copy and truncate the image and store it in the archive)
> a3) dd  (take the archived image, and restore it)
> a4) btrfs fi resize (expand the image)
> 
> in the second case I have to
> b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
> b2) mkfs,btrfs (create the filesystem with the final size)
> b3) tar xf ... (take the archived image and restore it)
> 
> 
> However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.
> 
> mkfs.btrfs --root ...                --> populate the filesystem
> mkfs.btrfs --shrink --root           --> populate and shrink the filesystem
> 
> however I find this useful only if it is possible to creating the filesystem in a file; ie.
> 
> mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>
> 
> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
> a) <file-to-be-created> contains the image
> b) <file-to-be-created> is the smallest possible size.

Yes, that's the original behavior. And what my rework can't do yet.
It can't determine the size of the device, so it can't continue.

If we decide to follow the original behavior, then I have to create 
sparse file first and truncate the file at the end.
But still quite easy to do.

And if we decide to follow mkfs.ext -d behavior, then I just need to 
remove 2 patches from the patchset (shrink patch and doc patch, which 
adds about 100 lines), and slightly modify the rework patch to remove 
the O_CREATE open flag.

> 
> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
> 
> BTW I compiled successfully the patches, and these seems to work.
> 
> PS: I tried to cross-compile mkfs.btrfs ton arm, but mkfs.btrfs was unable to work:
> 
> $ uname -a
> Linux bananapi 4.4.66-bananian #2 SMP Sat May 6 19:26:50 UTC 2017 armv7l GNU/Linux
> $ sudo ./mkfs.btrfs /dev/loop0
> btrfs-progs v4.12.1-5-g3c9451cd
> See http://btrfs.wiki.kernel.org for more information.
> 
> ERROR: superblock magic doesn't match
> Performing full device TRIM /dev/loop0 (10.00GiB) ...
> ERROR: open ctree failed
> 
> However this problem exists even with a plain v4.12.1. The first error seems to suggest that there is some endian-ness issue

I'd better get one cheap ARM board if I want to do native debug.

BTW, what's the output of dump-super here?
Which may gives us some clue to fix it.

Thanks,
Qu

> 
> BR
> G.Baroncelli
> 
>>
>> I had a discussion with Austin about this, thread named "[btrfs-progs] Bug in mkfs.btrfs -r".
>> The only equivalent I found is "mkfs.ext4 -d", which can only create new file if size is given and will not shrink fs.
>> (Genext2fs shrinks the fs, but is no longer in e2fsprogs)
>>
>> If we follow that behavior, the 3rd and 5th patches are not needed, which I'm pretty happy with.
>>
>> Functionally, both behavior can be implemented with current method, but I hope to make sure which is the designed behavior so I can stick to it.
>>
>> I hope you could make the final decision on this so I can update the patchset.
>>
>> Thanks,
>> Qu
>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Austin S. Hemmelgarn]

On 2017-09-05 15:05, Goffredo Baroncelli wrote:
> On 09/05/2017 10:19 AM, Qu Wenruo wrote:
>>
>>
>> On 2017年09月05日 02:08, David Sterba wrote:
>>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>>> pre-written content while without the need of root privilege.
>>>>
>>>> However the code is quite old and doesn't get much review or test.
>>>> This makes some strange behavior, from customized chunk allocation
>>>> (which uses the reserved 0~1M device space) to lack of special file
>>>> handler (Fixed in previous 2 patches).
>>>
>>> The cleanup in this area is most welcome. The patches look good after a
>>> quick look, I'll do another review round.
>>
>> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
>> I can fix it by creating a large sparse file first and then truncate it using current method easily.
>>
>> But this really concerns me, do we need to shrink the fs?
> 
> I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar....";
> 
> in the first case I have to do
> a1) mkfs.btrfs --root-dir....  (create the archive)
> a2) dd  (copy and truncate the image and store it in the archive)
> a3) dd  (take the archived image, and restore it)
> a4) btrfs fi resize (expand the image)
The primary use case for this is to generate installation images.  Using 
this method removes the need for tar in the installation environment, 
and if you defer step a4 until the first boot of the system, it also 
removes the need to have btrfs-progs in the installation environment. 
Taken together, that's a pretty significant space savings.

It's also somewhat useful for creating minimalistic seed device images, 
which have a couple of interesting uses, namely:

* Base system images for 'factory reset'.  The general principal is 
simple, your base system is a seed device, plus a storage device 
associated with it.  When you want to do a factory reset, you wipe the 
storage partition, and recreate an empty one associated with the seed 
image.  This usage pretty much requires a minimally sized filesystem, as 
anything more wastes space that would be otherwise usable by the end user.

* Seed-device based install images.  The general concept for this has 
been tossed around a couple of times before.  You start with a minimal 
seed device, boot to a live system using that and a temporary in-memory 
device for root, set up the persistent storage, re-balance everything to 
persistent storage, then remove the seed device and in-memory device so 
the user can keep using the system without needing to reboot.  This also 
needs a minimalistic image, for the same reason any install disc needs 
to have a minimal base image.

Note that without resize being able to shrink chunks (and ideally 
completely shrink them so there is no slack space in the FS), you have 
to use a hex editor to get a truly minimalistic filesystem image.
> 
> in the second case I have to
> b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
> b2) mkfs,btrfs (create the filesystem with the final size)
> b3) tar xf ... (take the archived image and restore it)
> 
> 
> However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.
> 
> mkfs.btrfs --root ...                --> populate the filesystem
> mkfs.btrfs --shrink --root           --> populate and shrink the filesystem
> 
> however I find this useful only if it is possible to creating the filesystem in a file; ie.
> 
> mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>
> 
> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
> a) <file-to-be-created> contains the image
> b) <file-to-be-created> is the smallest possible size.
> 
> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
FWIW, the current release behavior doesn't require the truncate, and 
properly generates the file for the filesystem.  It also does some odd 
things with chunk placement (including putting data in the 0-1M range 
which is supposed to be reserved), and that odd behavior is primarily 
what prompted this patch set.
> 
> BTW I compiled successfully the patches, and these seems to work.
> 
> PS: I tried to cross-compile mkfs.btrfs ton arm, but mkfs.btrfs was unable to work:
> 
> $ uname -a
> Linux bananapi 4.4.66-bananian #2 SMP Sat May 6 19:26:50 UTC 2017 armv7l GNU/Linux
> $ sudo ./mkfs.btrfs /dev/loop0
> btrfs-progs v4.12.1-5-g3c9451cd
> See http://btrfs.wiki.kernel.org for more information.
> 
> ERROR: superblock magic doesn't match
> Performing full device TRIM /dev/loop0 (10.00GiB) ...
> ERROR: open ctree failed
> 
> However this problem exists even with a plain v4.12.1. The first error seems to suggest that there is some endian-ness issue
> 
> BR
> G.Baroncelli
> 
>>
>> I had a discussion with Austin about this, thread named "[btrfs-progs] Bug in mkfs.btrfs -r".
>> The only equivalent I found is "mkfs.ext4 -d", which can only create new file if size is given and will not shrink fs.
>> (Genext2fs shrinks the fs, but is no longer in e2fsprogs)
>>
>> If we follow that behavior, the 3rd and 5th patches are not needed, which I'm pretty happy with.
>>
>> Functionally, both behavior can be implemented with current method, but I hope to make sure which is the designed behavior so I can stick to it.
>>
>> I hope you could make the final decision on this so I can update the patchset.

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Goffredo Baroncelli]

On 09/06/2017 01:31 PM, Austin S. Hemmelgarn wrote:
> On 2017-09-05 15:05, Goffredo Baroncelli wrote:
>> On 09/05/2017 10:19 AM, Qu Wenruo wrote:
>>>
>>>
>>> On 2017年09月05日 02:08, David Sterba wrote:
>>>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>>>> pre-written content while without the need of root privilege.
>>>>>
>>>>> However the code is quite old and doesn't get much review or test.
>>>>> This makes some strange behavior, from customized chunk allocation
>>>>> (which uses the reserved 0~1M device space) to lack of special file
>>>>> handler (Fixed in previous 2 patches).
>>>>
>>>> The cleanup in this area is most welcome. The patches look good after a
>>>> quick look, I'll do another review round.
>>>
>>> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
>>> I can fix it by creating a large sparse file first and then truncate it using current method easily.
>>>
>>> But this really concerns me, do we need to shrink the fs?
>>
>> I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar....";
>>
>> in the first case I have to do
>> a1) mkfs.btrfs --root-dir....  (create the archive)
>> a2) dd  (copy and truncate the image and store it in the archive)
>> a3) dd  (take the archived image, and restore it)
>> a4) btrfs fi resize (expand the image)
> The primary use case for this is to generate installation images.  Using this method removes the need for tar in the installation environment, and if you defer step a4 until the first boot of the system, it also removes the need to have btrfs-progs in the installation environment. Taken together, that's a pretty significant space savings.

Sorry but I don't understand. If you reach the step a3; you have:
- the final disk, and an environment fully working. So I am still inclined to think that using "mkfs.btrfs --root-dir" is more complicated in *this case*.
> 
> It's also somewhat useful for creating minimalistic seed device images, which have a couple of interesting uses, namely:
> 
> * Base system images for 'factory reset'.  The general principal is simple, your base system is a seed device, plus a storage device associated with it.  When you want to do a factory reset, you wipe the storage partition, and recreate an empty one associated with the seed image.  This usage pretty much requires a minimally sized filesystem, as anything more wastes space that would be otherwise usable by the end user.
> 
> * Seed-device based install images.  The general concept for this has been tossed around a couple of times before.  You start with a minimal seed device, boot to a live system using that and a temporary in-memory device for root, set up the persistent storage, re-balance everything to persistent storage, then remove the seed device and in-memory device so the user can keep using the system without needing to reboot.  This also needs a minimalistic image, for the same reason any install disc needs to have a minimal base image.

For the above cases I agree that this could be useful to have "--rootdir"
> 
> Note that without resize being able to shrink chunks (and ideally completely shrink them so there is no slack space in the FS), you have to use a hex editor to get a truly minimalistic filesystem image.
>>
>> in the second case I have to
>> b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
>> b2) mkfs,btrfs (create the filesystem with the final size)
>> b3) tar xf ... (take the archived image and restore it)
>>
>>
>> However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.
>>
>> mkfs.btrfs --root ...                --> populate the filesystem
>> mkfs.btrfs --shrink --root           --> populate and shrink the filesystem
>>
>> however I find this useful only if it is possible to creating the filesystem in a file; ie.
>>
>> mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>
>>
>> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
>> a) <file-to-be-created> contains the image
>> b) <file-to-be-created> is the smallest possible size.
>>
>> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
> FWIW, the current release behavior doesn't require the truncate, and properly generates the file for the filesystem.  
If you don't do truncate, you have the fully partition... Or there is something that I miss ?

[...]
-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Austin S. Hemmelgarn]

On 2017-09-06 12:43, Goffredo Baroncelli wrote:
> On 09/06/2017 01:31 PM, Austin S. Hemmelgarn wrote:
>> On 2017-09-05 15:05, Goffredo Baroncelli wrote:
>>> On 09/05/2017 10:19 AM, Qu Wenruo wrote:
>>>>
>>>>
>>>> On 2017年09月05日 02:08, David Sterba wrote:
>>>>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>>>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>>>>> pre-written content while without the need of root privilege.
>>>>>>
>>>>>> However the code is quite old and doesn't get much review or test.
>>>>>> This makes some strange behavior, from customized chunk allocation
>>>>>> (which uses the reserved 0~1M device space) to lack of special file
>>>>>> handler (Fixed in previous 2 patches).
>>>>>
>>>>> The cleanup in this area is most welcome. The patches look good after a
>>>>> quick look, I'll do another review round.
>>>>
>>>> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
>>>> I can fix it by creating a large sparse file first and then truncate it using current method easily.
>>>>
>>>> But this really concerns me, do we need to shrink the fs?
>>>
>>> I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar....";
>>>
>>> in the first case I have to do
>>> a1) mkfs.btrfs --root-dir....  (create the archive)
>>> a2) dd  (copy and truncate the image and store it in the archive)
>>> a3) dd  (take the archived image, and restore it)
>>> a4) btrfs fi resize (expand the image)
>> The primary use case for this is to generate installation images.  Using this method removes the need for tar in the installation environment, and if you defer step a4 until the first boot of the system, it also removes the need to have btrfs-progs in the installation environment. Taken together, that's a pretty significant space savings.
> 
> Sorry but I don't understand. If you reach the step a3; you have:
> - the final disk, and an environment fully working. So I am still inclined to think that using "mkfs.btrfs --root-dir" is more complicated in *this case*.
With the current released code (without these patches), `-r` can be used 
to generate a filesystem image that has zero free space.  In that case, 
step a3 does not give you a fully working environment, it gives you a 
basic environment that can probably be booted to a minimal degree and 
run just enough to be able to resize the filesystem to take up the whole 
disk.

IOW, the use case I'm referring to here is more akin to that of 
genext2fs -d than mkfs.ext4 -d, although mkfs.btrfs -r generates a 
minimally sized FS right now, while neither of the other two do so by 
default (although I could have sworn that genext2fs could).
>>
>> It's also somewhat useful for creating minimalistic seed device images, which have a couple of interesting uses, namely:
>>
>> * Base system images for 'factory reset'.  The general principal is simple, your base system is a seed device, plus a storage device associated with it.  When you want to do a factory reset, you wipe the storage partition, and recreate an empty one associated with the seed image.  This usage pretty much requires a minimally sized filesystem, as anything more wastes space that would be otherwise usable by the end user.
>>
>> * Seed-device based install images.  The general concept for this has been tossed around a couple of times before.  You start with a minimal seed device, boot to a live system using that and a temporary in-memory device for root, set up the persistent storage, re-balance everything to persistent storage, then remove the seed device and in-memory device so the user can keep using the system without needing to reboot.  This also needs a minimalistic image, for the same reason any install disc needs to have a minimal base image.
> 
> For the above cases I agree that this could be useful to have "--rootdir"
>>
>> Note that without resize being able to shrink chunks (and ideally completely shrink them so there is no slack space in the FS), you have to use a hex editor to get a truly minimalistic filesystem image.
>>>
>>> in the second case I have to
>>> b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
>>> b2) mkfs,btrfs (create the filesystem with the final size)
>>> b3) tar xf ... (take the archived image and restore it)
>>>
>>>
>>> However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.
>>>
>>> mkfs.btrfs --root ...                --> populate the filesystem
>>> mkfs.btrfs --shrink --root           --> populate and shrink the filesystem
>>>
>>> however I find this useful only if it is possible to creating the filesystem in a file; ie.
>>>
>>> mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>
>>>
>>> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
>>> a) <file-to-be-created> contains the image
>>> b) <file-to-be-created> is the smallest possible size.
>>>
>>> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
>> FWIW, the current release behavior doesn't require the truncate, and properly generates the file for the filesystem.
> If you don't do truncate, you have the fully partition... Or there is something that I miss ?
The current release, without these patches, run using a non-existent 
file and the `-r` option, will produce a filesystem image of the exact 
size needed to hold everything in the directory passed to `-r`.  It 
doesn't require truncation unless used on a file that already exists.

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Goffredo Baroncelli]

On 09/06/2017 07:16 PM, Austin S. Hemmelgarn wrote:
> On 2017-09-06 12:43, Goffredo Baroncelli wrote:
>> On 09/06/2017 01:31 PM, Austin S. Hemmelgarn wrote:
>>> On 2017-09-05 15:05, Goffredo Baroncelli wrote:
>>>> On 09/05/2017 10:19 AM, Qu Wenruo wrote:
>>>>>
>>>>>
>>>>> On 2017年09月05日 02:08, David Sterba wrote:
>>>>>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>>>>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>>>>>> pre-written content while without the need of root privilege.
>>>>>>>
>>>>>>> However the code is quite old and doesn't get much review or test.
>>>>>>> This makes some strange behavior, from customized chunk allocation
>>>>>>> (which uses the reserved 0~1M device space) to lack of special file
>>>>>>> handler (Fixed in previous 2 patches).
>>>>>>
>>>>>> The cleanup in this area is most welcome. The patches look good after a
>>>>>> quick look, I'll do another review round.
>>>>>
>>>>> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
>>>>> I can fix it by creating a large sparse file first and then truncate it using current method easily.
>>>>>
>>>>> But this really concerns me, do we need to shrink the fs?
>>>>
>>>> I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar....";
>>>>
>>>> in the first case I have to do
>>>> a1) mkfs.btrfs --root-dir....  (create the archive)
>>>> a2) dd  (copy and truncate the image and store it in the archive)
>>>> a3) dd  (take the archived image, and restore it)
>>>> a4) btrfs fi resize (expand the image)
>>> The primary use case for this is to generate installation images.  Using this method removes the need for tar in the installation environment, and if you defer step a4 until the first boot of the system, it also removes the need to have btrfs-progs in the installation environment. Taken together, that's a pretty significant space savings.
>>
>> Sorry but I don't understand. If you reach the step a3; you have:
>> - the final disk, and an environment fully working. So I am still inclined to think that using "mkfs.btrfs --root-dir" is more complicated in *this case*.
> With the current released code (without these patches), `-r` can be used to generate a filesystem image that has zero free space.  In that case, step a3 does not give you a fully working environment,

True, this doesn't *give* you a filly working environment, _but_ you perform the step a3 in a "fully working environment", an you have at hand the target disk..

What I am saying is that both step a1..a3 and b1..b3 are done in a "fully working environment". The only difference is that you will need a further step a4

> it gives you a basic environment that can probably be booted to a minimal degree and run just enough to be able to resize the filesystem to take up the whole disk.
> 
> IOW, the use case I'm referring to here is more akin to that of genext2fs -d than mkfs.ext4 -d, although mkfs.btrfs -r generates a minimally sized FS right now, while neither of the other two do so by default (although I could have sworn that genext2fs could).
>>>
>>> It's also somewhat useful for creating minimalistic seed device images, which have a couple of interesting uses, namely:
>>>
>>> * Base system images for 'factory reset'.  The general principal is simple, your base system is a seed device, plus a storage device associated with it.  When you want to do a factory reset, you wipe the storage partition, and recreate an empty one associated with the seed image.  This usage pretty much requires a minimally sized filesystem, as anything more wastes space that would be otherwise usable by the end user.
>>>
>>> * Seed-device based install images.  The general concept for this has been tossed around a couple of times before.  You start with a minimal seed device, boot to a live system using that and a temporary in-memory device for root, set up the persistent storage, re-balance everything to persistent storage, then remove the seed device and in-memory device so the user can keep using the system without needing to reboot.  This also needs a minimalistic image, for the same reason any install disc needs to have a minimal base image.
>>
>> For the above cases I agree that this could be useful to have "--rootdir"
>>>
>>> Note that without resize being able to shrink chunks (and ideally completely shrink them so there is no slack space in the FS), you have to use a hex editor to get a truly minimalistic filesystem image.
>>>>
>>>> in the second case I have to
>>>> b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
>>>> b2) mkfs,btrfs (create the filesystem with the final size)
>>>> b3) tar xf ... (take the archived image and restore it)
>>>>
>>>>
>>>> However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.
>>>>
>>>> mkfs.btrfs --root ...                --> populate the filesystem
>>>> mkfs.btrfs --shrink --root           --> populate and shrink the filesystem
>>>>
>>>> however I find this useful only if it is possible to creating the filesystem in a file; ie.
>>>>
>>>> mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>
>>>>
>>>> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
>>>> a) <file-to-be-created> contains the image
>>>> b) <file-to-be-created> is the smallest possible size.
>>>>
>>>> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
>>> FWIW, the current release behavior doesn't require the truncate, and properly generates the file for the filesystem.
>> If you don't do truncate, you have the fully partition... Or there is something that I miss ?
> The current release, without these patches, run using a non-existent file and the `-r` option, will produce a filesystem image of the exact size needed to hold everything in the directory passed to `-r`.  It doesn't require truncation unless used on a file that already exists.

Of course the truncate is not needed, because you are using a sparse file. But if you use a sparse file..... it is not even needed the shrinking! Because the file will consume the same space on the disk !
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Austin S. Hemmelgarn]

On 2017-09-06 13:48, Goffredo Baroncelli wrote:
> On 09/06/2017 07:16 PM, Austin S. Hemmelgarn wrote:
>> On 2017-09-06 12:43, Goffredo Baroncelli wrote:
>>> On 09/06/2017 01:31 PM, Austin S. Hemmelgarn wrote:
>>>> On 2017-09-05 15:05, Goffredo Baroncelli wrote:
>>>>> On 09/05/2017 10:19 AM, Qu Wenruo wrote:
>>>>>>
>>>>>>
>>>>>> On 2017年09月05日 02:08, David Sterba wrote:
>>>>>>> On Mon, Sep 04, 2017 at 03:41:05PM +0900, Qu Wenruo wrote:
>>>>>>>> mkfs.btrfs --rootdir provides user a method to generate btrfs with
>>>>>>>> pre-written content while without the need of root privilege.
>>>>>>>>
>>>>>>>> However the code is quite old and doesn't get much review or test.
>>>>>>>> This makes some strange behavior, from customized chunk allocation
>>>>>>>> (which uses the reserved 0~1M device space) to lack of special file
>>>>>>>> handler (Fixed in previous 2 patches).
>>>>>>>
>>>>>>> The cleanup in this area is most welcome. The patches look good after a
>>>>>>> quick look, I'll do another review round.
>>>>>>
>>>>>> To save you some time, I found that my rework can't create new image which old --rootdir can do. So it's still not completely the same behavior.
>>>>>> I can fix it by creating a large sparse file first and then truncate it using current method easily.
>>>>>>
>>>>>> But this really concerns me, do we need to shrink the fs?
>>>>>
>>>>> I still fatigue to understand in what "mkfs.btrfs --rootdir" would be better than a "simple tar....";
>>>>>
>>>>> in the first case I have to do
>>>>> a1) mkfs.btrfs --root-dir....  (create the archive)
>>>>> a2) dd  (copy and truncate the image and store it in the archive)
>>>>> a3) dd  (take the archived image, and restore it)
>>>>> a4) btrfs fi resize (expand the image)
>>>> The primary use case for this is to generate installation images.  Using this method removes the need for tar in the installation environment, and if you defer step a4 until the first boot of the system, it also removes the need to have btrfs-progs in the installation environment. Taken together, that's a pretty significant space savings.
>>>
>>> Sorry but I don't understand. If you reach the step a3; you have:
>>> - the final disk, and an environment fully working. So I am still inclined to think that using "mkfs.btrfs --root-dir" is more complicated in *this case*.
>> With the current released code (without these patches), `-r` can be used to generate a filesystem image that has zero free space.  In that case, step a3 does not give you a fully working environment,
> 
> True, this doesn't *give* you a filly working environment, _but_ you perform the step a3 in a "fully working environment", an you have at hand the target disk..
You could just as easily be booted into a minimalistic install 
environment, and if you netbooted that, then it's pretty likely that you 
want it as small as possible, and not needing tar or btrfs-progs for the 
actual install will save a lot of space (multiple MB doesn't sound like 
much, but when you're dealing with a tiny system to begin with, it can 
be very significant).
> 
> What I am saying is that both step a1..a3 and b1..b3 are done in a "fully working environment". The only difference is that you will need a further step a4
> 
>> it gives you a basic environment that can probably be booted to a minimal degree and run just enough to be able to resize the filesystem to take up the whole disk.
>>
>> IOW, the use case I'm referring to here is more akin to that of genext2fs -d than mkfs.ext4 -d, although mkfs.btrfs -r generates a minimally sized FS right now, while neither of the other two do so by default (although I could have sworn that genext2fs could).
>>>>
>>>> It's also somewhat useful for creating minimalistic seed device images, which have a couple of interesting uses, namely:
>>>>
>>>> * Base system images for 'factory reset'.  The general principal is simple, your base system is a seed device, plus a storage device associated with it.  When you want to do a factory reset, you wipe the storage partition, and recreate an empty one associated with the seed image.  This usage pretty much requires a minimally sized filesystem, as anything more wastes space that would be otherwise usable by the end user.
>>>>
>>>> * Seed-device based install images.  The general concept for this has been tossed around a couple of times before.  You start with a minimal seed device, boot to a live system using that and a temporary in-memory device for root, set up the persistent storage, re-balance everything to persistent storage, then remove the seed device and in-memory device so the user can keep using the system without needing to reboot.  This also needs a minimalistic image, for the same reason any install disc needs to have a minimal base image.
>>>
>>> For the above cases I agree that this could be useful to have "--rootdir"
>>>>
>>>> Note that without resize being able to shrink chunks (and ideally completely shrink them so there is no slack space in the FS), you have to use a hex editor to get a truly minimalistic filesystem image.
>>>>>
>>>>> in the second case I have to
>>>>> b1) tar cf ... (create the image an store it in the archive, this is a1+a2)
>>>>> b2) mkfs,btrfs (create the filesystem with the final size)
>>>>> b3) tar xf ... (take the archived image and restore it)
>>>>>
>>>>>
>>>>> However the code is already written (and it seems simple enough), so a possible compromise could be to have the "shrinking" only if another option is passed; eg.
>>>>>
>>>>> mkfs.btrfs --root ...                --> populate the filesystem
>>>>> mkfs.btrfs --shrink --root           --> populate and shrink the filesystem
>>>>>
>>>>> however I find this useful only if it is possible to creating the filesystem in a file; ie.
>>>>>
>>>>> mkfs.btrfs --shrink --root <path-to-source-fs> <file-to-be-created>
>>>>>
>>>>> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
>>>>> a) <file-to-be-created> contains the image
>>>>> b) <file-to-be-created> is the smallest possible size.
>>>>>
>>>>> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
>>>> FWIW, the current release behavior doesn't require the truncate, and properly generates the file for the filesystem.
>>> If you don't do truncate, you have the fully partition... Or there is something that I miss ?
>> The current release, without these patches, run using a non-existent file and the `-r` option, will produce a filesystem image of the exact size needed to hold everything in the directory passed to `-r`.  It doesn't require truncation unless used on a file that already exists.
> 
> Of course the truncate is not needed, because you are using a sparse file. But if you use a sparse file..... it is not even needed the shrinking! Because the file will consume the same space on the disk !
Unless you want to use the file elsewhere.  It's a pretty rare 
occurrence outside of testing that you generate a filesystem image and 
use it as-is without transferring it somewhere (usually onto an actual 
storage device).  Once you're talking about moving it, whether or not 
the file itself is sparse usually doesn't matter, especially if the file 
is leaving the local system by some means other than NFS or rsync.

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Goffredo Baroncelli]

On 09/06/2017 08:02 PM, Austin S. Hemmelgarn wrote:
> On 2017-09-06 13:48, Goffredo Baroncelli wrote:
>> On 09/06/2017 07:16 PM, Austin S. Hemmelgarn wrote:
[...]
>>>> Sorry but I don't understand. If you reach the step a3; you have:
>>>> - the final disk, and an environment fully working. So I am still inclined to think that using "mkfs.btrfs --root-dir" is more complicated in *this case*.
>>> With the current released code (without these patches), `-r` can be used to generate a filesystem image that has zero free space.  In that case, step a3 does not give you a fully working environment,
>>
>> True, this doesn't *give* you a filly working environment, _but_ you perform the step a3 in a "fully working environment", an you have at hand the target disk..
> You could just as easily be booted into a minimalistic install environment, and if you netbooted that, then it's pretty likely that you want it as small as possible, and not needing tar or btrfs-progs for the actual install will save a lot of space (multiple MB doesn't sound like much, but when you're dealing with a tiny system to begin with, it can be very significant).

Step a3 need to have access to the raw disk image build at step1; this is quite incompatible with a "minimalistic install environment"; and even if you have access it via net, in the same way you can have access to a fully working environment....


[...]

>>>>>>
>>>>>> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
>>>>>> a) <file-to-be-created> contains the image
>>>>>> b) <file-to-be-created> is the smallest possible size.
>>>>>>
>>>>>> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
>>>>> FWIW, the current release behavior doesn't require the truncate, and properly generates the file for the filesystem.
>>>> If you don't do truncate, you have the fully partition... Or there is something that I miss ?
>>> The current release, without these patches, run using a non-existent file and the `-r` option, will produce a filesystem image of the exact size needed to hold everything in the directory passed to `-r`.  It doesn't require truncation unless used on a file that already exists.
>>
>> Of course the truncate is not needed, because you are using a sparse file. But if you use a sparse file..... it is not even needed the shrinking! Because the file will consume the same space on the disk !

> Unless you want to use the file elsewhere.  It's a pretty rare occurrence outside of testing that you generate a filesystem image and use it as-is without transferring it somewhere (usually onto an actual storage device).  Once you're talking about moving it, whether or not the file itself is sparse usually doesn't matter, especially if the file is leaving the local system by some means other than NFS or rsync.

????
If you don't truncate you have the full-image in any case....


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


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Goffredo Baroncelli]

On 09/06/2017 05:20 AM, Qu Wenruo wrote:
>>
>> BTW I compiled successfully the patches, and these seems to work.
>>
>> PS: I tried to cross-compile mkfs.btrfs ton arm, but mkfs.btrfs was unable to work:
>>
>> $ uname -a
>> Linux bananapi 4.4.66-bananian #2 SMP Sat May 6 19:26:50 UTC 2017 armv7l GNU/Linux
>> $ sudo ./mkfs.btrfs /dev/loop0
>> btrfs-progs v4.12.1-5-g3c9451cd
>> See http://btrfs.wiki.kernel.org for more information.
>>
>> ERROR: superblock magic doesn't match
>> Performing full device TRIM /dev/loop0 (10.00GiB) ...
>> ERROR: open ctree failed
>>
>> However this problem exists even with a plain v4.12.1. The first error seems to suggest that there is some endian-ness issue
> 
> I'd better get one cheap ARM board if I want to do native debug.
> 
> BTW, what's the output of dump-super here?

Further tests seems to highlight that it was a my setup problem. Before I build mkfs.btrfs cross-compiling from an amd64, and I got the error; retrying on the native environment I was unable to reproduce the problem.

So please ignore the previous email.

> Which may gives us some clue to fix it.
> 
> Thanks,
> Qu


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5

Re: [PATCH 0/5] Mkfs: Rework --rootdir to a more generic behavior

[Austin S. Hemmelgarn]

On 2017-09-06 14:31, Goffredo Baroncelli wrote:
> On 09/06/2017 08:02 PM, Austin S. Hemmelgarn wrote:
>> On 2017-09-06 13:48, Goffredo Baroncelli wrote:
>>> On 09/06/2017 07:16 PM, Austin S. Hemmelgarn wrote:
> [...]
>>>>> Sorry but I don't understand. If you reach the step a3; you have:
>>>>> - the final disk, and an environment fully working. So I am still inclined to think that using "mkfs.btrfs --root-dir" is more complicated in *this case*.
>>>> With the current released code (without these patches), `-r` can be used to generate a filesystem image that has zero free space.  In that case, step a3 does not give you a fully working environment,
>>>
>>> True, this doesn't *give* you a filly working environment, _but_ you perform the step a3 in a "fully working environment", an you have at hand the target disk..
>> You could just as easily be booted into a minimalistic install environment, and if you netbooted that, then it's pretty likely that you want it as small as possible, and not needing tar or btrfs-progs for the actual install will save a lot of space (multiple MB doesn't sound like much, but when you're dealing with a tiny system to begin with, it can be very significant).
> 
> Step a3 need to have access to the raw disk image build at step1; this is quite incompatible with a "minimalistic install environment"; and even if you have access it via net, in the same way you can have access to a fully working environment....
The pretty standard case for a netboot install environment for Linux is 
to have a small netboot image (usually less than 50MB) that runs 
entirely from RAM, and then downloads the system image or packages to 
install.
> 
> 
> [...]
> 
>>>>>>>
>>>>>>> where <file-to-be-created> doesn't have to exists before mkfs.btrfs, and after
>>>>>>> a) <file-to-be-created> contains the image
>>>>>>> b) <file-to-be-created> is the smallest possible size.
>>>>>>>
>>>>>>> Definitely I don't like the truncate done by the operator by hand after the mkfs.btrfs (current behavior).
>>>>>> FWIW, the current release behavior doesn't require the truncate, and properly generates the file for the filesystem.
>>>>> If you don't do truncate, you have the fully partition... Or there is something that I miss ?
>>>> The current release, without these patches, run using a non-existent file and the `-r` option, will produce a filesystem image of the exact size needed to hold everything in the directory passed to `-r`.  It doesn't require truncation unless used on a file that already exists.
>>>
>>> Of course the truncate is not needed, because you are using a sparse file. But if you use a sparse file..... it is not even needed the shrinking! Because the file will consume the same space on the disk !
> 
>> Unless you want to use the file elsewhere.  It's a pretty rare occurrence outside of testing that you generate a filesystem image and use it as-is without transferring it somewhere (usually onto an actual storage device).  Once you're talking about moving it, whether or not the file itself is sparse usually doesn't matter, especially if the file is leaving the local system by some means other than NFS or rsync.
> 
> ????
> If you don't truncate you have the full-image in any case....
How about a more concrete example.  Say you're creating a filesystem 
image on your local system for a VM you'll run on some server.  Unless 
you use rsync, NFS, or some custom software, you will probably need 
enough free space on the server to hold the entirety of the filesystem 
image including all the free space that's sparse on the local system, 
because almost any other software doesn't properly do sparse writes, and 
you'll need to call `fallocate -d` on the file on the server to make it 
sparse again.  Even working locally, there are quite a few tools that 
won't copy the file sparsely.