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 root subvol exclusion mechanism #1931

Closed
phillxnet opened this issue May 25, 2018 · 1 comment · Fixed by #1935 or #2187
Closed

add root subvol exclusion mechanism #1931

phillxnet opened this issue May 25, 2018 · 1 comment · Fixed by #1935 or #2187
Assignees

Comments

@phillxnet
Copy link
Member

Currently we assume that a system pool subvol is represented by a top level directory in '/' corresponding to the subvols's name. This is not a universally safe assumption and most notably falls over in our current code / linux base for the root subvol. This appropriately, though arbitrarily, named subvol has, as a consequence of directly holding the root filesystem, the assumed /root path component within it. However this represents an arbitrary name collision between the /root directory and the subvol name 'root'. This subvol/dir name collision effectively covers up an issue specific to the pool containing the system and particularly that subvol mounted (via fstab) at /.

Essentially where as the 'home' subvol is mounted at /home, the root subvol is not mounted at '/root' but at '/' but the name collision of the already mounted root fs having, as standard, a /root directory within it's fs, conceals this anomaly specific to an already mounted root who's subvol name coincides with an existing top level directory within '/'.

As this anomaly is understood only to exist on the system pool (the one containing the linux '/' install) it is proposed that we add an exclusion mechanism for this and other 'sensitive' system subvols. This effectively removes the anomaly dependency and, in collaboration with the changes introduced in:
"improve subvol mount code robustness. Fixes #1923" for issue #1924
adds the ability to deal with the more common arrangement, for btrfs root installs, of having a root fs consisting of an arbitrarily named subvol (ie '@') which itself has subvols separating the various concerns of a root fs.

So by adding a subvol exclusion mechanism to the system pool (at the base level of current code) we are then able to process the more common arrangement of a root fs that is composed of a collection of sub-subvols. But this anomaly exclusion mechanism will remove, on our current linux base, the surfacing of the root subvol which is only successfully interpreted by way of the aforementioned '/root' dir name collision and the fact that, currently, a btrfs command executed on a subdirectory of a subvol returns the same result as if it were given the top directory of that subvol mount point, ie within our current code, when processing the subvol named 'root' we execute the following:

storageadmin/views/share_helpers - import_shares() we have, roughly via debug messaging:

---- Share name = root.
Updating pre-existing same pool db share entry.
Running command: /sbin/btrfs subvolume list /mnt2/system
Running command: /sbin/btrfs qgroup show /mnt2/system/root

Note in the last command instance we execute our qgroup show command assuming we are referencing the root subvol mount point when in fact we are referencing an arbitrary directory name that coincides with our subvol name that happens to exist within our intended subvol. But as the following 2 commands are equivalent we have our silent anomaly:

/sbin/btrfs qgroup show /mnt2/system/root
/sbin/btrfs qgroup show /mnt2/system

by way of comparison our home subvol is correctly referenced by it's path thus:

---- Share name = home.
Updating pre-existing same pool db share entry.
/sbin/btrfs subvolume list /mnt2/system
/sbin/btrfs qgroup show /mnt2/system/home

Where we see the '/mnt2/system/home' correctly corresponds to it's top level path.

So this issues proposal, and proof of concept pr (to follow), is to address the anomaly detailed above, that of the false assumption that for the actual fs root subvol there is a corresponding top level directory contained within, by not surfacing the 'root' subvol within the Web-UI by way of an exclusion mechanism that in turn allows the surfacing of system pool sub-subvols, which are inherently more informative / appropriate than the entire '/' anyway. This same subvol exclusion mechanism, proposed for and only tested on the os pool, also allows for the exclusion of other noisy/irrelevant, and potentially problematic for re-mount sub-subvols such as 'tmp', 'boot/grub2/x86_64-efi', and 'var'.

@phillxnet
Copy link
Member Author

I am currently working on this issue.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jun 8, 2018
Our current share/clone subvol surfacing mechanism assumes a top level
dir exists 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 rockstor#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 rockstor#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.
schakrava added a commit that referenced this issue Jun 13, 2018
…n_mechanism

add root subvol exclusion mechanism. Fixes #1931
phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Jun 24, 2020
Add boot/grub2/arm64-efi subvol exclusion to our existing system pool
subvol exclusions. As an appliance we attempt to hide the less
commonly applicable shares as they relate only to the system directly.

Summary:
- Builds on "add root subvol exclusion mechanism. Fixes rockstor#1931" pr.
- Includes both non boot to snap, and boot to snap patterns.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant