Make both btrfs_subvolumes_setup implementations available (follow up of issue 2079) #2080

jsmeix · 2019-03-12T14:02:09Z

Type: Enhancement
Impact: Low
Reference to related issue (URL):
Add new implementation to recreate all mounted Btrfs subvolumes #2079 (comment)
How was this pull request tested?
Not yet tested at all by me.
Brief description of the changes in this pull request:

Now there are two functions that implement btrfs subvolumes setup:

The old btrfs_subvolumes_setup is now btrfs_subvolumes_setup_SLES

The new one from #2079
is now btrfs_subvolumes_setup_generic

There are two new user config array variables
BTRFS_SUBVOLUME_SLES_SETUP
and
BTRFS_SUBVOLUME_GENERIC_SETUP
where the suer can - if needed - specify how his btrfs subvolumes
should be set up.

When neither BTRFS_SUBVOLUME_SLES_SETUP
nor BTRFS_SUBVOLUME_GENERIC_SETUP
is set the old behaviour is kept (now via btrfs_subvolumes_setup_SLES).

When both BTRFS_SUBVOLUME_SLES_SETUP
and BTRFS_SUBVOLUME_GENERIC_SETUP are set to a false value
no btrfs subvolumes setup is done.

When BTRFS_SUBVOLUME_GENERIC_SETUP is set to a true value
or when BTRFS_SUBVOLUME_SLES_SETUP is set to a false value
btrfs subvolumes setup is done via btrfs_subvolumes_setup_generic

When BTRFS_SUBVOLUME_GENERIC_SETUP is set to a false value
or when BTRFS_SUBVOLUME_SLES_SETUP is set to a true value
btrfs subvolumes setup is done via btrfs_subvolumes_setup_SLES

When the BTRFS_SUBVOLUME_GENERIC_SETUP array contains
devices those are set up via btrfs_subvolumes_setup_generic

When the BTRFS_SUBVOLUME_SLES_SETUP array contains
devices those are set up via btrfs_subvolumes_setup_SLES

… of issue_2079)

jsmeix · 2019-03-12T14:04:06Z

@rear/contributors @OliverO2
I did the pull request early so that you can have an early look at the code
and provide comments what you think about it.

Currently this is work in progress...

…RFS_SUBVOLUME_SLES_SETUP for improved precedence handling

…atedly

OliverO2

Some ideas for improvements, but should work without those.

usr/share/rear/layout/prepare/GNU/Linux/130_include_mount_filesystem_code.sh

OliverO2 · 2019-03-12T14:31:10Z

usr/share/rear/layout/prepare/GNU/Linux/130_include_mount_filesystem_code.sh

+                # Final fallback to be backward compatible (btrfs_subvolumes_setup_SLES is the old way):
+                LogPrint "Doing SLES-like btrfs subvolumes setup for $device on $mountpoint (no match in BTRFS_SUBVOLUME_GENERIC_SETUP or BTRFS_SUBVOLUME_SLES_SETUP)"
+                btrfs_subvolumes_setup_SLES $device $mountpoint $mountopts
+            done
            ;;


For me, it looks like this would work and support lots of possible combinations. (I'd probably just go for a simpler variant with less configurability.)

I prefer to provide the user as much freedom to specify what ReaR should do
as possible to be implemented with reasonable effort.

E.g. one can specify

BTRFS_SUBVOLUME_SLES_SETUP=( 'false' '/dev/sda3' '/dev/sdc5' )

to avoid btrfs_subvolumes_setup_SLES for all devices
except '/dev/sda3' and '/dev/sdc5' which means
btrfs_subvolumes_setup_generic is done on all other devices.

Let's try out and see how things behave in practice for now.
If it gets too complicated I can simplify it as long as
it is not yet officially documented in default.conf.

usr/share/rear/layout/prepare/GNU/Linux/130_include_mount_subvolumes_code.sh

usr/share/rear/layout/prepare/GNU/Linux/131_include_new_btrfs_subvolumes_code.sh

OliverO2 · 2019-03-12T14:40:58Z

usr/share/rear/layout/prepare/GNU/Linux/130_include_mount_subvolumes_code.sh

@@ -25,7 +28,7 @@ btrfs_subvolumes_setup() {
    mountopts="$3 $4"
    # Empty device or mountpoint may indicate an error. In this case be verbose and inform the user:
    if test -z "$device" -o -z "$mountpoint" ; then
-        LogPrint "Empty device='$device' or mountpoint='$mountpoint' may indicate an error, skipping btrfs_subvolumes_setup( $@ )."
+        LogPrintError "Empty device='$device' or mountpoint='$mountpoint' may indicate an error, skipping btrfs_subvolumes_setup( $@ )."
        Log "Return 0 from btrfs_subvolumes_setup( $@ )"


It is my understanding that file system creation code as in 130_include_filesystem_code.sh makes ReaR abort if any error occurs (and so does the new Btrfs setup implementation). Since subvolumes are file system-like objects (Btrfs doubles as a volume manager in this respect), it can be risky to continue with an incomplete setup: For example, the wrong disk partition may fill up with files restored from a backup.

I do not want to change with this pull request how the old code behaves.
I don't know if I can error out here without causing subtle regressions.
I wished I had better descibed in my comment why I don't error out here.

…tup needed

jsmeix · 2019-03-12T16:03:32Z

Tested it a bit on SLES12:

"rear recover" still works with the defaults (which is using the old code).

"rear recover" fails on SLES12 with its btrfs structure when the new code is used,
then the diskrestore.sh script fails, I have the "rear recover" log file
but will have a closer look tomorrow...

…y callee). Better descriptive names of scripts and renumbered scripts (avoid same numbers which looks sloppy).

…when needed.

jsmeix · 2019-03-13T11:12:11Z

My latest commit
c0b0ddd
intends to implement some automatism that ensures
during "rear recover" btrfs_subvolumes_setup_SLES()
gets called when needed so that for SLES things should even work
when the user has specified BTRFS_SUBVOLUME_GENERIC_SETUP="true".

OliverO2 · 2019-03-13T11:17:54Z

Sound reasonable to include such an automatism for now. I'm curious about discovering why the diskrestore.sh script created by the generic implementation failed on SLES12, though.

jsmeix · 2019-03-13T11:23:39Z

@OliverO2
I need to test my latest geratest automatisms first...
...and if it works it could (hopefully) be some kind of template
for you how you could later add an automatism to call
btrfs_subvolumes_setup_generic() as needed for your case.

OliverO2 · 2019-03-13T11:30:52Z

Take your time. The reason why I am curious is that the generic code should create a layout for any setup (which would include SLES12). It might not be the "right" or "ideal" layout for now, but at least diskrestore.sh should not abort.

jsmeix · 2019-03-13T13:10:59Z

Testing helps ( unsurprisingly ;-)
My automatism does not yet work - I know why - will fix it...

…ded.

jsmeix · 2019-03-13T14:18:44Z

Now things work for me so far so that I like to merge it right now
to have it in ReaR master code where all can test it more easily.

jsmeix · 2019-03-13T14:37:05Z

@OliverO2
the "rear -D recover" log and disklayout.conf and diskrestore.sh
from my failed attempt

rear-recover-fail.log

diskrestore-sh.txt

disklayout-conf.txt

In "rear -D recover" log there is

++ source /var/lib/rear/layout/diskrestore.sh
...
Create subvolume '/mnt/local/@/.snapshots/1/snapshot'
+++ btrfs subvolume set-default /mnt/local/@/.snapshots/1/snapshot
btrfs subvolume set-default: too few arguments
usage: btrfs subvolume set-default <subvolid> <path>

    Set the default subvolume of a filesystem

OliverO2 · 2019-03-13T14:42:56Z

Ah, I see: Your version of the Btrfs tool suite requires the subvolid parameter for the subvolume set-default command. Newer versions seem to be clever enough to figure out that /mnt/local/@/.snapshots/1/snapshot already uniquely specifies a subvolume. I could easily fix that (by using another invocation to retrieve the subvolid first) but probably no earlier than somewhere in April...

jsmeix · 2019-03-13T14:58:35Z

On my SLES15-like openSUSE Leap 15.0 system
"man btrfs subvolume" shows

set-default [<subvolume>|<id> <path>]

Set the default subvolume for the (mounted) filesystem.
...
There are two ways how to specify the subvolume,
by <id> or by the <subvolume> path.
The id can be obtained from
btrfs subvolume list
btrfs subvolume show
or
btrfs inspect-internal rootid

which belongs to btrfsprogs version 4.15.

On my SLES12 system I have btrfsprogs-4.5.3
where "man btrfs subvolume" shows

set-default <id> <path>

Set the subvolume of the filesystem <path>
which is mounted as default.
The subvolume is identified by <id>,
which is returned by the
subvolume list
command.

so for SLES12 the subvolume's "name" is not possible.
On SLES12 one must use the (numerical) ID,
cf. the btrfs subvolume set-default code in the (now renamed)
layout/prepare/GNU/Linux/136_include_btrfs_subvolumes_SLES_code.sh

…e_GNU_Linux_13X_include_scripts_that_were_renamed_via_issue_2080 Fixed comments and messages about the layout/prepare/GNU/Linux/13X_include_... scripts that had been renamed via #2080

In layout/prepare/GNU/Linux/135_include_btrfs_subvolumes_generic_code.sh fixed 'btrfs subvolume set-default' command for older versions of 'btrfsprogs' where that command requires both arguments 'subvolid' and 'path', see #2080 (comment)

Make both btrfs_subvolumes_setup implementations available (follow up…

6677717

… of issue_2079)

jsmeix added the enhancement Adaptions and new features label Mar 12, 2019

jsmeix added this to the ReaR v2.5 milestone Mar 12, 2019

jsmeix self-assigned this Mar 12, 2019

jsmeix added 2 commits March 12, 2019 15:37

Better ordering of the tests for BTRFS_SUBVOLUME_GENERIC_SETUP and BT…

75837f7

…RFS_SUBVOLUME_SLES_SETUP for improved precedence handling

Using double quotes as in "${arr[@]}" to get all array elements separ…

4bd1d59

…atedly

OliverO2 approved these changes Mar 12, 2019

View reviewed changes

Adapted info_message about SLES12-SP1 (and later) btrfs subvolumes se…

711c82e

…tup needed

jsmeix added 2 commits March 13, 2019 10:39

Do not call btrfs_subvolumes_setup_generic with mountopts (not used b…

cd2bd2e

…y callee). Better descriptive names of scripts and renumbered scripts (avoid same numbers which looks sloppy).

Automatically append devices to the BTRFS_SUBVOLUME_SLES_SETUP array …

c0b0ddd

…when needed.

Fixed automatic append devices to BTRFS_SUBVOLUME_SLES_SETUP when nee…

f4a7d22

…ded.

jsmeix merged commit b144e90 into rear:master Mar 13, 2019

jsmeix deleted the make_both_btrfs_subvolumes_setup_implementations_available_follow_up_of_issue_2079 branch March 13, 2019 14:26

jsmeix added the fixed / solved / done label Mar 13, 2019

jsmeix mentioned this pull request Mar 15, 2019

Fixed comments and messages about '13X_include_...' scripts that had been renamed via issue 2080 #2084

Merged

This was referenced Mar 15, 2019

Restoring Btrfs file system fails if a snapshot has been used for rollback #2067

Closed

Enhance btrfs_subvolumes_setup_generic() to make it work everywhere. #2085

Closed

OliverO2 mentioned this pull request Apr 16, 2019

btrfs subvolumes generic: fix 'subvolume set-default' for older versions #2119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make both btrfs_subvolumes_setup implementations available (follow up of issue 2079) #2080

Make both btrfs_subvolumes_setup implementations available (follow up of issue 2079) #2080

jsmeix commented Mar 12, 2019

jsmeix commented Mar 12, 2019

OliverO2 left a comment

OliverO2 Mar 12, 2019

jsmeix Mar 13, 2019

OliverO2 Mar 12, 2019

jsmeix Mar 13, 2019

jsmeix commented Mar 12, 2019

jsmeix commented Mar 13, 2019

OliverO2 commented Mar 13, 2019

jsmeix commented Mar 13, 2019

OliverO2 commented Mar 13, 2019

jsmeix commented Mar 13, 2019

jsmeix commented Mar 13, 2019

jsmeix commented Mar 13, 2019

OliverO2 commented Mar 13, 2019

jsmeix commented Mar 13, 2019 •

edited

Make both btrfs_subvolumes_setup implementations available (follow up of issue 2079) #2080

Make both btrfs_subvolumes_setup implementations available (follow up of issue 2079) #2080

Conversation

jsmeix commented Mar 12, 2019

jsmeix commented Mar 12, 2019

OliverO2 left a comment

Choose a reason for hiding this comment

OliverO2 Mar 12, 2019

Choose a reason for hiding this comment

jsmeix Mar 13, 2019

Choose a reason for hiding this comment

OliverO2 Mar 12, 2019

Choose a reason for hiding this comment

jsmeix Mar 13, 2019

Choose a reason for hiding this comment

jsmeix commented Mar 12, 2019

jsmeix commented Mar 13, 2019

OliverO2 commented Mar 13, 2019

jsmeix commented Mar 13, 2019

OliverO2 commented Mar 13, 2019

jsmeix commented Mar 13, 2019

jsmeix commented Mar 13, 2019

jsmeix commented Mar 13, 2019

OliverO2 commented Mar 13, 2019

jsmeix commented Mar 13, 2019 • edited

jsmeix commented Mar 13, 2019 •

edited