FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Desktop

 
 
LinkBack Thread Tools
 
Old 04-15-2010, 02:05 AM
David Lehman
 
Default Add proper support for destruction of disklabels.

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.partedDi sk.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

@@ -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.partedPartit ion)
- 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.part edPartition)
+ 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()

# reget parted.Partition for remaining preexisting devices
for device in self.devices:
- if isinstance(device, PartitionDevice):
- p = device.partedPartition
+ if isinstance(device, PartitionDevice) and device.exists:
+ device.resetPartedPartition()

# reget parted.Partition for existing devices we're removing
for action in self._actions:
- if isinstance(action.device, PartitionDevice):
- p = action.device.partedPartition
+ if isinstance(action.device, PartitionDevice) and
+ action.device.exists:
+ action.device.resetPartedPartition()

# setup actions to create any extended partitions we added
#
diff --git a/storage/formats/disklabel.py b/storage/formats/disklabel.py
index d4a706d..a76e452 100644
--- a/storage/formats/disklabel.py
+++ b/storage/formats/disklabel.py
@@ -219,6 +219,11 @@ class DiskLabel(DeviceFormat):
raise DeviceFormatError("device exists and is active")

DeviceFormat.create(self, *args, **kwargs)
+
+ # We're relying on someone having called resetPartedDisk -- we
+ # could ensure a fresh disklabel by setting self._partedDisk to
+ # None right before calling self.commit(), but that might hide
+ # other problems.
self.commit()
self.exists = True

--
1.6.6

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-15-2010, 08:34 AM
Hans de Goede
 
Default Add proper support for destruction of disklabels.

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.partedDi sk.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



@@ -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.partedPartit ion)
- 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.part edPartition)
+ 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 ?


# reget parted.Partition for remaining preexisting devices
for device in self.devices:
- if isinstance(device, PartitionDevice):
- p = device.partedPartition
+ if isinstance(device, PartitionDevice) and device.exists:
+ device.resetPartedPartition()

# reget parted.Partition for existing devices we're removing
for action in self._actions:
- if isinstance(action.device, PartitionDevice):
- p = action.device.partedPartition
+ if isinstance(action.device, PartitionDevice) and
+ action.device.exists:
+ action.device.resetPartedPartition()

# setup actions to create any extended partitions we added
#
diff --git a/storage/formats/disklabel.py b/storage/formats/disklabel.py
index d4a706d..a76e452 100644
--- a/storage/formats/disklabel.py
+++ b/storage/formats/disklabel.py
@@ -219,6 +219,11 @@ class DiskLabel(DeviceFormat):
raise DeviceFormatError("device exists and is active")

DeviceFormat.create(self, *args, **kwargs)
+
+ # We're relying on someone having called resetPartedDisk -- we
+ # could ensure a fresh disklabel by setting self._partedDisk to
+ # None right before calling self.commit(), but that might hide
+ # other problems.
self.commit()
self.exists = True



Other then the 2 small remarks above I like this

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-15-2010, 02:22 PM
David Cantrell
 
Default Add proper support for destruction of disklabels.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ack. For rhel6-branch, please include the rhbz reference (either Resolves,
Related, or Conflicts).

On Wed, 14 Apr 2010, 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.partedDi sk.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

@@ -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.partedPartit ion)
- 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.part edPartition)
+ 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()

# reget parted.Partition for remaining preexisting devices
for device in self.devices:
- if isinstance(device, PartitionDevice):
- p = device.partedPartition
+ if isinstance(device, PartitionDevice) and device.exists:
+ device.resetPartedPartition()

# reget parted.Partition for existing devices we're removing
for action in self._actions:
- if isinstance(action.device, PartitionDevice):
- p = action.device.partedPartition
+ if isinstance(action.device, PartitionDevice) and
+ action.device.exists:
+ action.device.resetPartedPartition()

# setup actions to create any extended partitions we added
#
diff --git a/storage/formats/disklabel.py b/storage/formats/disklabel.py
index d4a706d..a76e452 100644
--- a/storage/formats/disklabel.py
+++ b/storage/formats/disklabel.py
@@ -219,6 +219,11 @@ class DiskLabel(DeviceFormat):
raise DeviceFormatError("device exists and is active")

DeviceFormat.create(self, *args, **kwargs)
+
+ # We're relying on someone having called resetPartedDisk -- we
+ # could ensure a fresh disklabel by setting self._partedDisk to
+ # None right before calling self.commit(), but that might hide
+ # other problems.
self.commit()
self.exists = True




- --
David Cantrell <dcantrell@redhat.com>

Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAkvHIRkACgkQ5hsjjIy1Vkki2ACgiQcSEOSjhO piyeckCh8baqTQ
+XsAn3ShNfBChr9fdnpWIRzrAe+Iw55M
=MWLi
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-15-2010, 02:53 PM
David Lehman
 
Default Add proper support for destruction of disklabels.

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.partedDi sk.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.partedPartit ion)
> > - 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.part edPartition)
> > + 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.

Dave

>
> > # reget parted.Partition for remaining preexisting devices
> > for device in self.devices:
> > - if isinstance(device, PartitionDevice):
> > - p = device.partedPartition
> > + if isinstance(device, PartitionDevice) and device.exists:
> > + device.resetPartedPartition()
> >
> > # reget parted.Partition for existing devices we're removing
> > for action in self._actions:
> > - if isinstance(action.device, PartitionDevice):
> > - p = action.device.partedPartition
> > + if isinstance(action.device, PartitionDevice) and
> > + action.device.exists:
> > + action.device.resetPartedPartition()
> >
> > # setup actions to create any extended partitions we added
> > #
> > diff --git a/storage/formats/disklabel.py b/storage/formats/disklabel.py
> > index d4a706d..a76e452 100644
> > --- a/storage/formats/disklabel.py
> > +++ b/storage/formats/disklabel.py
> > @@ -219,6 +219,11 @@ class DiskLabel(DeviceFormat):
> > raise DeviceFormatError("device exists and is active")
> >
> > DeviceFormat.create(self, *args, **kwargs)
> > +
> > + # We're relying on someone having called resetPartedDisk -- we
> > + # could ensure a fresh disklabel by setting self._partedDisk to
> > + # None right before calling self.commit(), but that might hide
> > + # other problems.
> > self.commit()
> > self.exists = True
> >
>
> Other then the 2 small remarks above I like this
>
> Regards,
>
> Hans
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@redhat.com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-15-2010, 03:04 PM
Hans de Goede
 
Default Add proper support for destruction of disklabels.

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.partedDi sk.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.partedPartit ion)
- 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.part edPartition)
+ 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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 04-15-2010, 03:38 PM
David Lehman
 
Default Add proper support for destruction of disklabels.

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.partedDi sk.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.partedPartit ion)
> >>> - 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.part edPartition)
> >>> + 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@redhat.com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


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

Thread Tools




All times are GMT. The time now is 09:36 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org