Re: [PATCH] thin_dump: added --device-id, --skip-mappings, and new output --format's

[Eric Wheeler <dm-devel@lists.ewheeler.net>]

On Wed, 16 Mar 2016, Ming-Hung Tsai wrote:

> 2016-03-16 1:51 GMT+08:00 Eric Wheeler <dm-devel@lists.ewheeler.net>:
> > On Tue, 15 Mar 2016, Thanos Makatos wrote:
> >
> >> On 15 March 2016 at 01:45, Eric Wheeler <dm-devel@lists.ewheeler.net> wrote:
> >> > Hi Joe,
> >> >
> >> > We use thin_dump on live dm-thin metadata snapshots all the time.  In our
> >> > case, we want to dump the XML for new (snapshot) volumes instead of
> >> > dumping the entire 16gb metadata device (37.8% used) which takes ~20-30
> >> > minutes instead of ~5 seconds for a single volume with --device-id.
> >>
> >> I'm interested in extracting the mappings of a particular device, too.
> >> In fact, I've implemented this (along with extracting the mappings in
> >> binary format) and have only recently opened a PR:
> >> https://github.com/jthornber/thin-provisioning-tools/pull/49. It seems
> >> that we've replicated some work here, I'm not sure whether we're
> >> supposed to send patches to this list or open PR on github.
> >
> > Interesting, it is neat to see a binary format too.
> >
> > I think we need to come up with a consistent way to extend attributes
> > being passed down into mapping_tree_emitter::visit() by way of
> > thin_provisioning::metadata_dump().  What is preferred?
> >
> > I noticed that caching::metadata_dump() has the same prototype, so do
> > those need to remain compatable calls?
> >
> > Should we countinue to extend the arguments to metadata_dump()
> > as Thanos and I have done, or, should we pop up the mte call to the caller
> > and pass a pointer to thin_provisioning::metadata_dump(..., *mte) and let
> > the caller configure the mte with setter functions or constructor calls?
> 
> Hi,
> 
> You are not alone, I did the same feature and used it for a long time.
> My solution is similar to Thanos, I override thin_provisioning::metadata_dump()
> to accept the additional dev_id:
> 
> void metadata_dump(metadata::ptr md, emitter::ptr e);
> void metadata_dump(metadata::ptr md, emitter::ptr e, uint64_t dev_id);

That makes for 3 different implementations, we've all modified 
metadata_dump:


original:  ::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair);

Ming-Hung: void metadata_dump(metadata::ptr md, emitter::ptr e, [bool repair?]
+		uint64_t dev_id);

Thanos:    ::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair, 
+		const block_address *dev_id)

Me:        ::metadata_dump(metadata::ptr md, emitter::ptr e, bool repair, 
+		bool skip_mappings, 
+		block_address dev_id)


--
Eric Wheeler



> 
> Instead of passing dev_id to mapping_tree_emitter, I pass the dd_map
> with only one device,
> then let the mapping_tree_emitter to traverse the bottom-level tree directly:
> 
> void
> thin_provisioning::metadata_dump(metadata::ptr md, emitter::ptr e,
> uint64_t dev_id)
> {
>     uint64_t key[1] = {dev_id};
>     dev_tree::maybe_value single_mapping_tree_root =
> md->mappings_top_level_->lookup(key);
>     device_tree::maybe_value details = md->details_->lookup(key);
> 
>     // ... error handling
> 
>     details_extractor de;
>     de.visit(dev_id, *details);  // de contains only one device
> 
>     e->begin_superblock(...);
>     {
>         mapping_tree_emitter mte(md, e, de.get_details(),
> mapping_damage_policy(repair));
>         std::vector<uint64_t> path(1, dev_id); //
> btree_detail::btree_path is std::vector<uint64_t>
>         mte.visit(path, *single_mapping_tree_root);
>     }
>     e->end_superblock();
> }
> 
> But this looks still a bit messy: I should setup the visitors and
> trees carefully to do what I want.
> 
> I also wrote some debugging feature for thin_dump. I can push the code
> if you have interest.
> 
> 1. thin_dump -b <metadata_dev> : Dump the device_details tree only. Do
> not emit the data mappings.
> (This could be replace by the new thin_ls)
> 
> output (no mappings):
> <superblock ... >
>   <device ... >
>   </device>
>   <device ... >
>   </device>
> </superblock>
> 
> 2. thin_dump -r2 <metadata_dev>: dump the data mapping tree despite
> the device_details tree was broken
> 
> 
> 
> Ming-Hung Tsai
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel
>