It would be excellent if part of the install of Rockstor you were able to manually edit partition information so that the default shares are not created. It just looks messy in the UI - even if you could rename them after install that would be good.
root00 and home00
phillxnet
June 4, 2019, 12:13pm
2
@david_harrison Hello again.
Funny you should say that. As from stable channel release 3.9.2-27 (June 2018) via:https://github.com/rockstor/rockstor-core/issues/1931
rockstor:master ← phillxnet:1931_add_root_subvol_exclusion_mechanism
opened 06:35PM - 08 Jun 18 UTC
Our current share/clone subvol surfacing mechanism assumes a top level dir exist… s for a given pool (vol). This assumption is most notably flawed in the case of a system root ('/') as by definition it is the top level and has no '/' + 'dir-name' counterpart. However our current default system subol arrangement has '/' in a subvol named 'root' which coincidentally has the FHS counterpart subdir of '/root' and current btrfs commands return similarly for a subdirectory if that subdirectory is not also a subvol in it’s own right, which '/root' in our case is not. The mechanism chosen to initially address this issue is to exclude the Rockstor native re-mounting (in /mnt2) of known root subvolumes: ie: 'root', '@' and or subvols configured to be the default pool (vol) mount point (btrfs subvol get-default /) ie. '/@/.snapshots/1/snapshot' (ie a default snapper root config).
The subvol exclusion mechanism, as instantiated in this commit/pr, also serves to surface subvols of excluded subvols; ie '@/home' where @ is itself a subvol: a more common arrangement in modern root on btrfs configurations. This is due to our current share/clone single depth subvol consideration.
A wrinkle in subvol path exclusion practice is that when the '/' subvolume does not share a direct common subvol as parent, such as in a default snapper root config boot to snapshot arrangement, the path expressed for '/home' is then eg '@/home' as opposed to our previously expected and relative to shared direct parent (@) 'home'. This is accounted for, in this commit's / pr's first pass treatment, by simply doubling up relevant exclusions and hard wiring a blind stripping of leading '@/' chars: a sub optimal but proven functional (by included unit tests) fix. It is suggested that this approach be iteratively
improvement upon.
Summary:
1. Establish a named root subvol exclusion list.
2. Add a function to identify the default subvolid for the pool hosting ‘/’.
3. Reference the above 2 mechanisms to 'skip' consideration of associated subvols, by path and or by id respectively by way of (1.) and (2.) above (debug logged).
4. Refactor shares_info() (creating get_property() and snapshot_idmap()) to help remove code duplication and improve code clarity clone identification logic, and testability.
5. Add a redirect to '/' in cases where we are examining an excluded subvol which has no /mnt2 counterpart.
6. Brute force conversion to currently recognized relative paths, ie strip @/ chars.
7. Normalise use of toggle_path_rw() for immutable flag manipulation.
8. Remove redundant/unused function, See Note * for details.
9. Removed an outer scope variable name collision.
10. Account for upstream parent-child column inversion - group_is_assigned().
Note that 10 in above is an unrelated change to issue #1931 but is trivial in nature: upstream issue reference included in code comments.
Note *
Removed subvol_list_helper() and sole remaining user. Introduced 4 years 8 months ago in #105 to mitigate an rc=19 subvol list issue. This helper was not universally employed and we have had no reports of it's recent occurrence; hence removal. Comment left by prior sole use location, and related remaining rc = 19 exception clause.
Fixes #1931
Please see referenced issue text for differently worded exposition of the referenced root subvol anomaly re subvol path and FHS dir name collision.
@schakrava Ready for review.
Testing:
The following elements (indicated by their names) were created (and later deleted), via the Web-UI, and observed to be persistent post page refresh ('refresh-share-state' / 'refresh-snapshot-state') and post reboot ('bootstrap') on current (legacy) and proposed root subvol arrangements; proposed subvol arrangements considered installs with and without snapper root config.
system pool:
sys-share
sys-share-snap
sys-share-snap-rw - clone from this - clone-from-snap
sys-share-snap-rw-visible
clone-sys-share
data pool, ie rock-pool:
rock-share
rock-share-snap
rock-share-snap-rw - clone from this - clone-from-rock-share-snap
rock-share-snap-rw-visible
A full data-pool share replication cycle ( > 5 rep tasks) was also tested successfully between a legacy root arrangement and a proposed system subvol arrangement (snapper root config active).
Additional unit test output:
...
test_parse_snap_details (fs.tests.test_btrfs.BTRFSTests) ... ok
...
test_snapshot_idmap_home_rollback (fs.tests.test_btrfs.BTRFSTests) ... ok
test_snapshot_idmap_home_rollback_snap (fs.tests.test_btrfs.BTRFSTests) ... ok
test_snapshot_idmap_mid_replication (fs.tests.test_btrfs.BTRFSTests) ... ok
test_snapshot_idmap_no_snaps (fs.tests.test_btrfs.BTRFSTests) ... ok
test_snapshot_idmap_snapper_root (fs.tests.test_btrfs.BTRFSTests) ... ok
...
test_get_property_all (fs.tests.test_btrfs.BTRFSTests) ... ok
test_get_property_compression (fs.tests.test_btrfs.BTRFSTests) ... ok
test_get_property_ro (fs.tests.test_btrfs.BTRFSTests) ... ok
...
test_shares_info_legacy_system_pool_fresh (fs.tests.test_btrfs.BTRFSTests) ... ok
test_shares_info_legacy_system_pool_used (fs.tests.test_btrfs.BTRFSTests) ... ok
test_shares_info_system_pool_post_btrfs_subvol_list_path_changes (fs.tests.test_btrfs.BTRFSTests) ... ok
test_shares_info_system_pool_used (fs.tests.test_btrfs.BTRFSTests) ... ok
...
test_get_snap_2 (fs.tests.test_btrfs.BTRFSTests) ... ok
test_get_snap_legacy (fs.tests.test_btrfs.BTRFSTests) ... ok
Caveats:
We have some additional hard wiring added by this pr ie within some references to 'system': these are to be re-visited / addressed in related later pr's sharing the same project level goal. I will endeavour to reference this pr upon their submission. These references should not affect our existing installs function.
we don’t actually show the ‘root ’ subvol. As it turned out to be a subtle Rockstor bug which was discovered as part of our preparations for the move to an openSUSE base. But the ‘home ’ subvol is one I think we should keep and present given it’s ubiquitous use in our linux base.
However you will note that here I mention ‘root’ and ‘home’ not ‘root00’ and ‘home00’ This later pair are as a result of our upstream CentOS installer which, under certain circumstances, adds the ‘unattractive’ double zero ending for reasons as yet undetermined. It mostly doesn’t but of course when this does happen our current exclusion of ‘root’ doesn’t cover it; and given this is a ‘feature’ in an installer we are moving away from I think it is unlikely to get fixed.
But to your point re share renaming:
Definitely and we have had the following related issues on this:
https://github.com/rockstor/rockstor-core/issues/865 https://github.com/rockstor/rockstor-core/issues/864
open for some time. But the good news is that we are almost there on being able to support share and pool renaming; it would then be just the UI side of the code to do. I have done some testing locally with code that is ear marked for 3.9.2-49 and it’s looking like we should be able to support pool renaming in the not too distant future. We do have more pressing issues afoot but it’s definitely something we have been working towards in the background.
Hope that helps and thanks for sharing your suggestions and observations.
