On 2017年10月31日 10:55, Misono, Tomohiro wrote:
> 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@xxxxxxxxxxxxxx>
>>> ---
>>> 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.)
Makes sense.
I'll update kernel code to avoid UUID creation for non-fs trees.
Since they are not in UUID tree so there is no meaning creating uuid for
them.
Thanks,
Qu
>
> 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;
>>>
>>
>
Attachment:
signature.asc
Description: OpenPGP digital signature
