Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Apr 19, 2023

Conversation

ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Apr 14, 2023

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): SUSE/doc-sle#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
Copy link

coveralls commented Apr 14, 2023

Coverage Status

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

@ancorgs ancorgs changed the base branch from master to SLE-15-SP5 April 17, 2023 08:27
@ancorgs ancorgs marked this pull request as ready for review April 18, 2023 08:27
Copy link
Member

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ancorgs ancorgs merged commit 4af0e59 into yast:SLE-15-SP5 Apr 19, 2023
@yast-bot
Copy link

✔️ Internal Jenkins job #752 successfully finished
✔️ Created IBS submit request #295273

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants