[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