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

feat: Enable GFS2 support in blivet #418

Merged
merged 2 commits into from
Jan 27, 2024

Conversation

vojtechtrefny
Copy link
Collaborator

GFS2 is supported by blivet, but the support is disabled by default so we need to enable it.

Fixes: #417

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (98cd81f) 13.30% compared to head (030258d) 13.29%.

Files Patch % Lines
library/blivet.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #418      +/-   ##
==========================================
- Coverage   13.30%   13.29%   -0.01%     
==========================================
  Files           8        8              
  Lines        1781     1782       +1     
  Branches       79       79              
==========================================
  Hits          237      237              
- Misses       1544     1545       +1     
Flag Coverage Δ
sanity 16.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@richm
Copy link
Contributor

richm commented Jan 24, 2024

This breaks most of the tests. I've run tests on centos-9 and centos-8 and they fail with

TASK [linux-system-roles.storage : Manage the pools and volumes to match the specified state] ***
task path: /home/rmeggins/linux-system-roles/storage/tests/roles/linux-system-roles.storage/tasks/main-blivet.yml:84
...
MSG:

Failed to commit changes to disk: makes no sense to write a label when accepting default label

@vojtechtrefny
Copy link
Collaborator Author

This breaks most of the tests. I've run tests on centos-9 and centos-8 and they fail with

Sorry, should be fixed now.

@richm
Copy link
Contributor

richm commented Jan 25, 2024

What is the relationship between fs_label and fs_type gfs2?

GFS2 is supported by blivet, but the support is disabled by
default so we need to enable it.

Fixes: linux-system-roles#417
For existing devices we want to read the default value from the
device instead of trying to reset it to default/empty label when
the fs_label attribute is not set.

Related: linux-system-roles#417
@vojtechtrefny
Copy link
Collaborator Author

What is the relationship between fs_label and fs_type gfs2?

I have added comment explaining this in test-verify-volume-fs.yml. The problem is that blivet doesn't support specifying label when creating GFS2 so it has to be specified with the -t option passed manually in fs_create_options which means the verification will always fail -- label is set (we get that information from lsblk), but we didn't specify label in fs_label.

@richm richm merged commit f1a2198 into linux-system-roles:main Jan 27, 2024
17 of 19 checks passed
vojtechtrefny added a commit to vojtechtrefny/blivet that referenced this pull request Jun 3, 2024
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.

RFE: Support fs_type: gfs2
2 participants