[PATCH 5/5] Round filesystem target size to whole resize tool units. (#1163410)

Anne Mulhern amulhern at redhat.com
Tue Nov 18 12:58:56 UTC 2014





----- Original Message -----
> From: "Anne Mulhern" <amulhern at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Monday, November 17, 2014 6:42:11 PM
> Subject: Re: [PATCH 5/5] Round filesystem target size to whole resize tool	units.	(#1163410)
> 
> 
> 
> 
> 
> ----- Original Message -----
> > From: "David Lehman" <dlehman at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Monday, November 17, 2014 2:36:08 PM
> > Subject: [PATCH 5/5] Round filesystem target size to whole resize tool
> > units.	(#1163410)
> > 
> > Using int meant we always rounded down, which wasn't always correct.
> > ---
> >  blivet/formats/fs.py | 40 +++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 37 insertions(+), 3 deletions(-)
> > 
> > diff --git a/blivet/formats/fs.py b/blivet/formats/fs.py
> > index 1139547..cfb1b8e 100644
> > --- a/blivet/formats/fs.py
> > +++ b/blivet/formats/fs.py
> > @@ -36,7 +36,7 @@ from ..flags import flags
> >  from parted import fileSystemType
> >  from ..storage_log import log_exception_info, log_method_call
> >  from .. import arch
> > -from ..size import Size
> > +from ..size import Size, ROUND_UP, ROUND_DOWN
> >  from ..i18n import _, N_
> >  from .. import udev
> >  
> > @@ -78,6 +78,7 @@ class FS(DeviceFormat):
> >      _defaultCheckOptions = []
> >      _defaultInfoOptions = []
> >      _existingSizeFields = []
> > +    _resizefsUnit = None
> >      _fsProfileSpecifier = None           # mkfs option specifying
> >      fsprofile
> >  
> >      def __init__(self, **kwargs):
> > @@ -469,6 +470,27 @@ class FS(DeviceFormat):
> >              self.targetSize = self.minSize
> >              log.info("Minimum size changed, setting targetSize on %s to
> >              %s",
> >                       self.device, self.targetSize)
> > +
> > +        # Bump target size to nearest whole number of the resize tool's
> > units.
> > +        shrink = self.targetSize < self.currentSize
> > +        if shrink:
> > +            rounding = ROUND_UP
> > +        else:
> > +            rounding = ROUND_DOWN
> 
> Is this rounding direction choice meant to increase the likelihood
> that the value will be within bounds of min size and max size,
> or is there some other reason?
> 
> > +
> > +        rounded = self.targetSize.roundToNearest(self._resizefsUnit,
> > +                                                 rounding=rounding)
> > +
> > +        # Now, just in case, make sure that rounding doesn't change the
> > +        # direction of the resize. If it does, return without resizing.
> > +        if ((shrink and rounded > self.currentSize) or
> > +            (not shrink and rounded < self.currentSize)):
> > +            log.info("rounding target size to %s obviated resize of
> > filesystem "
> > +                     "on %s", self._resizefsUnit, self.device)
> > +            return
> > +        else:
> > +            self.targetSize = rounded
> > +
> 
> I would feel better if this adjustment occurred in the Action phase,
> not in the execution phase, so that other sizes could be adjusted
> appropriately
> in that part. I realize that may be impossible.
> 
> >          try:
> >              ret = util.run_program([self.resizefsProg] + self.resizeArgs)
> >          except OSError as e:
> > @@ -910,6 +932,7 @@ class Ext2FS(FS):
> >      _infofs = "dumpe2fs"
> >      _defaultInfoOptions = ["-h"]
> >      _existingSizeFields = ["Block count:", "Block size:"]
> > +    _resizefsUnit = "MiB"
> >      _fsProfileSpecifier = "-T"
> >      partedSystem = fileSystemType["ext2"]
> >  
> > @@ -983,8 +1006,13 @@ class Ext2FS(FS):
> >                  size = _size
> >                  orig_size = size
> >                  log.debug("size=%s, current=%s", size, self.currentSize)
> > +                # add some padding
> >                  size = min(size * Decimal('1.1'),
> > -                           size + Size("500 MiB"),
> > +                           size + Size("500 MiB"))
> 
> Why can not the padding be skipped here? It seems like the size should
> be correct and pre-rounded.
> 
> > +                # make sure that the padded and rounded min size is not
> > larger
> > +                # than the current size
> > +                size = min(size.roundToNearest(self._resizefsUnit,
> > +                                               rounding=ROUND_UP),
> >                             self.currentSize)
> >                  if orig_size < size:
> >                      log.debug("padding min size from %s up to %s",
> >                      orig_size, size)
> > @@ -1355,6 +1383,7 @@ class NTFS(FS):
> >      _infofs = "ntfsinfo"
> >      _defaultInfoOptions = ["-m"]
> >      _existingSizeFields = ["Cluster Size:", "Volume Size in Clusters:"]
> > +    _resizefsUnit = "B"
> >      partedSystem = fileSystemType["ntfs"]
> >  
> >      def _fsckFailed(self, rc):
> > @@ -1388,8 +1417,13 @@ class NTFS(FS):
> >                  log.warning("Unable to discover minimum size of filesystem
> >                  "
> >                              "on %s", self.device)
> >              else:
> > +                # add some padding to the min size
> >                  size = min(minSize * Decimal('1.1'),
> > -                           minSize + Size("500 MiB"),
> > +                           minSize + Size("500 MiB"))
> > +                # make sure the padded and rounded min size is not larger
> > than
> > +                # the current size
> > +                size = min(size.roundToNearest(self._resizefsUnit,
> > +
> 
> And same question here as well.
> 
>                                            rounding=ROUND_UP),
> >                             self.currentSize)

It also puzzles me that the min size is actually the minimum of
currentSize and the (padded) value obtained from the filesystem.

Why isn't that also true for calculating the minimum size for formats
where minSize is just obtained from _minSize? So, something like:

def minSize(self):
    if self.exists:
        return min(self._minSize.roundToNearest(self._resizefsUnit, rounding=ROUND_UP), self.currentSize)
    else:
        return self._minSize.roundToNearest(self._resizefsUnit, rounding=ROUND_UP)


> >                  if minSize < size:
> >                      log.debug("padding min size from %s up to %s",
> >                      minSize,
> >                      size)
> > --
> > 1.9.3
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> 
> - mulhern
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 

- mulhern


More information about the anaconda-patches mailing list