#2201 PR merged: Fix for issue 2200: support new use-case for BLOCKCLONE backup method…

Labels: enhancement, fixed / solved / done

petroniusniger opened issue at 2019-08-05 11:34:

… by attempting to unmount device before backup/restore.

  • Type:Enhancement

  • Impact: Normal

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

  • How was this pull request tested?

    • test VM installed with Leap 15.0 (grub2-efi, LVM, filesystems are ext4 and xfs)
    • created new, LUKS-encrypted logical volume with xfs mounted on /products
    • stored an extra decryption passphrase into slot #1 of LUKS header (main key in slot #0)
    • created a "multiple backup" configuration where the base system is backed up the usual way (ISO + NETFS to NFS share), ignoring /products, while the block device that hosts /products is then backed up (mkbackuponly) using the BLOCKCLONE method. See configuration files below:
      site.conf.txt
      base_system.conf.txt
      products_backup.conf.txt
    • the new option BLOCKCLONE_TRY_UNMOUNT is set to "yes"
    • during /products backup phase (mkbackuponly), the filesystem is now successfully unmounted prior to issuing the dd command
    • at the end of the backup phase, that filesystem is also successfully re-mounted
    • the test VM is then shut down and its virtual HDD is deleted and recreated empty (same size, in this case)
    • during base system recover phase, the encrypted filesystem is correctly recreated (the user is prompted to enter a new passphrase -- which will later be overwritten). No data is restored on that filesystem as we ignored it through BACKUP_PROG_EXCLUDE
    • during /products restore phase (restoreonly) the target device (/dev/vg00/lvol4) is successfully unmounted prior to issuing the dd command, which overwrites the recreated encrypted filesystem with the original LUKS keys and all the filesystem data
    • upon reboot of the VM, all filesystems are clean, /products can be decrypted using any of the 2 original passphrases and can be successfully mounted

  • Brief description of the changes in this pull request:

    • new configuration variable BLOCKCLONE_TRY_UNMOUNT in default.conf, including short documentation
    • 500_start_clone.sh now use existing global function umount_mountpoint() to try and unmount the source device if BLOCKCLONE_TRY_UNMOUNT is set to "yes". It obtains the mount point through a new global function called get_mountpoint()
    • if the unmounting fails, 500_start_clone.sh proceeds as before, issuing a warning message that the backup has been taken while the source device was mounted
    • if the device was initially mounted AND it could be unmounted, then 500_start_clone.sh attempts to re-mounted it before exiting
    • 400_restore_clone.sh now implements the same checks and unmount attempt as 500_start_clone.sh. It does NOT try to remount the target device at the end, but issues a warning message instead
    • bug fix in global function umount_mountpoint(): return value now visible by calling script
    • cosmetics in global function is_device_mounted(): local variables now defined as local
    • new global function get_mountpoint()
    • extensive documentation of the new BLOCKCLONE use-case in the User Guide, chapter 12
    • fixed typo in User Guide, chapter 11, "multiple backups"

jsmeix commented at 2019-08-07 11:56:

@gozora
could you have a look here?

FYI:
I am not in the office for some weeks so that I cannot do much for ReaR,
in particular I cannot try out something.

gozora commented at 2019-08-07 16:55:

I'll take a look as time permits (most probably during this weekend) ...

V.

petroniusniger commented at 2019-08-22 08:44:

Updated code to fulfil both suggestions by @gozora.

Short comment about fixes in is_device_mounted() function:

  • local variables now defined as local (was already the case in my first commit)
  • fixed an 'inverse logic' bug in the test for empty parameter: not mounted or not found should return 0, not 1
  • added missing early exit to the test for empty parameter -- passing an empty argument to the function might have resulted in an error similar to the following:
./fn_is_mounted: line 10: [: [SWAP]: binary operator expected

All of this does nothing to address the (valid) remark of @jsmeix about the behaviour of is_something() functions, but as I call is_device_mounted() from the code I contributed, I thought it fair game to fix these bugs at this time.

Regarding testing:

  • I've tested extensively all three functions I modified or added by taking them out of ReaR and by feeding them different values to trigger a positive or negative result, including empty strings
  • I've again performed a full "2-steps backups + 2-steps recover" scenario on a VM with a LUKS-encrypted filesystem. Everything behaved as expected.

Branch issue-2200 has been re-based against trunk/HEAD before commit/push.

gozora commented at 2019-08-26 08:47:

@petroniusniger thanks for updated PR.
Please fix those couple indentation problems, and I'll merge this PR.

Thanks!

V.

gozora commented at 2019-09-02 17:26:

@petroniusniger many thanks for contributing this new functionality and respective documentation to ReaR!

V.

[Export of Github issue for rear/rear.]