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

Samantha N. Bueno sbueno+anaconda at redhat.com
Mon Sep 14 19:01:47 UTC 2015


On Fri, Sep 11, 2015 at 04:46:54PM -0400, David Lehman wrote:
> On Fri, 2015-09-11 at 16:35 -0400, Samantha N. Bueno wrote:
> > On Fri, Aug 21, 2015 at 12:56:41PM -0500, David Lehman wrote:
> > > On 08/21/2015 12:25 PM, Samantha N. Bueno wrote:
> > > >      #     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.
> > 
> > Now that you've mentioned this, I'm a bit wary to push this patch. It
> > doesn't seem to have as big an impact to the time lag as I thought,
> > so I
> > think I am going to leave this out of the set for now if that's all
> > right with you.
> 
> /me makes taunting chicken sounds...
> 
> You could at least keep this one:

Ha -- that's true, this one seems pretty safe. :)

Samantha
 
> diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> index 6505825..a0ae641 100644
> --- a/blivet/partitioning.py
> +++ b/blivet/partitioning.py
> @@ -539,7 +539,7 @@ def doPartitioning(storage):
>              storage.devicetree._removeDevice(partition,
> modparent=False, force=True)
>  
>      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
> 
> David
> 
> > 
> > Samantha
> >  
> > > 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 "
> > > > 
> > > 
> > > _______________________________________________
> > > anaconda-patches mailing list
> > > anaconda-patches at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches


More information about the anaconda-patches mailing list