Re: [PATCH] btrfs: Use transid for DIR_ITEM/DIR_INDEX's location

[Josef Bacik <josef@toxicpanda.com>]

On 8/28/20 9:20 AM, Nikolay Borisov wrote:
> When a snapshot is created its root item is inserted in the root tree
> with the 'offset' field set to the transaction id when the snapshot
> was created. Immediately afterwards the offset is set to -1 so when
> the same key is used to create DIR_ITEM/DIR_INDEX in the destination
> file tree its offset is really set to -1. Root tree item:
> 
>      item 13 key (258 ROOT_ITEM 7) itemoff 12744 itemsize 439
>          generation 7 root_dirid 256 bytenr 30703616 level 0 refs 1
>          lastsnap 7 byte_limit 0 bytes_used 16384 flags 0x0(none)
>          uuid f13abf0d-b1f5-f34b-a179-fd1c2f89e762
>          parent_uuid 51a74677-a077-4c21-bd87-2141a147ff85
>          ctransid 7 otransid 7 stransid 0 rtransid 0
>          ctime 1598441149.466822752 (2020-08-26 11:25:49)
>          otime 1598441149.467474846 (2020-08-26 11:25:49)
>          drop key (0 UNKNOWN.0 0) level 0
> 
> DIR_INDEX item for the same rooti in the destination fs tree:
> 
> item 5 key (256 DIR_INDEX 9) itemoff 15967 itemsize 39
>          location key (258 ROOT_ITEM 18446744073709551615) type DIR
>          transid 7 data_len 0 name_len 9
>          name: snapshot1
> 
> The location key is generally used to read the root. This is not a
> problem per-se since the function dealing with root searching
> (btrfs_find_root) is well equipped to deal with offset being -1, namely:
> 
>      If ->offset of 'search_key' is -1ULL, it means we are not sure the
>      offset of the search key, just lookup the root with the highest
>      offset for a given objectid.
> 
> However this is a needless inconcistency in the way internal data
> structures are being created. This patch modifies the behavior so that
> DIR_INDEX/DIR_ITEM will have the offset field of the location key set
> to the transid. While this results in a change of the on-disk metadata,
> it doesn't constitute a functional change since older kernels can cope
> with both '-1' and transid as values of the offset field so no INCOMPAT
> flags are needed.
> 
> Finally while at it also move the initialization of the key in
> create_pending_snapshot closer to where it's being used for the first
> time.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef