Re: [PATCH RFC v2] UBI: New ioctl() to support ubidump

[Bill Pringlemeir <bpringlemeir@nbsps.com>]

On  1 Aug 2014, dedekind1@gmail.com wrote:

> On Fri, 2014-08-01 at 10:50 +0800, hujianyang wrote:

>> 1) No UBI stuff and read UBIFS stuff from UBI-driver This is a
>> easiest way and we don't need any ioctl either. We can't get
>> UBI-level stuff right now but we can put this into our design of
>> dumping an image file.

> Yes. That's a good first step.

I agree that this is a good way to partition the functionality.

>> 2) Read data from MTD-driver and find a way to get pnum This way is
>> basing on my v2 work. We can put an ioctl() to kernel and if kernel
>> don't support this ioctl(), this tool will do a full-scan on MTD
>> device. It's transparent to user. But we should take some time to
>> consider how to do this full-scan.

> I guess, I'd need to think some more about the ioctl(), and may be
> someone else suggests something.

Isn't this just more work?  The tool will rely on two things?

>> 3) Re-design this tool about dropping UBI driver based dumping.
>> This way is like the exist ubiformat utility. We don't use UBI driver
>> and just do read/write(maybe we can do some repairing later) with MTD
>> device or image files. It's not small.

> Probably, we'll think about this when we are done with 1) and may be
> 2).

Some of these points I don't understand.  The 'ubiformat' already uses
'libscan.c', which have the basics for creating a static 'PEB->LEB'
mapping.  So while I agree that maintaining two version of UBI is not
good, I think we already have this?

UBI has to deal with writing, especially atomic writes, and concurrency.
I don't think the tool needs either, so the 2nd version of UBI is
significantly simpler.  Just look at 'libscan.c' versus
'driver/mtd/ubi/*.c'.

Also, I think a lot of people will have issues where they can not mount
the device and the only thing they can get is the whole 'nanddump';
which won't be done through Linux, but some other means.  Ie, some ROM
boot code (which doesn't understand either UBI or UbiFS) to read out
NAND, a NAND programmer, etc.

Finally, the separate implementation can be good.  Having two versions
of code use the same data structures will often show issues.  If we want
to debug UBI, then using it is not so helpful.  Having the separate
(simpler) implementation with worse performance is exactly the kind of
thing you want for debugging and repair, isn't it?  Features like
'fastmap' are optional and we should fall back to a linear scan.  I
think that future UBI changes will strive to be backwards compatible
unless there is some really good reason not to be?

The big advantage of doing everything outside the kernel is we can
eventually add lots of extra consistency checks that you would never
want to do in a running file system.  Ie, verify a linear scan and the
fastmap are consistent, etc.

My intent is never to make work for anyone.  Adding a patch to libscan.c
seems easy to me.

 1. Allocate a 'pmap' array and a temporary 'sequence' array.
 2. Initialize 'pmap' to -1.
 3a. For each valid 'eb', get 'lnum' and 'sqnum' from vid header.
  b. if (pmap[lnum] == -1 || sequence[lnum] < sqnum)
      pmap[lnum] = eb, sequence[lnum] = sqnum;

but maybe I don't understand all the complexities.  Isn't it that
simple?

Now if we want to use the tool on live volumes, 'libscan.c' will not
work for that.  I thought that the concepts of a debugger may apply.  It
can look at a core file, or a paused processes memory.  But maybe it is
crazy to 'pause' Ubi/UbiFs.  Someone might want such 'snapshots' to
examine the evolution of storage or some race issues... but I guess that
is not the main reason of using the ioctl().

Fwiw,
Bill Pringlemeir.