#2079 PR merged: Add new implementation to recreate all mounted Btrfs subvolumes

Labels: enhancement, fixed / solved / done

OliverO2 opened issue at 2019-03-11 19:38:

Pull Request Details:

  • Type: Bug Fix

  • Impact: High (in specific Btrfs use cases, ReaR recover would fail)

  • Reference to related issue (URL): https://github.com/rear/rear/issues/2067

  • How was this pull request tested? On Ubuntu 18.04.2 LTS with different combinations of Btrfs file systems and subvolume hierarchies.

  • Brief description of the changes in this pull request:

This PR aims to recreate all Btrfs file systems which have been mounted on the original system. It is a re-write of the original Btrfs subvolume code.

Although this code is simpler and focuses on mounted subvolumes only, its effects are intended to be entirely compatible with Btrfs scenarios addressed previously (e.g. using snapper). The assumptions are:

  1. All Btrfs subvolumes, which were actually mounted on the original system, are relevant for recovery and will finally be populated with backup data (or if not, at least their existence won't hurt).
  2. Non-mounted subvolumes are not relevant for recovery at all.

Of course, we can only know for sure when people would actually try this on different systems. The code is not enabled by default, but must be activated by putting a line like this into the local configuration:

BTRFS_SETUP_IMPLEMENTATION="new"

Hope this helps.

jsmeix commented at 2019-03-12 09:17:

@OliverO2
Wow! I am deeply impressed.

By plain looking at the code I think your new btrfs_subvolumes_setup()
will not correctly recreate SUSE's SLES 12 SP1 (and later) special
btrfs subvolumes setup with a snapper controlled default subvolume
because it does not call /usr/lib/snapper/installation-helper, see
https://github.com/rear/rear/blob/master/usr/share/rear/layout/prepare/GNU/Linux/130_include_mount_subvolumes_code.sh#L209
and subsequent lines what it does, in particular

... Configuring snapper for root filesystem ...
... creating snapshot of first root filesystem ...

In SLES 12 SP1 (and later) for the subvolume that is mounted at /
there is already a snapper snapshot (that is needed to be able to
roll back to that initial pristine installation state).

Because your new btrfs_subvolumes_setup() is a strictly separated add-on
that already works for Ubuntu 18.04 I would like to merge it right now as is
so that we have it in ReaR 2.5 and then we can improve things step by step
as needed.

I plan already one change:
I would like to be able to use both the old btrfs_subvolumes_setup()
and the new btrfs_subvolumes_setup() as neeeded for each $device.
This means both functions must have different names and a new
config variable BTRFS_SETUP_NEW_IMPLEMENTATION would be an
array that contains those $device values where the new function is used
and layout/prepare/GNU/Linux/130_include_mount_filesystem_code.sh
will be adapted to call the right btrfs_subvolumes_setup... e.g. like

if IsInArray "$device" ${BTRFS_SETUP_NEW_IMPLEMENTATION[@]} ; then
    btrfs_subvolumes_setup_new $device $mountpoint $mountopts
else
    btrfs_subvolumes_setup_old $device $mountpoint $mountopts
fi

Of course the names should be better...

@rear/contributors
if you do not object, I would like to merge it as is today afternoon.

OliverO2 commented at 2019-03-12 10:20:

@jsmeix Thanks for looking at it that quickly!

As I've already indicated, I won't be able to really spend time on ReaR stuff for the rest of this month. Nevertheless, feel free to add missing functionality in the mean time.

Regarding switching between old and new implementation:

  • Rename the function any way you like, the current function name override is just a provisional solution.
  • Ideally, in the end we'd have only one implementation and it would be functionally complete and as simple as possible.
  • Could you explain a bit more of your use case for per-device switching? When would one use that instead of using just one implementation for all devices?
  • If per-device switching is required, I'd ask you to include a setting which says "use the old/new implementation for all devices" without forcing the user to figure out which devices were actually relevant. That way we could use such a setting it in a network-wide configuration for differently configured systems.

Regarding snapper:

  • Unfortunately I could not figure out quickly what installation-helper does as I did not find a manual page and the source code does not provide sufficient information about its purpose at first glance.
  • My idea was that any snapper configuration should be restored from a backup and from then on everything should run smoothly without extra assistance.
  • If I'm wrong and snapper really needs extra assistance, the necessary code should be built into the new solution, of course. (Unfortunately, I cannot really help here, as I do not plan to use snapper due to complexity issues - I'm using a pretty simple internal Btrfs-only snapshot solution.)

Regarding nitpicking ;-):

  • I decided to stick with plain bash code, as it's easily testable without including any library functions. is_true seems geared towards user-controlled configuration variables with a possible variety of values. mount_required is just a tiny little internal thingy with strictly controlled values.

jsmeix commented at 2019-03-12 11:15:

Of course the goal is to have only one implementation
that is functionally complete and simple and straightforward.

My use case for per-device switching is to be able to still use
the old btrfs_subvolumes_setup for snapper controlled btrfs filesystems
(in prticular when btrfs is used as root filesystem on SLES12)
while the new btrfs_subvolumes_setup could be used for
additional btrfs filesystems that the user may have created manually.

With BTRFS_SETUP_NEW_IMPLEMENTATION=( 'yes' ) (or true or 1)
versus BTRFS_SETUP_NEW_IMPLEMENTATION=( 'no' ) (or false or 0)
the new btrfs_subvolumes_setup can be enforced for all btrfs filesystems
or forbidden to be used for any btrfs filesystem.
With BTRFS_SETUP_NEW_IMPLEMENTATION=() we could implement
some autodetection what code should be used and so on...
cf. AUTORESIZE_PARTITIONS or MODULES and similar variables.

Neither could I figure out with reasonable effort what that
installation-helper thingy does - and I had even inspected
its source code but there one gets lost in function calls...
...so that I just call it as a black box that "does the right thing".
I know what it does from a general higher point of view
but the details may change for each SLES service pack, cf.
https://github.com/rear/rear/issues/1368#issuecomment-302410707
so calling it as a black box that "does the right thing"
is the only right thing I can do in ReaR with reasonable effort
and I do not plan to understand the internals due to complexity issues ;-)

OliverO2 commented at 2019-03-12 12:17:

@jsmeix
Your catch-all setting for BTRFS_SETUP_NEW_IMPLEMENTATION looks good to me.

Now I see the issues you have ben facing with snapper. I've also looked at snapper/installation-helper.cc and noticed - apart from its lack of documentation - that its main() function doesn't even consistently return an exit code. I hope that these circumstances and different Btrfs setups will not lead to too much bloat and/or fragility in ReaR's Btrfs code. I still wonder what the point is of letting installation-helper create a snapshot on an empty subvolume. Weren't that supposed to be done after the backup has been restored? (And even then, that snapshot would never be the original "pristine state" since a recovery does not have access to the original Btrfs snapshot's contents.)

Finally, an idea for a future release:

Right now the new implementation does not require any change to existing layout analysis code (the stuff in usr/share/rear/layout/save/GNU/Linux/230_filesystem_layout.sh). Once we're confident to settle everything, that code could eventually shrink to the point of just putting out one line per mounted subvolume like

btrfssubvolume $device $subvolume_mountpoint $mount_options $btrfs_subvolume_path $is_default $uses_copy_on_write

where

  • is_default specifies whether the subvolume is the default subvolume (Y) or not (N),
  • uses_copy_on_write specifies whether the subvolume uses CoW (Y, the default) or not (N, set via chattr +C).

Doing so could further simplify the code and make disklayout.conf more concise.

[Export of Github issue for rear/rear.]