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