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 
> 
> 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

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

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 

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

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

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

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

[Hugo Mills]

On Mon, Mar 19, 2018 at 02:02:23PM +0100, David Sterba wrote:
> On Mon, Mar 19, 2018 at 08:20:10AM +, 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


signature.asc
Description: Digital signature

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 +, 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Hugo Mills]

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 
> ---
> 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=152144463907830=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(_item.otime, now);
> + btrfs_set_stack_timespec_sec(_item.ctime, now);
> + } else {
> + memset(uuid, 0, BTRFS_UUID_SIZE);
> + memcpy(root_item.uuid, uuid, BTRFS_UUID_SIZE);
> + btrfs_set_stack_timespec_sec(_item.otime, 0);
> + btrfs_set_stack_timespec_sec(_item.ctime, 0);
> + }
>   write_extent_buffer(buf, _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, , objectid);
> @@ -325,6 +326,8 @@ static int create_tree(struct btrfs_trans_handle *trans,
>   btrfs_set_root_bytenr(_item, tmp->start);
>   btrfs_set_root_level(_item, btrfs_header_level(tmp));
>   btrfs_set_root_generation(_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  |


signature.asc
Description: Digital signature