[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-btrfs
Subject:    Re: [PATCH 1/2] btrfs-progs: mkfs: add uuid and otime to ROOT_ITEM of FS_TREE
From:       "Misono, Tomohiro" <misono.tomohiro () jp ! fujitsu ! com>
Date:       2017-10-31 2:55:47
Message-ID: 84edf00a-a305-7d44-15a8-7f09dff28055 () jp ! fujitsu ! com
[Download RAW message or body]

On 2017/10/30 17:32, Qu Wenruo wrote:
> 
> 
> On 2017年10月30日 16:14, 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)". Also, it lacks the otime information.
> 
> What about creating a wiki page for ROOT_ITEM and detailed restriction
> for its members?
> 
>>
>> 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 patch also inspires me about the btrfs_create_tree() function I'm
> introducing should also populate its UUID and timespec.
> 
>>
>> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
>> ---
>>  ctree.h       |  5 +++++
>>  mkfs/common.c | 14 ++++++++++++++
>>  mkfs/main.c   |  3 +++
>>  3 files changed, 22 insertions(+)
>>
>> diff --git a/ctree.h b/ctree.h
>> index 2280659..5737978 100644
>> --- a/ctree.h
>> +++ b/ctree.h
>> @@ -2071,6 +2071,11 @@ BTRFS_SETGET_STACK_FUNCS(root_stransid, struct btrfs_root_item,
>>  			 stransid, 64);
>>  BTRFS_SETGET_STACK_FUNCS(root_rtransid, struct btrfs_root_item,
>>  			 rtransid, 64);
>> +static inline void btrfs_set_root_uuid(struct btrfs_root_item *root_item,
>> +			 u8 uuid[BTRFS_UUID_SIZE])
>> +{
>> +	memcpy(root_item->uuid, uuid, BTRFS_UUID_SIZE);
>> +}
> 
> This is the stack version.
> 
> However there is no extent buffer version to set uuid as we just call
> write_extent_buffer(), so it seems unnecessary to me.
> 
>>  
>>  /* struct btrfs_root_backup */
>>  BTRFS_SETGET_STACK_FUNCS(backup_tree_root, struct btrfs_root_backup,
>> diff --git a/mkfs/common.c b/mkfs/common.c
>> index c9ce10d..808d343 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);
>> +			btrfs_set_root_uuid(&root_item, uuid);
>> +			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);
> 
> This leads to the question about the UUID meaning of different trees.
> 
> AFAIK every tree created by kernel has its own UUID, no one uses all zero.
> 
> In kernel btrfs_create_tree(), it always calls uuid_le_gen() to generate
> a new UUID.
> So I'm wonder if such branch is really needed.
> And maybe we fix all tree creation to generate UUID.
> 
> Despite the question about the meaning of UUID for root_item, I think
> the patch really makes sense.
> 
> Thanks,
> Qu
> 
Hello,

Thanks for the whole comments.
I notice UUID (and otime/ctime) of ROOT_ITEM is not used nor updated currently 
expect subvolumes. Therefore I think it is better to keep these fields to zero
to express non-used status. (So, it may be better that uuid/quota tree is not hold UUID.)

Regards,
Tomohiro

>> +			btrfs_set_root_uuid(&root_item, uuid);
>> +			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 1b4cabc..4184a2d 100644
>> --- a/mkfs/main.c
>> +++ b/mkfs/main.c
>> @@ -329,6 +329,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);
>> @@ -339,6 +340,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 */
>> +	btrfs_set_root_uuid(&root_item, uuid);
>>  	free_extent_buffer(tmp);
>>  
>>  	location.objectid = objectid;
>>
> 

--
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
[prev in list] [next in list] [prev in thread] [next in thread]