Re: [PATCH 2/3] Add proper support for destruction of disklabels.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 04/15/2010 04:53 PM, David Lehman wrote:
On Thu, 2010-04-15 at 10:34 +0200, Hans de Goede wrote:
Hi,

On 04/15/2010 04:05 AM, David Lehman wrote:
With this patch we can destroy a disklabel of one type and then create
a disklabel of a different type in the process of partitioning.
---
   storage/devices.py           |   42 ++++++++++++++++++++++++++++++------------
   storage/devicetree.py        |   12 ++++++++----
   storage/formats/disklabel.py |    5 +++++
   3 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index 2e36df0..8cfe692 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -1107,16 +1107,6 @@ class PartitionDevice(StorageDevice):
           return spec

       def _getPartedPartition(self):
-        if not self.exists:
-            return self._partedPartition
-
-        partitionDisk = self._partedPartition.disk.getPedDisk()
-        disklabelDisk = self.disk.format.partedDisk.getPedDisk()
-        if partitionDisk is not disklabelDisk:
-            # The disklabel's parted disk has been reset, reget our partition
-            log.debug("regetting partedPartition %s for %s because of PartedDisk reset" % (self._origPath, self.path))
-            self._setPartedPartition(self.disk.format.partedDisk.getPartitionByPath(self._origPath))
-
           return self._partedPartition

       def _setPartedPartition(self, partition):
@@ -1138,6 +1128,31 @@ class PartitionDevice(StorageDevice):
       partedPartition = property(lambda d: d._getPartedPartition(),
                                  lambda d,p: d._setPartedPartition(p))

+    def resetPartedPartition(self):
+        """ Re-get self.partedPartition from the original disklabel. """
+        log_method_call(self, self.name)
+        if not self.exists:
+            return
+
+        # find the correct partition on the original parted.Disk since the
+        # name/number we're now using may no longer match
+        _disklabel = self.disk.originalFormat
+
+        if self.isExtended:
+            # getPartitionBySector doesn't work on extended partitions
+            _partition = _disklabel.extendedPartition
+            log.debug("extended lookup found partition %s"
+                        % devicePathToName(getattr(_partition, "path", None)))
+        else:
+            # lookup the partition by sector to avoid the renumbering
+            # nonsense entirely
+            _sector = self.partedPartition.geometry.start + 1
+            _partition = _disklabel.partedDisk.getPartitionBySector(_sector)
+            log.debug("sector-based lookup found partition %s"
+                        % devicePathToName(getattr(_partition, "path", None)))
+
+        self.partedPartition = _partition
+
       def _getWeight(self):
           return self.req_base_weight


Oh, lookup by sector, I like, but why the +1 in:
              _sector = self.partedPartition.geometry.start + 1

This could theoretically cause issues with 1 sector large partitions

It was a first attempt, and I wasn't sure if using the start sector
might have issues. It turns out that you can pass the start or end
sector and still get the desired result, so this is fine for
single-sector partitions.



@@ -1341,8 +1356,11 @@ class PartitionDevice(StorageDevice):
               raise DeviceError("Cannot destroy non-leaf device", self.name)

           self.setupParents(orig=True)
-        self.disk.format.removePartition(self.partedPartition)
-        self.disk.format.commit()
+
+        # we should have already set self.partedPartition to point to the
+        # partition on the original disklabel
+        self.disk.originalFormat.removePartition(self.partedPartition)
+        self.disk.originalFormat.commit()

           self.exists = False

diff --git a/storage/devicetree.py b/storage/devicetree.py
index 75dde9c..c96bfbd 100644
--- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -627,16 +627,20 @@ class DeviceTree(object):
           for device in self.devices:
               if device.partitioned:
                   device.format.resetPartedDisk()
+                if device.originalFormat.type == "disklabel" and \
+                   device.originalFormat != device.format:
+                    device.originalFormat.resetPartedDisk()


Shouldn't the comparison be a "is not" in this case ?

In this case I believe the two are equivalent. We have not defined any
comparison operations for DeviceFormat classes, so python uses the
instance id, which is also its address.


you're falling (in the I agree sucky) parted geometry trap again, a
geometries end sector is the last sector of the partition so a 1 sector geometry
has start x and end also x (not x + 1). I agree we will likely never ever
see a 1 sector partition, but still.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux