[rhel6-branch][PATCH] Limit the maximum swap size to 10 % of disk space

Vratislav Podzimek vpodzime at redhat.com
Mon Jul 22 10:50:20 UTC 2013


On Fri, 2013-07-19 at 17:53 -0700, Brian C. Lane wrote:
> On Fri, Jul 19, 2013 at 10:40:07AM +0200, Vratislav Podzimek wrote:
> > On e.g. virtual machines with smalls disks and high amount of
> > memory it doesn't make sense to create large swap space leaving
> > only a little for the system and user data.
> > 
> > Also change a comment to a docstring and document parameters.
> > 
> > Resolves: rhbz#878907
> > Related: rhbz#879232
> 
> >              if self.recommended or self.hibernation:
> > -                (self.size, self.maxSizeMB) = iutil.swapSuggestion(hibernation=self.hibernation)
> > +                (self.size, self.maxSizeMB) = \
> > +                    iutil.swapSuggestion(hibernation=self.hibernation, disk_space=disk_space)
> 
> Breaking like that makes it less readable. I don't think we really need
> to stick with a strict 80 column max anymore. But if you do want to
> break the line, do it after  the first argument and align the others
> with the first:
> 
> (self.size, self.maxSizeMB) = iutil.swapSuggestion(hibernation=self.hibernation, \
>                                                    disk_space=disk_space)
Yeah, that looks better and the backslash is not needed that way. Fixing locally.

> 
> > diff --git a/storage/partitioning.py b/storage/partitioning.py
> > index e20a18f..16b5823 100644
> > --- a/storage/partitioning.py
> > +++ b/storage/partitioning.py
> > @@ -244,6 +244,20 @@ def doAutoPartition(anaconda):
> >      # sanity check the individual devices
> >      log.warning("not sanity checking devices because I don't know how yet")
> >  
> > +    # get a new swap suggestion, now that we know the total disk space we're
> > +    # gonna use
> > +    disk_space = 0
> > +    for disk in anaconda.id.storage.disks:
> > +        if not anaconda.id.storage.clearPartDisks \
> > +                or disk.name in anaconda.id.storage.clearPartDisks:
> > +            disk_space += disk.size
> > +
> > +    (min_size, max_size) = iutil.swapSuggestion(disk_space=disk_space)
> > +    for request in anaconda.id.storage.autoPartitionRequests:
> > +        if request.fstype == "swap":
> > +            request.size = min_size
> > +            request.maxSize= max_size
> > +
> 
> I think this needs to be inside a check for
> anaconda.id.storage.doAutoPart so that it doesn't clobber swap
> partition sizes specified by kickstarts.
You are right, I thought 'doAutoparition' is not called in case of
kickstart autopartitioning, but obviously it is. Good catch! Fixing
locally as well.

I've tested the updated version of this patch by going through as much
code paths as possible and everything seems to work as expected.
Anything else I should change or shall I push it?

Thanks,

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list