[PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[Misono, Tomohiro]

Currently, the top-level subvolume lacks the UUID. As a result, both
non-snapshot subvolume and snapshot of top-level subvolume do not have
Parent UUID and cannot be distinguisued. Therefore "fi show" of
top-level lists all the subvolumes which lacks the UUID in
"Snapshot(s)" filed.  Also, it lacks the otime information.

Fix this by adding the UUID and otime at the mkfs time.  As a
consequence, snapshots of top-level subvolume now have a Parent UUID and
UUID tree will create an entry for top-level subvolume at mount time.
This should not cause the problem for current kernel, but user program
which relies on the empty Parent UUID may be affected by this change.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
This is also needed in order that "sub list -s" works properly for
non-privileged user[1] even if there are snapshots of toplevel subvolume.

Currently the check if a subvolume is a snapshot is done by looking at the key
offset of ROOT_ITEM of subvolume (non-zero for snapshot) used by tree search ioctl.
However, non-privileged version of "sub list" won't use tree search ioctl and just
looking if parent uuid is null or not. Therefore there is no way to recognize
snapshots of toplevel subvolume.

[1] https://marc.info/?l=linux-btrfs&m=152144463907830&w=2

 mkfs/common.c | 14 ++++++++++++++
 mkfs/main.c   |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/mkfs/common.c b/mkfs/common.c
index 16916ca2..6924d9b7 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -44,6 +44,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 	u32 itemoff;
 	int ret = 0;
 	int blk;
+	u8 uuid[BTRFS_UUID_SIZE];
 
 	memset(buf->data + sizeof(struct btrfs_header), 0,
 		cfg->nodesize - sizeof(struct btrfs_header));
@@ -77,6 +78,19 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 		btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
 		btrfs_set_item_size(buf, btrfs_item_nr(nritems),
 				sizeof(root_item));
+		if (blk == MKFS_FS_TREE) {
+			time_t now = time(NULL);
+
+			uuid_generate(uuid);
+			memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
+			btrfs_set_stack_timespec_sec(&root_item.otime, now);
+			btrfs_set_stack_timespec_sec(&root_item.ctime, now);
+		} else {
+			memset(uuid, 0, BTRFS_UUID_SIZE);
+			memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
+			btrfs_set_stack_timespec_sec(&root_item.otime, 0);
+			btrfs_set_stack_timespec_sec(&root_item.ctime, 0);
+		}
 		write_extent_buffer(buf, &root_item,
 			btrfs_item_ptr_offset(buf, nritems),
 			sizeof(root_item));
diff --git a/mkfs/main.c b/mkfs/main.c
index 5a717f70..52d92581 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -315,6 +315,7 @@ static int create_tree(struct btrfs_trans_handle *trans,
 	struct btrfs_key location;
 	struct btrfs_root_item root_item;
 	struct extent_buffer *tmp;
+	u8 uuid[BTRFS_UUID_SIZE] = {0};
 	int ret;
 
 	ret = btrfs_copy_root(trans, root, root->node, &tmp, objectid);
@@ -325,6 +326,8 @@ static int create_tree(struct btrfs_trans_handle *trans,
 	btrfs_set_root_bytenr(&root_item, tmp->start);
 	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
 	btrfs_set_root_generation(&root_item, trans->transid);
+	/* clear uuid of source tree */
+	memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
 	free_extent_buffer(tmp);
 
 	location.objectid = objectid;
-- 
2.14.3

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 3944 bytes --]

On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote:
> Currently, the top-level subvolume lacks the UUID. As a result, both
> non-snapshot subvolume and snapshot of top-level subvolume do not have
> Parent UUID and cannot be distinguisued. Therefore "fi show" of
> top-level lists all the subvolumes which lacks the UUID in
> "Snapshot(s)" filed.  Also, it lacks the otime information.
> 
> Fix this by adding the UUID and otime at the mkfs time.  As a
> consequence, snapshots of top-level subvolume now have a Parent UUID and
> UUID tree will create an entry for top-level subvolume at mount time.
> This should not cause the problem for current kernel, but user program
> which relies on the empty Parent UUID may be affected by this change.

   Is there any way of adding a UUID to the top level subvol on an
existing filesystem? It would be helpful not to have to rebuild every
filesystem in the world to fix this.

   Hugo.

> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> ---
> This is also needed in order that "sub list -s" works properly for
> non-privileged user[1] even if there are snapshots of toplevel subvolume.
> 
> Currently the check if a subvolume is a snapshot is done by looking at the key
> offset of ROOT_ITEM of subvolume (non-zero for snapshot) used by tree search ioctl.
> However, non-privileged version of "sub list" won't use tree search ioctl and just
> looking if parent uuid is null or not. Therefore there is no way to recognize
> snapshots of toplevel subvolume.
> 
> [1] https://marc.info/?l=linux-btrfs&m=152144463907830&w=2
> 
>  mkfs/common.c | 14 ++++++++++++++
>  mkfs/main.c   |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/mkfs/common.c b/mkfs/common.c
> index 16916ca2..6924d9b7 100644
> --- a/mkfs/common.c
> +++ b/mkfs/common.c
> @@ -44,6 +44,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>  	u32 itemoff;
>  	int ret = 0;
>  	int blk;
> +	u8 uuid[BTRFS_UUID_SIZE];
>  
>  	memset(buf->data + sizeof(struct btrfs_header), 0,
>  		cfg->nodesize - sizeof(struct btrfs_header));
> @@ -77,6 +78,19 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
>  		btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
>  		btrfs_set_item_size(buf, btrfs_item_nr(nritems),
>  				sizeof(root_item));
> +		if (blk == MKFS_FS_TREE) {
> +			time_t now = time(NULL);
> +
> +			uuid_generate(uuid);
> +			memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
> +			btrfs_set_stack_timespec_sec(&root_item.otime, now);
> +			btrfs_set_stack_timespec_sec(&root_item.ctime, now);
> +		} else {
> +			memset(uuid, 0, BTRFS_UUID_SIZE);
> +			memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
> +			btrfs_set_stack_timespec_sec(&root_item.otime, 0);
> +			btrfs_set_stack_timespec_sec(&root_item.ctime, 0);
> +		}
>  		write_extent_buffer(buf, &root_item,
>  			btrfs_item_ptr_offset(buf, nritems),
>  			sizeof(root_item));
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 5a717f70..52d92581 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -315,6 +315,7 @@ static int create_tree(struct btrfs_trans_handle *trans,
>  	struct btrfs_key location;
>  	struct btrfs_root_item root_item;
>  	struct extent_buffer *tmp;
> +	u8 uuid[BTRFS_UUID_SIZE] = {0};
>  	int ret;
>  
>  	ret = btrfs_copy_root(trans, root, root->node, &tmp, objectid);
> @@ -325,6 +326,8 @@ static int create_tree(struct btrfs_trans_handle *trans,
>  	btrfs_set_root_bytenr(&root_item, tmp->start);
>  	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
>  	btrfs_set_root_generation(&root_item, trans->transid);
> +	/* clear uuid of source tree */
> +	memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
>  	free_extent_buffer(tmp);
>  
>  	location.objectid = objectid;

-- 
Hugo Mills             | This chap Anon is writing some perfectly lovely
hugo@... carfax.org.uk | stuff at the moment.
http://carfax.org.uk/  |
PGP: E2AB1DE4          |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[David Sterba]

On Mon, Mar 19, 2018 at 08:20:10AM +0000, Hugo Mills wrote:
> On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote:
> > Currently, the top-level subvolume lacks the UUID. As a result, both
> > non-snapshot subvolume and snapshot of top-level subvolume do not have
> > Parent UUID and cannot be distinguisued. Therefore "fi show" of
> > top-level lists all the subvolumes which lacks the UUID in
> > "Snapshot(s)" filed.  Also, it lacks the otime information.
> > 
> > Fix this by adding the UUID and otime at the mkfs time.  As a
> > consequence, snapshots of top-level subvolume now have a Parent UUID and
> > UUID tree will create an entry for top-level subvolume at mount time.
> > This should not cause the problem for current kernel, but user program
> > which relies on the empty Parent UUID may be affected by this change.
> 
>    Is there any way of adding a UUID to the top level subvol on an
> existing filesystem? It would be helpful not to have to rebuild every
> filesystem in the world to fix this.

We can do that by a special purpose tool. The easiest way is to set the
uuid on an unmouted filesystem, but as this is a one-time action I hope
this is acceptable. Added to todo, thanks for the suggestion.

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

On Mon, Mar 19, 2018 at 02:02:23PM +0100, David Sterba wrote:
> On Mon, Mar 19, 2018 at 08:20:10AM +0000, Hugo Mills wrote:
> > On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote:
> > > Currently, the top-level subvolume lacks the UUID. As a result, both
> > > non-snapshot subvolume and snapshot of top-level subvolume do not have
> > > Parent UUID and cannot be distinguisued. Therefore "fi show" of
> > > top-level lists all the subvolumes which lacks the UUID in
> > > "Snapshot(s)" filed.  Also, it lacks the otime information.
> > > 
> > > Fix this by adding the UUID and otime at the mkfs time.  As a
> > > consequence, snapshots of top-level subvolume now have a Parent UUID and
> > > UUID tree will create an entry for top-level subvolume at mount time.
> > > This should not cause the problem for current kernel, but user program
> > > which relies on the empty Parent UUID may be affected by this change.
> > 
> >    Is there any way of adding a UUID to the top level subvol on an
> > existing filesystem? It would be helpful not to have to rebuild every
> > filesystem in the world to fix this.
> 
> We can do that by a special purpose tool. The easiest way is to set the
> uuid on an unmouted filesystem, but as this is a one-time action I hope
> this is acceptable. Added to todo, thanks for the suggestion.

   Sounds good to me.

   Hugo.

-- 
Hugo Mills             | Talking about music is like dancing about
hugo@... carfax.org.uk | architecture
http://carfax.org.uk/  |
PGP: E2AB1DE4          |                                           Frank Zappa

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[Christoph Anton Mitterer]

On Mon, 2018-03-19 at 14:02 +0100, David Sterba wrote:
> We can do that by a special purpose tool.

No average user will ever run (even know) about that...

Could you perhaps either do it automatically in fsck (which is IMO als
a bad idea as fsck should be read-only per default)... or at least add
a warning to fsck, like a "Info: Please run tool foo to get bar done."?

Cheers,
Chris.

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[David Sterba]

On Mon, Mar 19, 2018 at 02:26:38PM +0100, Christoph Anton Mitterer wrote:
> On Mon, 2018-03-19 at 14:02 +0100, David Sterba wrote:
> > We can do that by a special purpose tool.
> 
> No average user will ever run (even know) about that...
> 
> Could you perhaps either do it automatically in fsck (which is IMO als
> a bad idea as fsck should be read-only per default)... or at least add
> a warning to fsck, like a "Info: Please run tool foo to get bar done."?

Makes sense, check can warn and point to the command, in a similar way
we have the fix-device-count.

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[David Sterba]

On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote:
> Currently, the top-level subvolume lacks the UUID. As a result, both
> non-snapshot subvolume and snapshot of top-level subvolume do not have
> Parent UUID and cannot be distinguisued. Therefore "fi show" of
> top-level lists all the subvolumes which lacks the UUID in
> "Snapshot(s)" filed.  Also, it lacks the otime information.
> 
> Fix this by adding the UUID and otime at the mkfs time.  As a
> consequence, snapshots of top-level subvolume now have a Parent UUID and
> UUID tree will create an entry for top-level subvolume at mount time.
> This should not cause the problem for current kernel, but user program
> which relies on the empty Parent UUID may be affected by this change.
> 
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>

So this adds uuid, ctime and otime to FS_TEEE but also to UUID_TREE and
DATA_RELOC_TREE. This is harmelss, but would be nice to mention in the
changelog, I'll apply the patch add that. Thanks.

Re: [PATCH] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE

[Misono Tomohiro]

On 2018/03/22 2:48, David Sterba wrote:
> On Mon, Mar 19, 2018 at 05:16:42PM +0900, Misono, Tomohiro wrote:
>> Currently, the top-level subvolume lacks the UUID. As a result, both
>> non-snapshot subvolume and snapshot of top-level subvolume do not have
>> Parent UUID and cannot be distinguisued. Therefore "fi show" of
>> top-level lists all the subvolumes which lacks the UUID in
>> "Snapshot(s)" filed.  Also, it lacks the otime information.
>>
>> Fix this by adding the UUID and otime at the mkfs time.  As a
>> consequence, snapshots of top-level subvolume now have a Parent UUID and
>> UUID tree will create an entry for top-level subvolume at mount time.
>> This should not cause the problem for current kernel, but user program
>> which relies on the empty Parent UUID may be affected by this change.
>>
>> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> 
> So this adds uuid, ctime and otime to FS_TEEE but also to UUID_TREE and
> DATA_RELOC_TREE. This is harmelss, but would be nice to mention in the
> changelog, I'll apply the patch add that. Thanks.

UUID is cleared at create_tree(), so I think you mean otime and ctime.
However, other tree's ROOT_ITEM does not hold o/ctime and I'd like to
clear o/ctime for UUID/DATA_RELOC_TREE too.

So, I will send v2 patch and could you please use that instead of this?

Regards,
Tomohiro Misono

[PATCH v2] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of, FS_TREE

[Misono Tomohiro]

Currently, the top-level subvolume lacks the UUID. As a result, both
non-snapshot subvolume and snapshot of top-level subvolume do not have
Parent UUID and cannot be distinguisued. Therefore "fi show" of
top-level lists all the subvolumes which lacks the UUID in
"Snapshot(s)" filed.  Also, it lacks the otime information.

Fix this by adding the UUID and otime at the mkfs time.  As a
consequence, snapshots of top-level subvolume now have a Parent UUID and
UUID tree will create an entry for top-level subvolume at mount time.
This should not cause the problem for current kernel, but user program
which relies on the empty Parent UUID may be affected by this change.

Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
---
v1 -> v2: clear o/ctime in create_tree()

 mkfs/common.c | 14 ++++++++++++++
 mkfs/main.c   |  5 +++++
 2 files changed, 19 insertions(+)

diff --git a/mkfs/common.c b/mkfs/common.c
index 16916ca2..6924d9b7 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -44,6 +44,7 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 	u32 itemoff;
 	int ret = 0;
 	int blk;
+	u8 uuid[BTRFS_UUID_SIZE];
 
 	memset(buf->data + sizeof(struct btrfs_header), 0,
 		cfg->nodesize - sizeof(struct btrfs_header));
@@ -77,6 +78,19 @@ static int btrfs_create_tree_root(int fd, struct btrfs_mkfs_config *cfg,
 		btrfs_set_item_offset(buf, btrfs_item_nr(nritems), itemoff);
 		btrfs_set_item_size(buf, btrfs_item_nr(nritems),
 				sizeof(root_item));
+		if (blk == MKFS_FS_TREE) {
+			time_t now = time(NULL);
+
+			uuid_generate(uuid);
+			memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
+			btrfs_set_stack_timespec_sec(&root_item.otime, now);
+			btrfs_set_stack_timespec_sec(&root_item.ctime, now);
+		} else {
+			memset(uuid, 0, BTRFS_UUID_SIZE);
+			memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
+			btrfs_set_stack_timespec_sec(&root_item.otime, 0);
+			btrfs_set_stack_timespec_sec(&root_item.ctime, 0);
+		}
 		write_extent_buffer(buf, &root_item,
 			btrfs_item_ptr_offset(buf, nritems),
 			sizeof(root_item));
diff --git a/mkfs/main.c b/mkfs/main.c
index 5a717f70..40efa26b 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -315,6 +315,7 @@ static int create_tree(struct btrfs_trans_handle *trans,
 	struct btrfs_key location;
 	struct btrfs_root_item root_item;
 	struct extent_buffer *tmp;
+	u8 uuid[BTRFS_UUID_SIZE] = {0};
 	int ret;
 
 	ret = btrfs_copy_root(trans, root, root->node, &tmp, objectid);
@@ -325,6 +326,10 @@ static int create_tree(struct btrfs_trans_handle *trans,
 	btrfs_set_root_bytenr(&root_item, tmp->start);
 	btrfs_set_root_level(&root_item, btrfs_header_level(tmp));
 	btrfs_set_root_generation(&root_item, trans->transid);
+	/* clear uuid and o/ctime of source tree */
+	memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
+	btrfs_set_stack_timespec_sec(&root_item.otime, 0);
+	btrfs_set_stack_timespec_sec(&root_item.ctime, 0);
 	free_extent_buffer(tmp);
 
 	location.objectid = objectid;
-- 
2.14.3

Re: [PATCH v2] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of, FS_TREE

[David Sterba]

On Fri, Mar 23, 2018 at 05:16:49PM +0900, Misono Tomohiro wrote:
> Currently, the top-level subvolume lacks the UUID. As a result, both
> non-snapshot subvolume and snapshot of top-level subvolume do not have
> Parent UUID and cannot be distinguisued. Therefore "fi show" of
> top-level lists all the subvolumes which lacks the UUID in
> "Snapshot(s)" filed.  Also, it lacks the otime information.
> 
> Fix this by adding the UUID and otime at the mkfs time.  As a
> consequence, snapshots of top-level subvolume now have a Parent UUID and
> UUID tree will create an entry for top-level subvolume at mount time.
> This should not cause the problem for current kernel, but user program
> which relies on the empty Parent UUID may be affected by this change.
> 
> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
> ---
> v1 -> v2: clear o/ctime in create_tree()

Yeah that's better, thanks. Patch replaced.