[PATCH 3/3] Minimize number of times storage.partitions is accessed. (#1155984)

David Lehman dlehman at redhat.com
Fri Aug 21 17:56:41 UTC 2015


On 08/21/2015 12:25 PM, Samantha N. Bueno wrote:
> This is also a non-trivial culprit in causing disk partitioning to seem
> slow. Every time storage.partitions is accessed, getDevicesByInstance()
> is called, which, collectively, had significant overhead with high
> numbers of disks.
>
> Resolves: rhbz#1155984
> ---
>   blivet/partitioning.py | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> index f0a9a7a..c6c0ca5 100644
> --- a/blivet/partitioning.py
> +++ b/blivet/partitioning.py
> @@ -860,11 +860,12 @@ def updateExtendedPartitions(storage, disks):
>       """
>       # XXX hack -- if we created any extended partitions we need to add
>       #             them to the tree now
> +    partitions = storage.partitions
>       for disk in disks:
>           extended = disk.format.extendedPartition
>           if not extended:
>               # remove any obsolete extended partitions
> -            for part in storage.partitions:
> +            for part in partitions:
>                   if part.disk == disk and part.isExtended:
>                       if part.exists:
>                           storage.destroyDevice(part)
> @@ -880,7 +881,7 @@ def updateExtendedPartitions(storage, disks):
>                   device.partedPartition = extended
>
>           # remove any obsolete extended partitions
> -        for part in storage.partitions:
> +        for part in partitions:
>               if part.disk == disk and part.isExtended and \
>                  part.partedPartition not in disk.format.partitions:
>                   if part.exists:
> @@ -929,13 +930,13 @@ def doPartitioning(storage):
>       # XXX This does not remove the extended from the parted.Disk, but it should
>       #     cause removeNewPartitions to remove it since there will no longer be
>       #     a PartitionDevice for it.
> -    for partition in storage.partitions:
> +    partitions = storage.partitions
> +    for partition in partitions:
>           if not partition.exists and partition.isExtended and \
>              not storage.devicetree.findActions(device=partition, action_type="create"):
>               storage.devicetree._removeDevice(partition, modparent=False, force=True)

Since you might be removing a partition from the list here you should
also remove it from 'partitions' at the same time, which will mean you
should iterate over a slice/copy of partitions in the above loop.

In the same spirit, please make sure nothing is done in between the
rest of these that might change storage.partitions.

David

>
> -    partitions = storage.partitions[:]
> -    for part in storage.partitions:
> +    for part in partitions:
>           part.req_bootable = False
>           if not part.exists:
>               # start over with flexible-size requests
> @@ -956,7 +957,7 @@ def doPartitioning(storage):
>           raise
>       else:
>           # Mark all growable requests as no longer growable.
> -        for partition in storage.partitions:
> +        for partition in partitions:
>               log.debug("fixing size of %s", partition)
>               partition.req_grow = False
>               partition.req_base_size = partition.size
> @@ -968,7 +969,7 @@ def doPartitioning(storage):
>           # The number and thus the name of partitions may have changed now,
>           # allocatePartitions() takes care of this for new partitions, but not
>           # for pre-existing ones, so we update the name of all partitions here
> -        for part in storage.partitions:
> +        for part in partitions:
>               # leave extended partitions as-is -- we'll handle them separately
>               if part.isExtended:
>                   continue
> @@ -976,7 +977,7 @@ def doPartitioning(storage):
>
>           updateExtendedPartitions(storage, disks)
>
> -        for part in [p for p in storage.partitions if not p.exists]:
> +        for part in [p for p in partitions if not p.exists]:
>               problem = part.checkSize()
>               if problem < 0:
>                   raise PartitioningError(_("partition is too small for %(format)s formatting "
>



More information about the anaconda-patches mailing list