On Thu, 2010-04-15 at 17:04 +0200, Hans de Goede wrote:
> 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.
Fair enough. Since it works using the start sector, there's no reason
not to do so. Changed locally.
Dave
>
> Regards,
>
> Hans
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
