[PATCH 2/2] Update partitions' numbers and names when adding new partition (#1166598)
Vratislav Podzimek
vpodzime at redhat.com
Thu Dec 4 08:50:53 UTC 2014
On Mon, 2014-12-01 at 08:29 -0500, Anne Mulhern wrote:
>
>
>
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Wednesday, November 26, 2014 8:37:32 AM
> > Subject: [PATCH 2/2] Update partitions' numbers and names when adding new partition (#1166598)
> >
> > Adding a new partition may change partitions' order (e.g. adding sda1 after
> > sda2) so we need to handle that correctly.
> >
> > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > ---
> > blivet/devicetree.py | 9 +++++++++
> > blivet/formats/disklabel.py | 26 ++++++++++++++++++++++++--
> > python-blivet.spec | 2 +-
> > 3 files changed, 34 insertions(+), 3 deletions(-)
> >
> > diff --git a/blivet/devicetree.py b/blivet/devicetree.py
> > index f6cf600..98b1251 100644
> > --- a/blivet/devicetree.py
> > +++ b/blivet/devicetree.py
> > @@ -26,6 +26,7 @@ import re
> > import shutil
> > import pprint
> > import copy
> > +import itertools
> >
> > from .errors import CryptoError, DeviceError, DeviceTreeError,
> > DiskLabelCommitError, DMError, FSError, InvalidDiskLabelError, LUKSError,
> > MDRaidError, StorageError
> > from .devices import BTRFSDevice, BTRFSSubVolumeDevice, BTRFSVolumeDevice,
> > BTRFSSnapShotDevice
> > @@ -402,6 +403,14 @@ class DeviceTree(object):
> > newdev.addHook(new=new)
> > self._devices.append(newdev)
> >
> > + # adding a partition may change partedPartition's paths which are
> > used
> > + # to construct names which thus should be updated
> > + if isinstance(newdev, PartitionDevice):
> > + sibling_parts_lists = [self.getChildren(parent) for parent in
> > newdev.parents]
>
> Any reason not to make the above lazy, instead of a list?
The reason was that itertools.chain() is a bit weird and combination of
list + the * expansion was finally a working one. But a generator would
work the same.
>
> > + sibling_parts = itertools.chain(*sibling_parts_lists)
> > + for part in sibling_parts:
> > + part.updateName()
> > +
> > # don't include "req%d" partition names
> > if ((newdev.type != "partition" or
> > not newdev.name.startswith("req")) and
> > diff --git a/blivet/formats/disklabel.py b/blivet/formats/disklabel.py
> > index c69aa04..590b58c 100644
> > --- a/blivet/formats/disklabel.py
> > +++ b/blivet/formats/disklabel.py
> > @@ -311,8 +311,30 @@ class DiskLabel(DeviceFormat):
> > geometry=geometry)
> >
> > constraint = parted.Constraint(exactGeom=geometry)
> > - self.partedDisk.addPartition(partition=new_partition,
> > - constraint=constraint)
> > +
> > + # adding a partition the start of which is before some already added
> > + # partition requires change of partitions' order because parted just
> > + # increments the number while partitions are added
> > + if any(part.geometry.start > start for part in
> > self.partedDisk.partitions):
> > + parts = self.partedDisk.partitions[:]
> > +
> > + # first remove all partitions
> > + for part in parts:
> > + self.partedDisk.removePartition(part)
> > +
> > + # then add them back sorted together with the new one
> > + parts.append(new_partition)
> > + parts.sort(key=lambda p: p.geometry.start)
> > + for part in parts:
> > + # reset the partition's number so that it gets a new one
> > when
> > + # added to partedDisk
> > + part.resetNumber()
>
> It feels like it would be better to handle this number resetting within pyparted,
> probably in Disk.addPartition().
It definitely feels like that, but parted/pyparted has very little
information about the partitions. It has no idea if the given partition
exists on a disk or not, if we are putting it back or it's completely
new or if we are doing something weird like reverting actions or just we
just want to add a new partition.
>
> > + constr = parted.Constraint(exactGeom=part.geometry)
> > + self.partedDisk.addPartition(partition=part,
> > + constraint=constr)
> > + else:
> > + self.partedDisk.addPartition(partition=new_partition,
> > +
>
> overall, the above code would be simpler if partitions were always kept sorted
> by start, and this method could rely on that holding true.
Cannot agree more. But as it turned out yesterday, we cannot even rely
on partitions existing on a disk having their numbers in the ascending
order when ordered by start.
This patch had to be reverted and we need something better for master
(and rhel7-branch too, I'm afraid).
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list