[rear-devel] [rear/rear] dc1f10: Reduce difference in rsync restore script

Johannes Meixner noreply at github.com
Thu Jun 24 14:46:29 CEST 2021 

  Branch: refs/heads/master
  Home:   https://github.com/rear/rear
  Commit: dc1f10bd37b1ae7930020b7a3ed78e83764240a6
      https://github.com/rear/rear/commit/dc1f10bd37b1ae7930020b7a3ed78e83764240a6
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Reduce difference in rsync restore script

Sync some whitespace from
backup/RSYNC/default/500_make_rsync_backup.sh
to
restore/RSYNC/default/400_restore_rsync_backup.sh


  Commit: 50838fb4b9b363c3944d02eda8f62652bb48494d
      https://github.com/rear/rear/commit/50838fb4b9b363c3944d02eda8f62652bb48494d
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Correct error detection when restoring via rsync

The exit code from the rsync process is stored in $TMP_DIR/retval, but
we forgot to actually check it and we used the exit code from the
compound command, which was the last command in sequence - the echo
command that actually stored the exit code.

Copy the similar code from 500_make_rsync_backup.sh, which handles it
correctly.

Fixes #2612


  Commit: 4a6814cb597caf164eba0cb8e41aa43ff6e21819
      https://github.com/rear/rear/commit/4a6814cb597caf164eba0cb8e41aa43ff6e21819
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Use LogPrintError for displaying the rsync error

Errors when restoring are important, because they often indicate a
broken system. Reflect it by using LogPrintError instead of LogPrint.

No functional change as of now because currently "rear recover" runs in
verbose mode and would display the message anyway.


  Commit: d041fbe2e05c4798f42f3fd53c1010936b3bfdfd
      https://github.com/rear/rear/commit/d041fbe2e05c4798f42f3fd53c1010936b3bfdfd
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Fix typos in comments


  Commit: 4aaab600b45bc8f0eaacf54875e24dcf21cf6f02
      https://github.com/rear/rear/commit/4aaab600b45bc8f0eaacf54875e24dcf21cf6f02
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh

  Log Message:
  -----------
  Use LogPrintError instead of VERBOSE=1 LogPrint

The result is virtually the same (the only difference is in stderr vs.
stdout), but LogPrintError is more idiomatic.

TODO: we should probably abort in this case because there is strong risk
that the backup will be unusable and the user will not notice the error
message until it is too late.


  Commit: c420d254f31b349348c8a586601ff956b1afb385
      https://github.com/rear/rear/commit/c420d254f31b349348c8a586601ff956b1afb385
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh

  Log Message:
  -----------
  Eliminate superfluous triple double quotes

"""foo bar""" is exactly the same as "foo bar", but it is needlessly
long and leaves one wondering whether the quote triplification has some
deep meaning.


  Commit: 62e588e88c95a430e65d948454530cf5aa04a105
      https://github.com/rear/rear/commit/62e588e88c95a430e65d948454530cf5aa04a105
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/NETFS/default/500_make_backup.sh
    M usr/share/rear/backup/RSYNC/GNU/Linux/610_start_selinux.sh
    M usr/share/rear/backup/RSYNC/GNU/Linux/620_force_autorelabel.sh
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh
    M usr/share/rear/restore/DUPLICITY/default/400_restore_duplicity.sh
    M usr/share/rear/restore/RBME/default/400_restore_backup.sh
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  cleanup: rename the _rc variable, make it local

Use backup_prog_rc as the name of the variable holding the return code
from the backup program instead of _rc


  Commit: f6bccc355616cd67cfb007291219efb494460445
      https://github.com/rear/rear/commit/f6bccc355616cd67cfb007291219efb494460445
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/restore/DUPLICITY/default/400_restore_duplicity.sh
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  cleanup: rename _message to restore_log_message

and make it a local variable


  Commit: 3fe2213e643bd3c6913f19dc33c2a69eb6b18d86
      https://github.com/rear/rear/commit/3fe2213e643bd3c6913f19dc33c2a69eb6b18d86
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh

  Log Message:
  -----------
  cleanup: rename _mntpt to remote_mountpoint

and declare it as local


  Commit: a1925f04335cd341525ed868791c7407a6257acd
      https://github.com/rear/rear/commit/a1925f04335cd341525ed868791c7407a6257acd
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/output/RSYNC/default/900_copy_result_files.sh

  Log Message:
  -----------
  Fix typo in a log message


  Commit: e7efac5564221f6be04a2df824b3843cd6ff3646
      https://github.com/rear/rear/commit/e7efac5564221f6be04a2df824b3843cd6ff3646
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/GNU/Linux/620_force_autorelabel.sh
    M usr/share/rear/output/RSYNC/default/200_make_prefix_dir.sh
    M usr/share/rear/output/RSYNC/default/900_copy_result_files.sh
    M usr/share/rear/prep/RSYNC/default/100_check_rsync.sh
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh
    M usr/share/rear/verify/RSYNC/default/550_check_remote_backup_archive.sh

  Log Message:
  -----------
  cleanup: use explicit tests instead of ...IfError

The ...IfError functions have multiple problems, see the comments at
their definition.


  Commit: 45e3c6e8041ff337b4dc64e33ed6686510b8879b
      https://github.com/rear/rear/commit/45e3c6e8041ff337b4dc64e33ed6686510b8879b
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Do not create unused $TMP_DIR/rsync/$NETFS_PREFIX


  Commit: 1a5066f3efe93773fad9f9a40907e00e54f43a3c
      https://github.com/rear/rear/commit/1a5066f3efe93773fad9f9a40907e00e54f43a3c
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/GNU/Linux/610_start_selinux.sh
    M usr/share/rear/backup/RSYNC/GNU/Linux/620_force_autorelabel.sh
    M usr/share/rear/backup/RSYNC/default/700_copy_backup_log.sh
    M usr/share/rear/output/RSYNC/default/200_make_prefix_dir.sh
    M usr/share/rear/output/RSYNC/default/900_copy_result_files.sh
    M usr/share/rear/prep/RSYNC/default/100_check_rsync.sh

  Log Message:
  -----------
  Properly quote ${BACKUP_RSYNC_OPTIONS[@]}

Prevents splitting options containing whitespace.

Use the ${BACKUP_RSYNC_OPTIONS[*]} form inside larger quoted strings.
Expansion of ${BACKUP_RSYNC_OPTIONS[@]} inside a larger quoted string
leads to splitting the string into multiple strings. This is usually not
intended, although in case of functions like Log it is harmless, since
they can handle multiple strings properly.


  Commit: 69023f1db552941ab997571825f6f84129c34528
      https://github.com/rear/rear/commit/69023f1db552941ab997571825f6f84129c34528
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/200_check_rsync_relative_option.sh
    M usr/share/rear/prep/NETFS/default/400_automatic_exclude_recreate.sh
    M usr/share/rear/prep/RSYNC/GNU/Linux/200_selinux_in_use.sh
    M usr/share/rear/restore/RSYNC/default/200_remove_relative_rsync_option.sh
    M usr/share/rear/verify/RSYNC/GNU/Linux/600_check_rsync_xattr.sh

  Log Message:
  -----------
  cleanup: do not use $( echo ...) with <<<

The <<< redirection already performs parameter expansion, so
 <<< $( echo ${...} ) is unnecessary and can be replaced by just <<< ${...}.
Quote array expansion and use [*]. It is not strictly necessary, because
 <<< does not perform word splitting, but is done here for of
consistency with other uses of array expansions when a single string is
desired.


  Commit: edf3b6d64d5e9b8ed839b947ecac13e60e55a5c7
      https://github.com/rear/rear/commit/edf3b6d64d5e9b8ed839b947ecac13e60e55a5c7
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh

  Log Message:
  -----------
  cleanup: rename _message to backup_log_message

and declare it as local


  Commit: 3280d560158ec95f95a63c0d7c17d0cc0461682a
      https://github.com/rear/rear/commit/3280d560158ec95f95a63c0d7c17d0cc0461682a
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Stop mentioning "archive" when using rsync

rsync does not create archives, so referring to "archive" in the
log/user messages is confusing. Refer to "backup" or "destination"
instead.


  Commit: d0843c760cab872424b7e5d104d1417c22fe15ab
      https://github.com/rear/rear/commit/d0843c760cab872424b7e5d104d1417c22fe15ab
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh

  Log Message:
  -----------
  If rsync reports an error, abort backup process

The former approach of logging a "WARNING !" and continuing is
unsatisfactory for backup reliability. If ReaR gets invoked by some
automation, this warning will get unread and unusable backups can be
produced. When this happens, one will discover it only during recovery,
i.e. too late. To prevent this, when any error (nonzero exit code) is
detected, abort via Error.

The new error message tells users that they can exclude problematic
files from the backup if the error is spurious.


  Commit: fe3424be6d46038065d6efa063d5e6ad9a304953
      https://github.com/rear/rear/commit/fe3424be6d46038065d6efa063d5e6ad9a304953
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/output/RSYNC/default/900_copy_result_files.sh

  Log Message:
  -----------
  cleanup: use [*] for array expansion in strings

Using the ${RESULT_FILES[@]} form inside a longer quoted string splits
the string into multiple pieces, which is usually not intended (although
harmless in the case of functions like Error or Log that process
multiple arguments in a similar way like echo).


  Commit: a865e33e12d55f3f92ec06eda0efb147c153076b
      https://github.com/rear/rear/commit/a865e33e12d55f3f92ec06eda0efb147c153076b
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/prep/RSYNC/default/100_check_rsync.sh
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh

  Log Message:
  -----------
  cleanup: do not test $? after invoking a command

Instead of examining $? use the command directly in the condition.
This conforms to the usual convention and is better compatible with set
-e (errexit).

Also do not echo a variable to a pipe, use the <<< redirection.


  Commit: 19ee83a6f079ba52250d9f6c8516532173f1b6a8
      https://github.com/rear/rear/commit/19ee83a6f079ba52250d9f6c8516532173f1b6a8
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh

  Log Message:
  -----------
  Remove a broken test for --fake-super arg to rsync

The test was broken in multiple ways: it had a typo and it would have to
use quoting and [*] instead of [@] in order to work properly. The intent
was wrong as well: if the remote user is not root, we must use
--fake-super (otherwise file metadata will not be preserved), so if
--fake-super is not supported, we should abort always, not only when
--fake-super has been explicitly given. Therefore, delete the whole test
and leave the unconditional Error.


  Commit: 9e90820b477c8c3801036058910f567ebd606408
      https://github.com/rear/rear/commit/9e90820b477c8c3801036058910f567ebd606408
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/conf/default.conf
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  If checking integrity abort restore on rsync error

For BACKUP=NETFS, when BACKUP_INTEGRITY_CHECK is set, the restore
process errors out when the backup program (tar, rsync) returns an error
(nonzero exit code).

Do the same for BACKUP=RSYNC. This allows the user to catch possibly
broken restored systems early. Related issue: #2612

Not done by default: the reasoning is that it is better to continue the
recover process in order to get the system into a basically usable
state (the problem may be with restoring some unimportant file).

This does not mean that BACKUP_INTEGRITY_CHECK is now fully supported
for BACKUP=RSYNC: checking of md5sums is not there.


  Commit: c899116a89bacc2fda17180b9ec056e397a246a5
      https://github.com/rear/rear/commit/c899116a89bacc2fda17180b9ec056e397a246a5
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh

  Log Message:
  -----------
  Return the rsync exit code from the restore script

Allows detecting (in an upper layer) that there were rsync problems when
integrity checking is not enabled. Currently the exit code is not used
for anything, but could be used in the future.


  Commit: 1b038ef91603833867dfc93a3204b53ed2d00295
      https://github.com/rear/rear/commit/1b038ef91603833867dfc93a3204b53ed2d00295
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/RSYNC/default/700_copy_backup_log.sh

  Log Message:
  -----------
  cleanup: make the timestamp var lowercase & local

- in order to conform to Coding-style.


  Commit: 47b17fece6b8d2d787832aaafc7f7c7c3746e123
      https://github.com/rear/rear/commit/47b17fece6b8d2d787832aaafc7f7c7c3746e123
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/verify/RSYNC/GNU/Linux/600_check_rsync_xattr.sh

  Log Message:
  -----------
  Fix forgotten variable rename

In 20b214db4d8142814ec9c38536f93d40ef46645d, RSYNC_OPTIONS got renamed
to BACKUP_RSYNC_OPTIONS, but one instance was left. Fix that.

Also append to an array using +=, it is more readable and less
error-prone.


  Commit: 76cd3e6d464eaf29ff3f1138695f277cf2b917bb
      https://github.com/rear/rear/commit/76cd3e6d464eaf29ff3f1138695f277cf2b917bb
  Author: Pavel Cahyna <pcahyna at redhat.com>
  Date:   2021-06-21 (Mon, 21 Jun 2021)

  Changed paths:
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh

  Log Message:
  -----------
  Check fake super support only when RSYNC_PROTO=ssh

If RSYNC_PROTO=rsync, there is no way to know what the server supports,
the "$TMP_DIR/rsync_protocol" file gets created only when using ssh.
Performing this check for rsync protocol is useless anyway:
--rsync-path="rsync --fake-super" will not do the right thing, because
--rsync-path is not used when talking to the rsync daemon. (I would
assume that -M--fake-super would work, but in my tests it does not work
either, unfortunately.)
Fixing the fake super check for the RSYNC_PROTO=rsync case was probably
the intent of 3548430302cef3a8445de970eb763da33d9f6e8e, but it can't
work this way and fixing it properly will be more work (if it can be
done at all).


  Commit: bd484848eeed0c2a761f3244e4ab5e96396257af
      https://github.com/rear/rear/commit/bd484848eeed0c2a761f3244e4ab5e96396257af
  Author: Johannes Meixner <jsmeix at suse.com>
  Date:   2021-06-24 (Thu, 24 Jun 2021)

  Changed paths:
    M usr/share/rear/backup/NETFS/default/500_make_backup.sh
    M usr/share/rear/backup/RSYNC/GNU/Linux/610_start_selinux.sh
    M usr/share/rear/backup/RSYNC/GNU/Linux/620_force_autorelabel.sh
    M usr/share/rear/backup/RSYNC/default/200_check_rsync_relative_option.sh
    M usr/share/rear/backup/RSYNC/default/500_make_rsync_backup.sh
    M usr/share/rear/backup/RSYNC/default/700_copy_backup_log.sh
    M usr/share/rear/conf/default.conf
    M usr/share/rear/output/RSYNC/default/200_make_prefix_dir.sh
    M usr/share/rear/output/RSYNC/default/900_copy_result_files.sh
    M usr/share/rear/prep/NETFS/default/400_automatic_exclude_recreate.sh
    M usr/share/rear/prep/RSYNC/GNU/Linux/200_selinux_in_use.sh
    M usr/share/rear/prep/RSYNC/default/100_check_rsync.sh
    M usr/share/rear/prep/RSYNC/default/150_check_rsync_protocol_version.sh
    M usr/share/rear/restore/DUPLICITY/default/400_restore_duplicity.sh
    M usr/share/rear/restore/RBME/default/400_restore_backup.sh
    M usr/share/rear/restore/RSYNC/default/200_remove_relative_rsync_option.sh
    M usr/share/rear/restore/RSYNC/default/400_restore_rsync_backup.sh
    M usr/share/rear/verify/RSYNC/GNU/Linux/600_check_rsync_xattr.sh
    M usr/share/rear/verify/RSYNC/default/550_check_remote_backup_archive.sh

  Log Message:
  -----------
  Merge pull request #2632 from pcahyna/cleanup-fix-rsync-updated

Cleanup rsync and fix error reporting:
Cleanups of rsync code to use better variable names and local variables,
stop using StopIfError, use better redirections etc.
Fixed a problem with rsync error detection that caused rsync errors
during backup restore to be ignored, see
https://github.com/rear/rear/issues/2612
Now a warning is displayed and if BACKUP_INTEGRITY_CHECK is true,
it is elevated to an error and ReaR aborts.
Furthermore check fake super support only when RSYNC_PROTO=ssh
and removed a broken test for --fake-super arg to rsync, cf.
https://github.com/rear/rear/pull/2577


Compare: https://github.com/rear/rear/compare/e670f504220f...bd484848eeed

More information about the rear-devel mailing list