[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