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

Add system-wide subvol exclusion mechanism (#2223) #2270

Conversation

phillxnet
Copy link
Member

Adds an additional hard coded subvolume exclusion list, akin to the existing system drive only one. All subvolume matches, by name, to list members are ignored. List pre-populated by ".beeshome" and it's possible boot-to-snapshot counterpart "@/.beeshome".

Includes a unit test mocking a typical data drive including the target ".beeshome" subvolume to prove the exclusion mechanisms function.

Fixes #2223
Ready for review.

Testing

The additional unit test was added to prove this exclusion by subvol name:

Failure case:

Pre exclusion mechanism.

./bin/test --settings=test-settings -v 3 -p test_btrfs*

======================================================================
FAIL: test_shares_info_systemwide_exclusion_datapool (fs.tests.test_btrfs.BTRFSTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/rockstor-dev/src/rockstor/fs/tests/test_btrfs.py", line 1290, in test_shares_info_systemwide_exclusion_datapool
    expected))
AssertionError: Failed data pool subvol exclusion:
returned {'netstat-config': '0/259', 'sftp-test': '0/257', 'rock-ons-root': '0/258', '.beeshome': '0/275'},
expected {'netstat-config': '0/259', 'sftp-test': '0/257', 'rock-ons-root': '0/258'}.


----------------------------------------------------------------------

Success case:

Post exclusion mechanism.

test_shares_info_systemwide_exclusion_datapool (fs.tests.test_btrfs.BTRFSTests) ... ok

All-test run was as expected:

./bin/test -v 2
...
Ran 213 tests in 35.364s

OK

Adds an additional hard coded subvolume exclusion list, akin to the
existing system drive only one. All subvolume matches, by name, to list
members are ignored. List pre-populated by ".beeshome" and it's possible
boot-to-snapshot counterpart "@/.beeshome".

Includes a unit test mocking a typical data drive including the target
".beeshome" subvolume to prove the exclusion mechanisms function.
@phillxnet phillxnet added the needs review Ideally by prior rockstor-core contributor label Jan 29, 2021
Copy link
Member

@FroggyFlox FroggyFlox left a comment

Choose a reason for hiding this comment

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

@phillxnet ,

I manually created a .beeshome subvolume in my data_pool:
btrfs subvolume create /mnt2/data_pool/.besshome

Pre-PR:

  • share is listed in Rockstor webUI

Post-PR:

  • share is not listed in Rockstor webUI anymore
  • logs show:
[29/Jan/2021 16:35:51] DEBUG [system.osi:466] --- Inheriting base_root_disk info ---
[29/Jan/2021 16:35:52] DEBUG [fs.btrfs:772] Skipping system-wide excluded subvol: name=(.beeshome).

I also could confirm that all 213 tests ran OK on my end.

Looks all good to me!

@phillxnet
Copy link
Member Author

@FroggyFlox Re:

I manually created a .beeshome subvolume in my data_pool:

Yes, In my tests I used Rockstor to create this subvol and confirmed that it was identically created, i.e. as a top level subvol.
As per @ubenmacki issue text forum link and the associated btrfs subvol list output.

Thanks for the review. Most helpful and good to have a double check, especially on critical path code changes such as this. But it's essentially a very simple exclusion "if" so again I think it's a safe addition in our release candidate phase.

@phillxnet phillxnet merged commit bee4b48 into rockstor:master Jan 30, 2021
@phillxnet phillxnet deleted the 2223_add_system-wide_subvolume_exclusion_mechanism branch January 30, 2021 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Ideally by prior rockstor-core contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add .beeshome to the list of ignored subvolumes
2 participants