----- Original Message -----
From: "Anne Mulhern" <amulhern(a)redhat.com>
To: "anaconda patch review" <anaconda-patches(a)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(a)redhat.com>
> To: anaconda-patches(a)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(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
- mulhern
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
- mulhern