[blivet 0/8] Refactor abstract RAID concept out of mdraid.py

David Lehman dlehman at redhat.com
Thu Nov 21 17:39:36 UTC 2013


On Thu, 2013-11-21 at 11:59 -0500, mulhern wrote:
> These patches support a refactoring of mdraid.py into two files.
> The new file, raid.py contains a bunch of classes representing RAID
> levels.

I've only read over these briefly so far and they look good overall. I
do have a few initial comments.

> 
> The first four patches add some unit tests to catch any regressions
> that the refactoring might introduce. In some cases, some changes
> were made to the tested code, as seemed appropriate.
> 
> The next patch actually implements the RAID classes and their tests.

The only real complaint I have here is the continued use of integers as
the primary means of identification of RAID levels. I really want part
of this patch set to be a change to either use strings like "raid1" or
instances of the classes themselves as the sole means of specifying RAID
levels. In short, there's no need to play with integers at all anymore
as a means for specifying RAID levels. Calling code can be adapted as
needed, as can the MDRaidArrayDevice class.

> 
> The following patches use the RAID classes in mdraid.py and then use
> functions in mdraid.py to simplify some methods in devices.MDRaidArray.

As suggested above, I also want the MDRaidArrayDevice class to use the
new level classes. My preference would be that self.level be an instance
of one of the classes, but I could probably live with it being a string
that could be easily compared or resolved to the corresponding level
class.

> 
> The only class that communicate directly with the raid module is mdraid.
> Everything else goes through mdraid. Changes to mdraid were minimized as
> much as possible.

I would like to make it so that devicelibs.lvm and possibly
devicelibs.btrfs could also interface with devicelibs.raid using their
own values for chunk size. This isn't strictly required for the initial
implementation, but keep in mind the idea that we want those other
devicelibs modules to use the raid module as well.

Out of curiosity, have you tested this using anaconda yet?

David

> 
> mulhern (8):
>   Add some basic unit tests for MDRaidArrayDevice.
>   Make MDRaidArrayDevice initializer not except raid level of None.
>   Add some additional unit tests in mdraid_tests.py.
>   A few small tests for MDFactory class.
>   Implement a RAID class hierarchy.
>   Changed devicelibs.mdraid to make use of devicelibs.raid.
>   Use mdraid chunk size to set MDRaidArray's value.
>   Changed some methods in MDRaidArray to use mdraid methods.
> 
>  blivet/devicelibs/mdraid.py          | 201 ++++++++++--------
>  blivet/devicelibs/raid.py            | 330 ++++++++++++++++++++++++++++
>  blivet/devices.py                    | 121 +++++------
>  blivet/errors.py                     |   3 +
>  tests/devicefactory_test.py          |  59 +++++
>  tests/devicelibs_test/mdraid_test.py |  69 +++++-
>  tests/devicelibs_test/raid_test.py   | 106 +++++++++
>  tests/devices_test.py                | 402 +++++++++++++++++++++++++++++++++++
>  8 files changed, 1124 insertions(+), 167 deletions(-)
>  create mode 100644 blivet/devicelibs/raid.py
>  create mode 100644 tests/devicefactory_test.py
>  create mode 100644 tests/devicelibs_test/raid_test.py
>  create mode 100644 tests/devices_test.py
> 




More information about the anaconda-patches mailing list