[master/f20] Show Invalid Disk Label for damaged GPT (#1020974)

Vratislav Podzimek vpodzime at redhat.com
Wed Dec 4 07:10:07 UTC 2013


On Tue, 2013-12-03 at 17:47 -0800, Brian C. Lane wrote:
> From: "Brian C. Lane" <bcl at redhat.com>
> 
> A GPT labeled disk could have a damaged partition table, but still be
> useful. If that is the case we don't want to treat it like an empty
> disk. With these changes it will show up as 'Invalid Disk Label' which
> will be translated.
> 
> This patch also checks for partitions in the lvm filter that depend on
> devices being destroyed. If their dependent device is going away they
> are removed from the filter, preventing the lvm pv create from failing
> when devices with the same name are created.
> ---
>  blivet/devicetree.py        | 15 +++++++++++++--
>  blivet/formats/disklabel.py |  3 +--
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/blivet/devicetree.py b/blivet/devicetree.py
> index 1324499..b5f8f5d 100644
> --- a/blivet/devicetree.py
> +++ b/blivet/devicetree.py
> @@ -47,6 +47,7 @@ from flags import flags
>  from storage_log import log_method_call, log_method_return
>  import parted
>  import _ped
> +from i18n import _
>  
>  import logging
>  log = logging.getLogger("blivet")
> @@ -227,6 +228,13 @@ class DeviceTree(object):
>          for action in self._actions:
>              log.debug("action: %s" % action)
>  
> +            # Remove any partitions that depend on destroyed ones from the lvm filters.
> +            if isinstance(action, ActionDestroyDevice) or \
> +               isinstance(action, ActionDestroyFormat):
> +                for device in self._devices:
> +                    if device.dependsOn(action.device):
You could squash these two lines into a single one using the generator
expression. But I don't know which way you prefer more.

> +                        lvm.lvm_cc_removeFilterRejectRegexp(device.name)
> +
>          for action in self._actions[:]:
>              log.info("executing action: %s" % action)
>              if not dryRun:
> @@ -238,6 +246,7 @@ class DeviceTree(object):
>                      self.teardownAll()
>                      action.execute()
>  
> +
No extra blank line needed here, I guess.

>                  udev_settle()
>                  for device in self._devices:
>                      # make sure we catch any renumbering parted does
> @@ -1119,9 +1128,11 @@ class DeviceTree(object):
>                                 device=device.path,
>                                 labelType=labelType,
>                                 exists=True)
> -        except InvalidDiskLabelError:
> +        except InvalidDiskLabelError as e:
>              log.info("no usable disklabel on %s" % device.name)
> -            return
> +            if disklabel_type == "gpt":
> +                log.debug(e)
> +                device.format = getFormat(_("Invalid Disk Label"))
Shouldn't there be an else branch with the return statement to keep the
original behaviour for non-gpt labels?

>          else:
>              device.format = format
>  
> diff --git a/blivet/formats/disklabel.py b/blivet/formats/disklabel.py
> index a79f0a0..5015548 100644
> --- a/blivet/formats/disklabel.py
> +++ b/blivet/formats/disklabel.py
> @@ -150,7 +150,7 @@ class DiskLabel(DeviceFormat):
>                      self._partedDisk = parted.Disk(device=self.partedDevice)
>                  except (_ped.DiskLabelException, _ped.IOException,
>                          NotImplementedError) as e:
> -                    raise InvalidDiskLabelError()
> +                    raise InvalidDiskLabelError(e)
>  
>                  if self._partedDisk.type == "loop":
>                      # When the device has no partition table but it has a FS,
> @@ -444,4 +444,3 @@ class DiskLabel(DeviceFormat):
>              return 0
>  
>  register_device_format(DiskLabel)
> -
Otherwise this looks good to me.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list