Re: [PATCH 1/2] Don't remove an inconsistent lvm partition from the devicetree (#496638)

[David Lehman <dlehman redhat com>]

On Thu, 2009-04-23 at 17:01 +0200, Hans de Goede wrote:
> When a PV partition is part of an inconsistent VG, and the user chooses
> to ignore it we currently remove it from the devicetree, but since
> partition_gui.py populate() uses parted on the disk to find out about
> the partitions, it will still see it and then backtrace when it cannot
> find it in the devicetree.

I've forgotten why we can't just ignore partitions we find that aren't
in the device tree. Other than FREESPACE partitions, I can't think of a
reason why we'd want to do anything with such a partition, unless it
involves our naming shenanigans with new partitions.

> 
> This patch fixes this by instead making the partition immutable. Note
> that I've put the reason for it being immutable in the new immutableDevices
> array, so that if we have similar cases in the future we can use
> immutableDevices for that too. We might even want to move some of our
> existing cases there.

We could also just add a check in Storage/deviceImmutable to see if it's
listed/marked as part of a broken VG.

I'm not actually trying to be a pain in the ass -- I just think that
adding another list of devices/reasons means we're doing something
wrong.

Dave

> ---
>  storage/__init__.py   |    4 ++++
>  storage/devicetree.py |   15 +++++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/storage/__init__.py b/storage/__init__.py
> index 5b6e0f6..c05d9a4 100644
> --- a/storage/__init__.py
> +++ b/storage/__init__.py
> @@ -537,6 +537,10 @@ class Storage(object):
>                      msg += "%s: %s" % (dev, reasons[dev])
>                  return msg
>  
> +        for i in self.devicetree.immutableDevices:
> +            if i[0] == device.name:
> +                return i[1]
> +
>          return False
>  
>      def deviceDeps(self, device):
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index 996e702..85bef4b 100644
> --- a/storage/devicetree.py
> +++ b/storage/devicetree.py
> @@ -205,6 +205,7 @@ class DeviceTree(object):
>          self._ignoredDisks = []
>          for disk in ignored:
>              self.addIgnoredDisk(disk)
> +        self.immutableDevices = []
>          lvm.lvm_cc_resetFilter()
>  
>      def addIgnoredDisk(self, disk):
> @@ -1537,7 +1538,12 @@ class DeviceTree(object):
>                      lvm.lvm_cc_addFilterRejectRegexp(device.name)
>                      lvm.blacklistVG(device.name)
>                      for parent in device.parents:
> -                        self._removeDevice(parent, moddisk=False)
> +                        if parent.type == "partition":
> +                            self.immutableDevices.append([parent.name,
> +                                _("This partition is part of an inconsistent LVM Volume Group.")])
> +                        else:
> +                            self._removeDevice(parent, moddisk=False)
> +                            self.addIgnoredDisk(parent.name)
>                          lvm.lvm_cc_addFilterRejectRegexp(parent.name)
>  
>                  return
> @@ -1575,7 +1581,12 @@ class DeviceTree(object):
>                      lvm.blacklistVG(device.vg.name)
>                      # ignore all the pvs
>                      for parent in device.vg.parents:
> -                        self._removeDevice(parent, moddisk=False)
> +                        if parent.type == "partition":
> +                            self.immutableDevices.append([parent.name,
> +                                _("This partition is part of an inconsistent LVM Volume Group.")])
> +                        else:
> +                            self._removeDevice(parent, moddisk=False)
> +                            self.addIgnoredDisk(parent.name)
>                          lvm.lvm_cc_addFilterRejectRegexp(parent.name)
>                  return
>