yast / yast-storage-ng

Rewrite of https://github.com/yast/yast-storage
http://yast.github.io/
GNU General Public License v2.0
14 stars 20 forks source link

Partitioner: skip MD chunk size when not applicable (bsc#1205172) #1331

Closed ancorgs closed 1 year ago

ancorgs commented 1 year ago

This PR addresses two separate problems related to MD RAIDs.

Problem 1

The switch from libstorage to libstorage-ng introduced changes in the identifiers used internally to represent types of Raid parity (eg. parity_first became first, o2 became offset_2`, etc.).

That change reached AutoYaST. So the possible values of the property <parity_algorithm> in the AutoYaST profile are slightly different between SLE-12 and SLE-15 (storage-ng). That was only detected recently while checking the source code. It has not been reported by any affected user.

Solution to problem 1

Implement backwards compatibility with SLE-12 profiles by recognizing legacy values for <parity_algorithm> (eg. parity_first or o2).

I also created the corresponding pull request for the official AutoYaST documentation, so the new identifiers are used there (with a mention to the legacy ones): https://github.com/SUSE/doc-sle/pull/1485

Testing of solution to problem 1

Added new unit tests to ensure both old and new representations of the parity are correctly processed by AutoYaST.

Previously existing tests already proved that exporting (cloning) uses the new values.

Problem 2

The Chunk Size options makes no sense for RAID1 devices. Still, YaST historically presents this dialog as second step in the process of creating such a device.

raid1

Previous versions of mdadm just ignored the corresponding --chunk= argument when specified during the creation of a RAID1 device. So the information managed by YaST during the whole Partitioner execution was simply pointless.

Starting with mdadm version 4.2 (which is included in both openSUSE Tumbleweed and SLE-15-SP5), the --chunk= argument is not ignored anymore if used together with --level=1. Instead, it now throws an error.

Solution for problem 2

Don't display the dialog and don't set the internal value of Y2Storage:::Md.chunk_size for RAID1 devices. With that, YaST will make no assumptions about that pointless attribute and will not try to set its value in the corresponding libstorage-ng call to mdadm.

Testing solution for problem 2

Manually tested. The dialog is indeed skipped and using the option "Device -> Show Details" displays no value for new RAID1 devices.

More references

https://trello.com/c/jTPYqFTU/3209-2-ostumbleweed-p2-1205172-staging-mdadm-42-fails-installation-on-raid1-with-yast

coveralls commented 1 year ago

Coverage Status

Coverage: 97.75% (-0.007%) from 97.757% when pulling 51beeedbea70f40045318635f3d64b611573b91d on ancorgs:md_options_sp5 into 1e893370023ae8a940e226156d61fc5255adc18d on yast:SLE-15-SP5.

yast-bot commented 1 year ago

:heavy_check_mark: Internal Jenkins job #752 successfully finished :heavy_check_mark: Created IBS submit request #295273