On Wed, 2009-03-11 at 17:51 +0100, Joel Granados Moreno wrote:
> * storage/devicelibs/lvm.py (pvstatus_ok): New function that tests the
> status of a PV. If PV part of a complete VG or not part of a VG at
> all, we consider it to be OK. otherwise it needs to be cleaned.
This feels like the way we used to do it. Comments inline below.
> * storage/devicelibs/lvm.py (vgreduce): Add the rm=True arg to the
> vgreduce call. This will allow us to ignore the device list and tell
> LVM to clean all the additional LVM metadata.
Seems good.
> * storage/devicelibs/lvm.py (zeroLvmMetadata): New function to try to
> clobber LVM metadata completely. This should be used as a last resort
> and zeros out the device from 512bytes to 10Mb.
See comments inline.
> * storage/devicetree (questionClobberInconsistengVG): New function. Its
> the callback used to ask the user if the LVM metadata should be erased
> from a dirty PV.
Good idea.
> * storage/devicetree (addUdevDevice): Use pvstatus_ok to check for the
> status of each PV we find.
This will be problematic IMO. See comments inline.
> ---
> storage/devicelibs/lvm.py | 87 +++++++++++++++++++++++++++++++++++++++++++-
> storage/devicetree.py | 71 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 155 insertions(+), 3 deletions(-)
>
> diff --git a/storage/devicelibs/lvm.py b/storage/devicelibs/lvm.py
> index 0faee10..d4926a6 100644
> --- a/storage/devicelibs/lvm.py
> +++ b/storage/devicelibs/lvm.py
> @@ -49,6 +49,23 @@ def has_lvm():
>
> return has_lvm
>
> +def zeroLvmMetadata(device):
> + """ This dds from byte 513 to 100Mb of the disk.
> +
> + We assume that lvm has nothing to do with the first 512 bytes.
> + We assume that 20Mb is enough. My tests failed with 10Mb for
> + some unknown reason.
> + """
> + args = ["if=/dev/zero" , "of=%s"%device] + \
> + ["seek=1", "bs=512", "count=400"]
> + rc = iutil.execWithRedirect("dd", args,
> + stdout = "/dev/tty5",
> + stderr = "/dev/tty5",
> + searchPath=1)
> + if rc:
> + raise LVMError("dd failed for %s" % device)
> +
> +
I see 100Mb, 10Mb, and 20Mb, mentioned here, and your summary says 10Mb.
Which is it? (I know, it's 20, but the comments need cleanup.)
> def getPossiblePhysicalExtents(floor=0):
> """Returns a list of integers representing the possible values for
> the physical extent of a volume group. Value is in KB.
> @@ -171,6 +188,60 @@ def pvinfo(device):
>
> return info
>
> +def pvstatus_ok(device):
> + """ Check validity of lvm structure with Physical Volume device.
This is problematic, besides being the way we did things before that so
often did not work.
At any given time during scanning you cannot expect that all of a VG's
PVs are available or visible (think LUKS, PV-on-mdraid, &c). The way to
do this is to check things out after we finish scanning. One simple
thing to do is add a property 'complete' to LVMVolumeGroupDevice that
compares the length of self.pvs with self.pvCount (if not None).
Wherever possible, we should build up the device/format classes instead
of stuffing tons of utility functions into devicelibs/.
> -def vgreduce(vg_name, pv_list):
> - rc = iutil.execWithRedirect("lvm", ["vgreduce", vg_name] + pv_list,
> +def vgreduce(vg_name, pv_list, rm=False):
> + """ Reduce a VG.
> +
> + rm -> with RemoveMissing option.
> + Use pv_list when rm=False, otherwise ignore pv_list and call vgreduce with
> + the --removemissing option.
> + """
This one is good.
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index e974717..fb82e28 100644
> --- a/storage/devicetree.py
> +++ b/storage/devicetree.py
> @@ -135,6 +135,35 @@ def questionInitializeDisk(intf=None, name=None):
> retVal = True
> return retVal
>
> +def questionClobberInconsistengVG(intf=None, pv_name=None, vg_name=None):
I think it should be "Inconsistent" or "Incomplete".
> @@ -1168,12 +1197,52 @@ class DeviceTree(object):
> try:
> vg_name = udev_device_get_vg_name(info)
> except KeyError:
> - # no vg name means no vg -- we're done with this pv
> + # This means either there are valid PVs with no VG. Or,
> + # leftover PVs from an incomplete VG.
> + if not lvm.pvstatus_ok(device.path):
> + if questionClobberInconsistengVG(intf = self.intf,
> + pv_name = device.path):
> + lvm.pvremove(device.path)
> + # Give the device the a default format
> + for arg in ["vgName", "vgUuid", "peStart"]:
> + if kwargs.has_key(arg):
> + del kwargs[arg]
> + device.format = formats.getFormat(*[""], **kwargs)
> +
> + else:
> + self.ignoredDisks.append(device.name)
It is fine to do this check, but it should be done at a point when we
can expect to know everything there is to know.
Dave
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
