#1284 PR merged: Make NETFS_RESTORE_CAPABILITIES more fail safe

Labels: enhancement, documentation, cleanup, fixed / solved / done

jsmeix opened issue at 2017-04-11 09:29:

Changed NETFS_RESTORE_CAPABILITIES
from a binary variable to an array that contains
those directories where ReaR should save and
restore file capabilities, see
https://github.com/rear/rear/issues/1283
and the related issue
https://github.com/rear/rear/issues/1175
This change is not backward compatible because now
NETFS_RESTORE_CAPABILITIES="yes"
does no longer work as expected.
I cannot enhance it so that
NETFS_RESTORE_CAPABILITIES="yes"
works as before which would mean to
run 'getcap -r /' as generic fallback
because that command is the reason for
https://github.com/rear/rear/issues/1283
which this pull request intends to fix.

jsmeix commented at 2017-04-11 09:30:

@gdha @gozora
this is currently only a proposal.
I did not test it in any way - and currently
I do not have time for such tests.

jsmeix commented at 2017-04-11 09:31:

@dagwieers
I appreciate also your review!

And of course I appreciate if needed
further adaptions and enhancements.

jsmeix commented at 2017-04-11 09:49:

@gdha I think we cannot move a script that
creates $VAR_DIR/recovery/capabilities
to the 'backup' stage, see
https://github.com/rear/rear/issues/1283#issuecomment-293207445

gdha commented at 2017-04-11 09:50:

@jsmeix That is correct, but nothing forbids us to add this file in the BACKUP_URL path.

jsmeix commented at 2017-04-11 10:04:

We also cannot use the @gozora hack, see
https://github.com/rear/rear/issues/1283#issuecomment-293209833

@gdha
I guess with "add this file in the BACKUP_URL path"
you actually meant to include this file in the backup
so that it gets restored as
$TARGET_FS_ROOT/$VAR_DIR/recovery/capabilities
and then use it from there in the ReaR recovery system
to restore the capabilities?

As usual - trying to implement a workaround so that it
automatically does things right becomes a pile of more
and more workarounds on top of the initial workaround
and this all only to try to make users "happy"
(will they ever really get happy this way?)
who actually have a bug in 'tar' ?

jsmeix commented at 2017-04-11 10:08:

Wait!
Shouldn't /var/lib/rear/recovery/capabilities
be automatically included in the backup so that
after backup restore it would be automatically there as
$TARGET_FS_ROOT/var/lib/rear/recovery/capabilities
so that all what might be needed is to adapt
restore/NETFS/default/510_set_capabilities.sh
to read its input from
$TARGET_FS_ROOT/var/lib/rear/recovery/capabilities
?

On my SLES12 test system where I user ReaR
as GitHub checkout into the /root/rear/ directory
I have in backup.tar.gz the whole
root/rear/var/lib/rear/recovery/
directory with all files.

This means it gets restored as
$TARGET_FS_ROOT/root/rear/var/lib/rear/recovery/

jsmeix commented at 2017-04-11 12:13:

Now it seems to work for me:

In contrast to my initial comment I made it even
backward compatible so that now
NETFS_RESTORE_CAPABILITIES="Yes"
uses as fallback '/' which is sufficiently fast for me
both during "rear mkbackup" and "rear recover".

On my test system (without tons of mounted stuff)
I get for "rear -d -D mkbackup" in the log

2017-04-11 13:46:39.062951633 Saving file capabilities
...
2017-04-11 13:46:43.153278799 Leaving ...

i.e. 4 seconds for 'getcap -r /'
and during "rear -d -D recover" in the log

2017-04-11 11:50:49.177377723 Restoring file capabilities
...
2017-04-11 11:50:49.184422239 Leaving ...

i.e. basically nothing for restoring file capabilities.

On systems with tons of mounted stuff where 'getcap -r /'
is too slow the user can as needed specify on what
directories 'getcap -r' should be run.

On my test system I have

# getcap -r / 2>/dev/null
/usr/lib/gstreamer-1.0/gst-ptp-helper = cap_net_bind_service+ep
/usr/bin/ping6 = cap_net_raw+ep
/usr/bin/ping = cap_net_raw+ep

so that I also tested
NETFS_RESTORE_CAPABILITIES=( '/usr' '/var' )
which also works well for me.

FYI:
Regardless that I use a ReaR GitHub checkout
in /root/rear on the original system I have in the
recovery system

RESCUE e205:~ # cat /var/lib/rear/recovery/capabilities 
/usr/lib/gstreamer-1.0/gst-ptp-helper = cap_net_bind_service+ep
/usr/bin/ping6 = cap_net_raw+ep
/usr/bin/ping = cap_net_raw+ep

and after "rear -d -D recover" I get

RESCUE e205:~ # getcap -r /mnt/local 2>/dev/null
/mnt/local/usr/bin/ping6 = cap_net_raw+ep
/mnt/local/usr/bin/ping = cap_net_raw+ep
/mnt/local/usr/lib/gstreamer-1.0/gst-ptp-helper = cap_net_bind_service+ep

jsmeix commented at 2017-04-11 12:15:

I think if there are no furious objections I will merge it
because it is an improvement to what it was before
and then others can further adapt and enhance it
"to make all and everything just work out of the box"
;-)

jsmeix commented at 2017-04-11 12:35:

rescue/NETFS/default/600_store_NETFS_variables.sh
is buggy because with
NETFS_RESTORE_CAPABILITIES=()
rescue/NETFS/default/600_store_NETFS_variables.sh
results in /etc/rear/rescue.conf

NETFS_RESTORE_CAPABILITIES='()'

which is not an empty array but the string "()" so that
restore/NETFS/default/510_set_capabilities.sh
does not return but tries to restore file capabilities :-(

FYI

# NETFS_RESTORE_CAPABILITIES=()
# declare -p ${!NETFS*} | sed -e 's/declare .. //'
NETFS_RESTORE_CAPABILITIES='()'

jsmeix commented at 2017-04-11 12:59:

rescue/NETFS/default/600_store_NETFS_variables.sh
is broken:
For
NETFS_RESTORE_CAPABILITIES=( 'No' )
it stores in /etc/rear/rescue.conf
NETFS_RESTORE_CAPABILITIES='([0]="No")'
i.e. NETFS_RESTORE_CAPABILITIES is the string

([0]="No")

jsmeix commented at 2017-04-11 14:21:

Simple 'set' seems to "just work" for me:

# NETFS_BAR=()
# NETFS_FOO="foo bar"
# NETFS_RESTORE_CAPABILITIES=( 'no' 'foo bar' )

# declare -p ${!NETFS*}
declare -a NETFS_BAR='()'
declare -- NETFS_FOO="foo bar"
declare -a NETFS_RESTORE_CAPABILITIES='([0]="no" [1]="foo bar")'

# set | grep ^NETFS
NETFS_BAR=()
NETFS_FOO='foo bar'
NETFS_RESTORE_CAPABILITIES=([0]="no" [1]="foo bar")

In contrast to 'declare -p' simple 'set' reports directly usable
variable assignments without additional single quotes.

jsmeix commented at 2017-04-11 14:45:

O.k. now it seems to work well for me so that I merge it.


[Export of Github issue for rear/rear.]