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@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >
Attachment:
signature.asc
Description: OpenPGP digital signature
