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

[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,
> >
> > Please review the patch below when you have a moment.  I am interested in
> > your feedback, and also interested in having this functionality merged
> > upstream.  This was written against thin-provisioning-tools.git tag
> > v0.5.6.
> >
> > 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?

I'm open to whatever---just making sure whatever we do stays clean and 
scales for the future.

--
Eric Wheeler