Re: About 'key type for persistent [...] items'

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3807 bytes --]



On 2017年11月25日 09:12, Qu Wenruo wrote:
> 
> 
> On 2017年11月25日 03:16, Hans van Kranenburg wrote:
>> Hi,
>>
>> Last week, when implementing the automatic classifier to dynamically
>> create tree item data objects by key type in python-btrfs, I ran into
>> the following commits in btrfs-progs:
>>
>>   commit 8609c8bad68528f668d9ce564b868aa4828107a0
>>   btrfs-progs: print-tree: factor out temporary_item dump
>> and
>>   commit a4b65f00d53deb1b495728dd58253af44fcf70df
>>   btrfs-progs: print-tree: factor out persistent_item dump
>>
>> ...which are related to kernel...
>>
>>   commit 50c2d5abe64c1726b48d292a2ab04f60e8238933
>>   btrfs: introduce key type for persistent permanent items
>> and
>>   commit 0bbbccb17fea86818e1a058faf5903aefd20b31a
>>   btrfs: introduce key type for persistent temporary items
>>
>> Afaics the goal is to overload types because there can be only 256 in
>> total.
> 
> Personally speaking, to overload types, we can easily make different
> meanings of type based on tree owner.
> 
>> However, I'm missing the design decisions behind the
>> implementation of it. It's not in the commit messages, and it hasn't
>> been on the mailing list.
> 
> Btrfs_tree.h has a good enough description for it.
> 
>>
>> Before, there was an 1:1 mapping from key types to data structures. Now,
>> with the new PERSISTENT_ITEM_KEY and TEMPORARY_ITEM_KEY, it seems items
>> which use this type can be using any data structure they want, so it's
>> some kind of YOLO_ITEM_KEY.
> 
> For PERSISTENT and TEMPORARY item, they use the objectid and type to
> determine the real data structure type.
> 
> Since in case of existing PERSISTENT/TEMPORARY item usage, their
> objectid is not really used for storing data.

PS: PERSISTENT/TEMPORARY keys are not the only keys where only
key->offset is really used.

CHUNK_ITME also has a fixed objecitd, BTRFS_FIRST_CHUNK_TREE_OBJECTID.

And QGROUP_STATUS is even a step further, with 0 as both objectid and
offset.

So from this point, such PERSISTENT/TEMPORARY design doesn't really
follow the existing type design.
It may only help for later expansion, but I doubt if we will use such
schema.

Thanks,
Qu

> 
>>
>> The print-tree code in progs 8609c8b and a4b65f0 seems incomplete. For
>> example, for the PERSISTENT_ITEM_KEY, there's a switch (objectid) with
>> case BTRFS_DEV_STATS_OBJECTID.
>>
>> However, BTRFS_DEV_STATS_OBJECTID is just the value 0. So, that means
>> that if I want to have another tree where BTRFS_MOUTON_OBJECTID is also
>> 0, and I'm storing a btrfs_kebab_item struct indexed at
>> (BTRFS_MOUTON_OBJECTID, PERSISTENT_ITEM_KEY, 31337), then print_tree.c
>> will try to parse the data by calling print_dev_stats?
> 
> In this case, you shouldn't have your "BTRFS_MOUNTON_OBJECTID" assigned
> to 0.
> 
>>
>> What's the idea behind that? Instead of having the key type field define
>> the struct and meaning, we now suddenly need the tuple (tree, objectid,
>> type),
> 
> Not exactly, it's (objectid, type) only to determine the data structure
> type for PERSISTENT/TEMPORARY key type.
> Btrfs doesn't (yet) use root to determine the meaning.
> 
> So current btrfs-progs works fine.
> 
> Thanks,
> Qu
> 
>> and we need all three to determine what's inside the item data?
>> So, the code in print_tree.c would also need to know about the tree
>> number and pass that into the different functions.
>>
>> Am I missing something, or is my observation correct?
>>
>> Thanks,--
>> Hans van Kranenburg
>> --
>> 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
>>
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]