On Fri, 2009-09-11 at 21:17 +0200, Hans de Goede wrote:
> Hi,
>
> Hmm, after your changes doDeleteDependentDevices() will only
> work for disks. Is it only used for disks ? If so it should be renamed
> IMHO.
Yeah, it could use some clarification. doDeleteDependentDevices is for
partitioned devices, while doDeleteDevice is for all others. I'll do a
follow-up patch to rename the functions and fix the possible infinite
loop you pointed out.
Dave
>
> Regards,
>
> Hans
>
>
> On 09/11/2009 07:55 PM, David Lehman wrote:
> > This helps us to avoid parted's renumbering nonsense.
> >
> > This patch also streamlines clearPartitions so that it removes the
> > partitions inside the loop instead of building a list and iterating over
> > it separately to remove the partitions.
> > ---
> > partIntfHelpers.py | 41 ++++++++++++++++++++++++++++-------------
> > storage/partitioning.py | 14 +++++---------
> > 2 files changed, 33 insertions(+), 22 deletions(-)
> >
> > diff --git a/partIntfHelpers.py b/partIntfHelpers.py
> > index a68704f..cf136fb 100644
> > --- a/partIntfHelpers.py
> > +++ b/partIntfHelpers.py
> > @@ -144,7 +144,11 @@ def doDeleteDevice(intf, storage, device, confirm=1, quiet=0):
> > return True
> >
> > def doDeleteDependentDevices(intf, storage, device, confirm=1, quiet=0):
> > - """ Remove all devices/partitions currently on device """
> > + """ Remove all devices/partitions currently on device.
> > +
> > + device -- a partitioned device such as a disk
> > +
> > + """
> > if confirm:
> > rc = intf.messageWindow(_("Confirm Delete"),
> > _("You are about to delete all partitions on "
> > @@ -155,20 +159,31 @@ def doDeleteDependentDevices(intf, storage, device, confirm=1, quiet=0):
> > if not rc:
> > return False
> >
> > - deps = storage.deviceDeps(device)
> > - if not deps:
> > - # nothing to do
> > + immutable = []
> > + partitions = [p for p in storage.partitions if p.disk == device]
> > + if not partitions:
> > return False
> >
> > - immutable = []
> > - while deps:
> > - leaves = [d for d in deps if d.isleaf]
> > - for leaf in leaves:
> > - if storage.deviceImmutable(leaf):
> > - immutable.append(leaf.path)
> > - else:
> > - storage.destroyDevice(leaf)
> > - deps.remove(leaf)
> > + partitions.sort(key=lambda p: p.partedPartition.number, reverse=True)
> > + for p in partitions:
> > + deps = storage.deviceDeps(p)
> > + clean = True # true if part and its deps were removed
> > + while deps:
> > + leaves = [d for d in deps if d.isleaf]
> > + for leaf in leaves:
> > + if storage.deviceImmutable(leaf):
> > + immutable.append(leaf.path)
> > + clean = False
> > + else:
> > + storage.destroyDevice(leaf)
> > + deps.remove(leaf)
> > +
> > + if storage.deviceImmutable(p):
> > + immutable.append(p.path)
> > + clean = False
> > +
> > + if clean:
> > + storage.destroyDevice(p)
> >
> > if immutable and not quiet:
> > remaining = "\t" + "\n\t".join(immutable) + "\n"
> > diff --git a/storage/partitioning.py b/storage/partitioning.py
> > index e1c2a8d..80724d6 100644
> > --- a/storage/partitioning.py
> > +++ b/storage/partitioning.py
> > @@ -324,8 +324,11 @@ def clearPartitions(storage):
> >
> > # we are only interested in partitions that physically exist
> > partitions = [p for p in storage.partitions if p.exists]
> > - disks = [] # a list of disks from which we've removed partitions
> > - clearparts = [] # list of partitions we'll remove
> > + # 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)
> > for part in partitions:
> > log.debug("clearpart: looking at %s" % part.name)
> > if not shouldClear(part, storage.clearPartType, storage.clearPartDisks):
> > @@ -345,13 +348,6 @@ def clearPartitions(storage):
> > devices.remove(leaf)
> >
> > log.debug("partitions: %s" % [p.getDeviceNodeName() for p in part.partedPartition.disk.partitions])
> > - disk_name = os.path.basename(part.partedPartition.disk.device.path)
> > - if disk_name not in disks:
> > - disks.append(disk_name)
> > -
> > - clearparts.append(part)
> > -
> > - for part in clearparts:
> > storage.destroyDevice(part)
> >
> > # now remove any empty extended partitions
>
> _______________________________________________
> 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
