Bug#396023: LVM removal will not work properly with the applied patch

[David Härdeman <david@hardeman.nu>]

On Wed, Oct 24, 2007 at 09:26:41PM +0200, Jérémy Bobbio wrote:
> On Mon, Jul 02, 2007 at 02:30:55PM +0200, Fabio Massimo Di Nitto wrote:
> > I am seeing different issues such as:
> >
> > 1) take a fresh disk with no lvm
> > 2) install using automatic lvm setup and it will work
> > 3) install on it again this time not using lvm and it will work
> > 4) install again this time using lvm and it will fail
> >
> > The problem is that at #4 the disk has no partitions type LVM but the lvm
> > metadata from #2 are still there.
> >
> > pvscan still works but all other lvm operations will correctly report that there
> > is an error.
> >
> > Basically #3 did not wipe the disk properly.
> > [...]
> > I have different ideas on how to approach the problem but in one way or another
> > they might hit other kinds of installations (Colin Watson mentioned low-mem as
> > one of them).
> >
> > My suggestion is:
> > - Make partman-auto depends on the proper bits that it is actually using.
> > - Add code to modprobe/verify that dm-mod is available.
> > - Fix dm_wipe_lvm to not require dmsetup to detect the volumes and use lvm2
> >   tools instead (I am not happy to execute vgchanges if we can avoid it).
>
> The attached patch partially implements these suggestions and removes
> bits of duplicated code in partman-partitioning and partman-auto.
> Here's the details:

Thanks for working on this and saving me lots of effort :)

> The creation of a new label for a disk (thus wiping any partitions) has
> been factored out in create_new_label() in partman-base.
>    
> This function is then used by :
> * partman-partinioning/storage_device/label/do_option ;
> * partman-auto/auto-shared:wipe_disk().
>
> create_new_label() uses dm_wipe_disk() when applicable and falls back to
> lvm_wipe_disk() if dm_mod is not loaded.  The later does not require
> dmsetup or the activation of the devices.

I don't get this part...why check if dm-mod is loaded? Using dmsetup 
requires dm-mod to be loaded, so shouldn't create_new_label just check 
if dm_wipe_disk is available and if so, use it. Then dm_wipe_disk (which 
is now part of partman-crypto which in turn depends on dmsetup) can make 
sure dm-mod is loaded?

> Both dm_wipe_disk() and lvm_wipe_disk() might require partman to be
> restarted in order to get its state properly refreshed.  This has been
> factored out in partman-base/definitions.sh:updated_parted().

Isn't "update_parted" a bit too general? The function *only* removes 
stale device-mapper entries from partman's cache...

> In order to keep some modularity, dm_wipe_disk() has been moved from
> partman-auto to partman-crypto:crypto_tools.sh.  In a similar fashion,
> lvm_wipe_disk() has been added to partman-lvm:lvm_tools.sh.
>
> The value added by the refactoring is less than it could possibly be,
> though: once LVM has been configured, the devices are "locked" for
> partman and the creation of a new label will prevented.  This check
> could probably be relaxed with these new changes.
>
> I have tested various cases and this patch will make d-i work correctly
> with Fabio's scenario.  I would like some reviews though as I am far
> from being a partman guru.

I've done a quick read of the patch, and I have no further objections

--
David Härdeman