[blivet:master 00/20] Patches related to _resizefsUnit

mulhern amulhern at redhat.com
Tue Dec 23 23:42:13 UTC 2014

This turns out to be actually quite a lot of patches about Sizes.

The first 8 patches are minor fixes for little things that I ran into
while worrying about Sizes.

With "Make _Prefix entries..." I'm starting to try to set things up so that
it is possible to have some kind of values that represent units to pass
to convertTo() and roundToNearest(). By using string specifiers of units
as arguments to these methods, we end up using _parseUnits() to find out
the units. This is kind of a mistake, because it is a method that is
really designed for parsing human input size strings, so it is
unnecessarily heavyweight for a context where the units is precisely known
and specified explicitly in the program.

"Do not even pretend..." patch is a bit of a one off. I really think we can
get rid of that code, about the only thing it is likely ever to do for us
is halt some install sometime (if it is ever run).

Then come three patches about resizeArgs() and _resizefsUnit. The last
two of these may be somewhat controversial. Here's my take on things:

Without the "Use _resizefsUnit in resizeArgs()..." patch a developer has to
have or somehow obtain some knowledge about the connection between
the unit in resizeArgs() and the one in getMinSize(). Let's suppose they
change _resizefsUnit (because they want to change how getMinSize() behaves).
They can:
(1) Fail to realize that they need to change resizeArgs() as well. If they
are lucky, and they changed _resizefsUnit to a multiple of the units in
resizeArgs() there may or not appear to be a problem. If they are unlucky
and they changd _resizefsUnit to something other than a multiple, then
the result of the resize operation will most likely be wrong. That would
be a bug.
(2) Realize that they need to change the units of resizeArgs() correspondingly.
If so, they ought to have the good sense to look at the containing
expression, and infer that they need to change the format string as well.

With the "Use _resizefsUnit in resizeArgs()..." patch a developer does not
need to know any secret lore. Being conscientious, they do a search for the
string _resizefsUnit and find it in resizeArgs(). They know there is a
connection and only possibility (2) from above is available.

My patch eliminates the possibility of bugs arising from situation (1).
I realize it relies on the programmer being able to realize that if the
units of the size change, so should the format string, but I _think_
that's not hard.

Finally, there are two patches about Size.__new__. The motivation
for these two is to avoid running the _parseSpec() method from
Size.__new__() as _parseSpec() is really designed to handle
user-input strings. It is far too general for literals specified in the
program and is correspondingly busy. The locale stuff, the regular
expression matching, iterating through a list dynamically calculating
string specs and comparing them with the string given is huge overkill
when the unit required is already known at compile time.

This approach is less invasive than some others and does not _require_
any change to user code. I think that ideally user input parsing would
be handled in anaconda, since that is not really blivet's job at all, but
I think that would be in the far future, so might as well do this for now.

This approach speeds construction of Size() object up by a factor of 3 or 4,
suprisingly little. But it seems like the more correct thing. If everybody
accepts it, I'll also convert the Size literals in tests subdirectory to
use tuples.

mulhern (20):
  Get rid of unnecessary use of long.
  Make possiblePhysicalExtents() a bit more direct.
  Do not compare the same two values twice.
  Do not calculate Sizes unless they are needed.
  Avoid using Size constant in FileDevice._create().
  Eliminate redundant test.
  Comment _prefixTestHelper() and eliminate some redundancies.
  Hoist _BINARY_FACTOR * min_value calculation out of loop.
  Make _Prefix entries named constants.
  Add unitStr() method.
  Make _parseUnits() return a unit constant, rather than a number.
  Get whole unit tuple in loop when searching for correct units.
  Do not even pretend that ReiserFS is resizable.
  Change convertTo() and roundToNearest() so each takes a units
  Use convertTo in humanReadable().
  Do not supply a default implementation for the resizeArgs() method.
  Use _resizefsUnit in resizeArgs() method implementations.
  Add _resizefsUnit to TmpFS and use in appropriate places.
  Add a new way of specifying Sizes().
  Use tuples to construct Sizes() wherever units are known.

 blivet/devicefactory.py           |   6 +-
 blivet/devicelibs/btrfs.py        |   4 +-
 blivet/devicelibs/crypto.py       |   4 +-
 blivet/devicelibs/lvm.py          |  45 +++++-----
 blivet/devicelibs/mdraid.py       |  10 +--
 blivet/devicelibs/swap.py         |  15 ++--
 blivet/devices/file.py            |   8 +-
 blivet/devices/lvm.py             |  16 ++--
 blivet/devices/partition.py       |  12 +--
 blivet/formats/biosboot.py        |   6 +-
 blivet/formats/fs.py              |  77 ++++++++---------
 blivet/formats/prepboot.py        |   6 +-
 blivet/formats/swap.py            |   4 +-
 blivet/partitioning.py            |   6 +-
 blivet/platform.py                |  22 ++---
 blivet/size.py                    | 168 ++++++++++++++++++++------------------
 blivet/udev.py                    |  12 +--
 blivet/util.py                    |   6 +-
 tests/devicelibs_test/lvm_test.py |   5 --
 tests/size_test.py                |  96 +++++++++++-----------
 tests/storagetestcase.py          |   4 +-
 21 files changed, 261 insertions(+), 271 deletions(-)


More information about the anaconda-patches mailing list