[anaconda:master 0/2] RAID patches

Vratislav Podzimek vpodzime at redhat.com
Thu Jun 5 07:28:22 UTC 2014


On Mon, 2014-06-02 at 16:35 -0400, mulhern wrote:
> These will require the changes in the corresponding blivet RAID patches,
> so I plan to bump the version number.
> 
> The first is big, the second is much smaller. If I could actually profile
> I'ld be happier, but I believe that the speedup is noticeable to a human.
> 
> The only thing that bothers me is this sequence (which was not introduced
> by these patches):
> * For, say /, with LVM seleected, press Modify and choose a raid level that
> BTRFS and LVM are supposed to share, e.g., RAID0. Save the result.
> * Now, change device type to BTRFS, press modify again, and notice that
> the RAID level selection is RAID0. Press Cancel or Save.
> * Change device type back to LVM, press Modify, again, and the selected
> RAID level is now None, not RAID0.
> * Go back to BTRFS and notice that the selected RAID level is now 'single',
> BTRFS's default.
> 
> I think that the simple thing to do here is to make sure that, whenever the
> device type is changed to a new one, the RAID level is set back to the default
> for that device type. So, in step (2) above, the selected raid level should
> be the default for BTRFS, "single".
> 
> For step (2), whether Cancel or Save is pressed should matter. If Cancel,
> then, on step(3), the previously selected RAID level, RAID0, should be
> observed. If Save, then, on Step (3), the default RAID level for LVM, "None",
> should be observed.
> 
> Note that getting the raid level for an LVM volume group seems to mostly get
> None, and this may be the source of the problem. It will always be None,
> unless the volume group has exactly one physical volume, because that's what
> get_raid_level() does.
> 
> None of this was introduced by my changes here. But I think it would probably
> be best to fix it, and also make sure that it is not sign of a deeper problem.
> 
> Feedback appreciated, thanks.
> 
> mulhern (2):
>   RAID related changes for custom spoke.
>   Memoize the results for *RaidLevelsSupported() functions
> 
>  pyanaconda/storage_utils.py                        |  17 ---
>  pyanaconda/ui/gui/spokes/custom.glade              |   4 -
>  pyanaconda/ui/gui/spokes/custom.py                 |  93 ++++++------
>  .../ui/gui/spokes/lib/custom_storage_helpers.glade |   4 +
>  .../ui/gui/spokes/lib/custom_storage_helpers.py    | 163 ++++++++++++++++++---
>  5 files changed, 189 insertions(+), 92 deletions(-)
I don't like one thing on these patches -- adding new camelCase
functions/methods next to the underscore_named ones. I know these two
styles are mixed in many places, but I'd be really glad if we stuck
with underscore_based names for new functions/methods where possible.

Otherwise these look good to me apart from the one comment to PATCH 2/2.

-- 
Vratislav Podzimek

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




More information about the anaconda-patches mailing list