thin_check doesn't use data space map to open data mapping tree

thin_check doesn't use data space map to open data mapping tree

[M.H. Tsai]

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

Hi All,

I found two doubtful points in thin-provisioning-tools,
which might need your help to provide some opinions:

1. thin_check doesn't open metadata and data space maps.
   It uses the core space_map to open the mapping_tree,
   instead of the data space map. Is that legal?
   (Although block_time_ref_counter was never used during tree traversal)

2. Class mapping_tree_detail::ll_damage_visitor should not be used to
traverse
   a single_mapping_tree. This class was designed for two-level data
mapping tree,
   or the top-level data mapping tree (i.e., mapping_tree and dev_tree
types).
   It generates missing_devices while visiting a damage with
zero-length-path,
   which is not semantically suitable for single_mapping_tree.


Thanks,
Ming-Hung Tsai

[-- Attachment #1.2: Type: text/html, Size: 1055 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: thin_check doesn't use data space map to open data mapping tree

[Joe Thornber]

On Mon, Apr 06, 2015 at 10:18:45PM +0800, M.H. Tsai wrote:
> Hi All,
> 
> I found two doubtful points in thin-provisioning-tools,
> which might need your help to provide some opinions:

Firstly well done for digging through these tools.  I love it when
people take the time to read code.

> 1. thin_check doesn't open metadata and data space maps.
>    It uses the core space_map to open the mapping_tree,
>    instead of the data space map. Is that legal?
>    (Although block_time_ref_counter was never used during tree traversal)

Is it legal?  Yes, walking the mappings doesn't require any
use of the space map.

But it does highlight the fact that I'm not checking the space maps
correctness currently.  The first release of the tools _did_ do this,
but it was ommitted when I did a big tidy up.  There is a branch with
some work for this area that I need to merge in.

> 2. Class mapping_tree_detail::ll_damage_visitor should not be used to
> traverse
>    a single_mapping_tree. This class was designed for two-level data
> mapping tree,
>    or the top-level data mapping tree (i.e., mapping_tree and dev_tree
> types).
>    It generates missing_devices while visiting a damage with
> zero-length-path,
>    which is not semantically suitable for single_mapping_tree.

Yep, I think you're right.  Thanks.

- Joe