[PATCH 2/4] Clean up disk (re)initialization code.

David Lehman dlehman at redhat.com
Tue Jul 10 16:38:34 UTC 2012


CLEARPART_TYPE_ALL means clear existing partitions and create a new
disklabel.

CLEARPART_TYPE_LINUX means clear linux-native partitions (or linux-
native whole-disk formatting) and create a new disklabel if the
removals result in an empty disk.

CLEARPART_TYPE_LIST means clear the specified devices (and devices
built on top of them) and create a new disklabel if the removals
result in an empty disk.

Zerombr becomes an alias for 'clearpart --initlabel'.

To initialize all disks:

  clearpart --all

To initialize uninitalized disks but otherwise clear no partitions:

  clearpart --none --initlabel

This adds a flag to the storage discovery config class that determines
whether or not clearPartitions will remove not-yet-created devices and
disklabels. The default is to not clear non-existent devices/formats.

After the first trip to the storage spoke, we should not remove any
layout the user set up in previous visits unless they choose autopart
again, in which case we start from scratch each time, or de-select
disks. Since the disk set can change each time through we have to be
able to initialize uninitialized disks even on the way to the custom ui.
---
 pyanaconda/iw/filter_gui.py             |    2 +-
 pyanaconda/kickstart.py                 |    2 +-
 pyanaconda/storage/__init__.py          |  140 +++++++++++++++----------------
 pyanaconda/storage/devices.py           |    9 ++
 pyanaconda/storage/devicetree.py        |    1 -
 pyanaconda/storage/formats/disklabel.py |   10 ++
 pyanaconda/storage/partitioning.py      |    2 +-
 pyanaconda/ui/gui/spokes/storage.py     |   12 ++-
 8 files changed, 100 insertions(+), 78 deletions(-)

diff --git a/pyanaconda/iw/filter_gui.py b/pyanaconda/iw/filter_gui.py
index 40b9563..59da8fd 100644
--- a/pyanaconda/iw/filter_gui.py
+++ b/pyanaconda/iw/filter_gui.py
@@ -621,7 +621,7 @@ class FilterWindow(InstallWindow):
         zfcp.ZFCP().startup(anaconda.intf)
         dasd.DASD().startup(anaconda.intf,
                                     anaconda.storage.config.exclusiveDisks,
-                                    anaconda.storage.config.zeroMbr)
+                                    anaconda.storage.config.initializeDisks)
         disks = self._getFilterDisks()
 
         mcw = MultipathConfigWriter()
diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
index a3a6c5d..42723e0 100644
--- a/pyanaconda/kickstart.py
+++ b/pyanaconda/kickstart.py
@@ -412,7 +412,7 @@ class ClearPart(commands.clearpart.F17_ClearPart):
         storage.config.clearPartDevices = self.devices
 
         if self.initAll:
-            storage.config.reinitializeDisks = self.initAll
+            storage.config.initializeDisks = self.initAll
 
         storage.clearPartitions()
 
diff --git a/pyanaconda/storage/__init__.py b/pyanaconda/storage/__init__.py
index db566b4..5f55067 100644
--- a/pyanaconda/storage/__init__.py
+++ b/pyanaconda/storage/__init__.py
@@ -77,16 +77,7 @@ def storageInitialize(storage, ksdata, protected):
 
     # Before we set up the storage system, we need to know which disks to
     # ignore, etc.  Luckily that's all in the kickstart data.
-    storage.config.zeroMbr = ksdata.zerombr.zerombr
-    storage.config.ignoreDiskInteractive = ksdata.ignoredisk.interactive
-    storage.config.ignoredDisks = ksdata.ignoredisk.ignoredisk
-    storage.config.exclusiveDisks = ksdata.ignoredisk.onlyuse
-
-    if ksdata.clearpart.type is not None:
-        storage.config.clearPartType = ksdata.clearpart.type
-        storage.config.clearPartDisks = ksdata.clearpart.drives
-        if ksdata.clearpart.initAll:
-            storage.config.reinitializeDisks = ksdata.clearpart.initAll
+    storage.config.update(ksdata)
 
     lvm.lvm_vg_blacklist = []
 
@@ -317,12 +308,24 @@ class StorageDiscoveryConfig(object):
         self.clearPartType = None
         self.clearPartDisks = []
         self.clearPartDevices = []
-        self.reinitializeDisks = False
-        self.zeroMbr = None
+        self.initializeDisks = False
         self.protectedDevSpecs = []
         self.diskImages = {}
         self.mpathFriendlyNames = True
 
+        # Whether clearPartitions removes scheduled/non-existent devices and
+        # disklabels depends on this flag.
+        self.clearNonExistent = False
+
+    def update(self, ksdata):
+        self.ignoredDisks = ksdata.ignoredisk.ignoredisk[:]
+        self.exclusiveDisks = ksdata.ignoredisk.onlyuse[:]
+        self.clearPartType = ksdata.clearpart.type
+        self.clearPartDisks = ksdata.clearpart.drives[:]
+        self.clearPartDevices = ksdata.clearpart.devices[:]
+        self.initializeDisks = ksdata.clearpart.initAll
+        self.zeroMbr = ksdata.zerombr.zerombr
+
     def writeKS(self, f):
         # clearpart
         if self.clearPartType is None or self.clearPartType == CLEARPART_TYPE_NONE:
@@ -334,7 +337,7 @@ class StorageDiscoveryConfig(object):
 
         if self.clearPartDisks:
             args += ["--drives=%s" % ",".join(self.clearPartDisks)]
-        if self.reinitializeDisks:
+        if self.initializeDisks:
             args += ["--initlabel"]
 
         f.write("#clearpart %s\n" % " ".join(args))
@@ -483,13 +486,16 @@ class Storage(object):
             if device.format.type == "luks" and device.format.exists:
                 self.__luksDevs[device.format.uuid] = device.format._LUKS__passphrase
 
+        if self.data:
+            self.config.update(self.data)
+
         if not flags.imageInstall:
             self.iscsi.startup()
             self.fcoe.startup()
             self.zfcp.startup()
             self.dasd.startup(None,
                               self.config.exclusiveDisks,
-                              self.config.zeroMbr)
+                              self.config.initializeDisks)
         clearPartType = self.config.clearPartType # save this before overriding it
         if self.data and self.data.upgrade.upgrade:
             self.config.clearPartType = CLEARPART_TYPE_NONE
@@ -519,15 +525,6 @@ class Storage(object):
 
         self.dumpState("initial")
 
-        # if zerombr is set, go ahead and slap a disklabel on any disk that
-        # doesn't have (recognizeable) formatting
-        if self.config.zeroMbr:
-            for disk in self.disks:
-                if disk.format.type is None:
-                    log.info("zerombr: initializing %s" % disk.name)
-                    self.reinitializeDisk(disk)
-
-        # we may have added candidate boot disks just above by initializing
         self.updateBootLoaderDiskList()
 
     @property
@@ -754,22 +751,37 @@ class Storage(object):
         clearPartDevices = kwargs.get("clearPartDevices",
                                       self.config.clearPartDevices)
 
-        if clearPartType in [CLEARPART_TYPE_NONE, None]:
+        for disk in device.disks:
+            # this will not include disks with hidden formats like multipath
+            # and firmware raid member disks
+            if clearPartDisks and disk.name not in clearPartDisks:
+                return False
+
+        if not self.config.clearNonExistent:
+            if not device.exists:
+                return False
+
+            if device.isDisk:
+                if not device.format.exists:
+                    return False
+
+                for partition in self.devicetree.getChildren(device):
+                    if not (partition.isMagic or self.shouldClear(partition)):
+                        return False
+
+        # the only devices we want to clear when clearPartType is
+        # CLEARPART_TYPE_NONE are uninitialized disks in clearPartDisks, and
+        # then only when we have been asked to initialize disks as needed
+        if clearPartType in [CLEARPART_TYPE_NONE, None] and \
+           not (device.isDisk and device.format.type is None and
+                self.config.initializeDisks):
             return False
 
         if isinstance(device, PartitionDevice):
             # Never clear the special first partition on a Mac disk label, as
             # that holds the partition table itself.
             # Something similar for the third partition on a Sun disklabel.
-            if device.disk.format.labelType == "mac" and \
-               device.partedPartition.number == 1:
-                return False
-            elif device.disk.format.labelType == "sun" and \
-                 device.partedPartition.number == 3:
-                return False
-
-            # If we got a list of disks to clear, make sure this one's on it
-            if clearPartDisks and device.disk.name not in clearPartDisks:
+            if device.isMagic:
                 return False
 
             # We don't want to fool with extended partitions, freespace, &c
@@ -782,17 +794,27 @@ class Storage(object):
                not device.getFlag(parted.PARTITION_RAID) and \
                not device.getFlag(parted.PARTITION_SWAP):
                 return False
-        elif device.isDisk and not device.partitioned:
-            # If we got a list of disks to clear, make sure this one's on it
-            if clearPartDisks and device.name not in clearPartDisks:
-                return False
+        elif device.isDisk:
+            if device.partitioned and clearPartType != CLEARPART_TYPE_ALL:
+                # if clearPartType is not CLEARPART_TYPE_ALL but we'll still be
+                # removing every partition from the disk, return True since we
+                # will want to be able to create a new disklabel on this disk
+                for partition in self.devicetree.getChildren(device):
+                    if not (partition.isMagic or self.shouldClear(partition)):
+                        return False
 
             # Never clear disks with hidden formats
             if device.format.hidden:
                 return False
 
+            # When clearPartType is CLEARPART_TYPE_LINUX and a disk has non-
+            # linux whole-disk formatting, do not clear it. The exception is
+            # the case of an uninitialized disk when we've been asked to
+            # initialize disks as needed
             if clearPartType == CLEARPART_TYPE_LINUX and \
-               not device.format.linuxNative:
+               not (self.config.initializeDisks and
+                    device.format.type is None) and \
+               not device.partitioned and not device.format.linuxNative:
                 return False
 
         # Don't clear devices holding install media.
@@ -836,22 +858,16 @@ class Storage(object):
                 - Needs some error handling
 
         """
-        if self.config.clearPartType in (None, CLEARPART_TYPE_NONE):
-            # not much to do
-            return
-
-        # XXX FIXME: We can still clear partitions. What we can't do is create
-        #            appropriately-typed disklabels.
         if not hasattr(self.platform, "diskLabelTypes"):
             raise StorageError("can't clear partitions without platform data")
 
-        # we are only interested in partitions that physically exist
-        partitions = [p for p in self.partitions if p.exists]
         # Sort partitions by descending partition number to minimize confusing
         # things like multiple "destroy sda5" actions due to parted renumbering
         # partitions. This can still happen through the UI but it makes sense to
         # avoid it where possible.
-        partitions.sort(key=lambda p: p.partedPartition.number, reverse=True)
+        partitions = sorted(self.partitions,
+                            key=lambda p: p.partedPartition.number,
+                            reverse=True)
         for part in partitions:
             log.debug("clearpart: looking at %s" % part.name)
             if not self.shouldClear(part):
@@ -863,35 +879,17 @@ class Storage(object):
         # now remove any empty extended partitions
         self.removeEmptyExtendedPartitions()
 
-        # make sure that the the boot device has the correct disklabel type if
-        # we're going to completely clear it.
-        # also handles clearpart --initlabel
-        boot_disk = self.bootDisk
-        for disk in self.partitioned:
-            if not boot_disk and not self.config.reinitializeDisks:
-                break
-
-            if not self.config.reinitializeDisks and \
-               (boot_disk is not None and disk != boot_disk):
-                continue
-
+        # ensure all disks have appropriate disklabels
+        for disk in self.disks:
             if not self.shouldClear(disk):
                 continue
 
-            # don't reinitialize the disklabel if the disk contains install media
-            # or pvs from inconsistent vgs
-            if filter(lambda p: p.dependsOn(disk), self.protectedDevices):
-                continue
-
-            if not self.config.reinitializeDisks and \
-               disk.format.labelType == self.platform.bestDiskLabelType(disk):
-                continue
-
-            self.reinitializeDisk(disk)
+            log.debug("clearpart: initializing %s" % disk.name)
+            self.initializeDisk(disk)
 
         self.updateBootLoaderDiskList()
 
-    def reinitializeDisk(self, disk):
+    def initializeDisk(self, disk):
         """ (Re)initialize a disk by creating a disklabel on it.
 
             The disk should not contain any partitions except perhaps for a
@@ -912,7 +910,7 @@ class Storage(object):
                         self.devicetree._removeDevice(part, moddisk=False)
 
         if disk.partitioned and disk.format.partitions:
-            raise ValueError("cannot reinitialize a disk that has partitions")
+            raise ValueError("cannot initialize a disk that has partitions")
 
         # remove existing formatting from the disk
         destroy_action = ActionDestroyFormat(disk)
diff --git a/pyanaconda/storage/devices.py b/pyanaconda/storage/devices.py
index 1e22138..6b979e8 100644
--- a/pyanaconda/storage/devices.py
+++ b/pyanaconda/storage/devices.py
@@ -1446,6 +1446,15 @@ class PartitionDevice(StorageDevice):
 
         self.partedPartition.unsetFlag(flag)
 
+    @property
+    def isMagic(self):
+        if not self.disk:
+            return False
+
+        number = getattr(self.partedPartition, "number", -1)
+        magic = self.disk.format.magicPartitionNumber
+        return (number == magic)
+
     def probe(self):
         """ Probe for any missing information about this device.
 
diff --git a/pyanaconda/storage/devicetree.py b/pyanaconda/storage/devicetree.py
index 907c315..2c36f83 100644
--- a/pyanaconda/storage/devicetree.py
+++ b/pyanaconda/storage/devicetree.py
@@ -170,7 +170,6 @@ class DeviceTree(object):
         self.populated = False
 
         self.exclusiveDisks = getattr(conf, "exclusiveDisks", [])
-        self.zeroMbr = getattr(conf, "zeroMbr", False)
         self.shouldClear = shouldClear or (lambda d: False)
         self.iscsi = iscsi
         self.dasd = dasd
diff --git a/pyanaconda/storage/formats/disklabel.py b/pyanaconda/storage/formats/disklabel.py
index 16def28..338a880 100644
--- a/pyanaconda/storage/formats/disklabel.py
+++ b/pyanaconda/storage/formats/disklabel.py
@@ -426,5 +426,15 @@ class DiskLabel(DeviceFormat):
 
         return free
 
+    @property
+    def magicPartitionNumber(self):
+        """ Number of disklabel-type-specific special partition. """
+        if self.labelType == "mac":
+            return 1
+        elif self.labelType == "sun":
+            return 3
+        else:
+            return 0
+
 register_device_format(DiskLabel)
 
diff --git a/pyanaconda/storage/partitioning.py b/pyanaconda/storage/partitioning.py
index b77f499..74eb685 100644
--- a/pyanaconda/storage/partitioning.py
+++ b/pyanaconda/storage/partitioning.py
@@ -295,8 +295,8 @@ def doAutoPartition(storage, data):
     devs = []
 
     if storage.doAutoPart:
+        # XXX this doesn't belong on newui
         scheduleShrinkActions(storage)
-        storage.clearPartitions()
 
         disks = _getCandidateDisks(storage)
         devs = _scheduleImplicitPartitions(storage, disks)
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 8771132..4cf3f5e 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -265,10 +265,16 @@ class StorageSpoke(NormalSpoke):
 
         self.data.bootloader.location = "mbr"
 
+        self.data.clearpart.initAll = True
         self.data.clearpart.type = self.clearPartType
-
-        if self.autopart:
-            self.data.clearpart.execute(self.storage, self.data, self.instclass)
+        self.storage.config.update(self.data)
+
+        # If autopart is selected we want to remove whatever has been
+        # created/scheduled to make room for autopart.
+        # If custom is selected, we want to leave alone any storage layout the
+        # user may have set up before now.
+        self.storage.config.clearNonExistent = self.data.autopart.autopart
+        self.data.clearpart.execute(self.storage, self.data, self.instclass)
 
         # Pick the first disk to be the destination device for the bootloader.
         # This appears to be the minimum amount of configuration required to
-- 
1.7.7.6



More information about the anaconda-patches mailing list