#3138 PR open: Add support for Dell PowerProtect Data Manager

schlomo opened issue at 2024-01-26 10:32:

Relax-and-Recover (ReaR) Pull Request Template

Please fill in the following items before submitting a new pull request:

Pull Request Details:

  • Type: New Feature

  • Impact: Normal

  • Reference to related issue (URL): Fixes #3126

  • How was this pull request tested?

Manual tests in Dell lab environment on RHEL/compatible 7,8,9 and SLES 12,15 and Ubuntu 20,22

  • Description of the changes in this pull request:

Add support for PPDM and fix bugs we encounted.

Introduces Bash associative arrays, mandating Bash 4

schlomo commented at 2024-01-26 10:33:

@rear/contributors Please have a look, I'd like to do the last fixes on Monday and merge because that is the limit for the Dell lab where I can run tests.

pcahyna commented at 2024-01-26 10:51:

First note: please fix the commit message of f15344afe91ac07b6015563f917dc7b141519bed and force-push.

pcahyna commented at 2024-01-26 10:56:

Second impression: please do not mix formatting changes and actual functionality changes in the same commit. 24c122798406e0d62887e0e2909bb942d44a88dd is about "Ensure minimum Bash version ", but it contains hundreds of unrelated changed lines.
I also have the impression that many of the formatting changes should be avoided, as they make the code look worse (and less in line with the examples in https://github.com/rear/rear/wiki/Coding-Style ).

schlomo commented at 2024-01-26 11:03:

Thanks @pcahyna for helping us to be better contributors, you are right in both points.

schlomo commented at 2024-01-26 11:08:

Im actually unhappy with
https://github.com/rear/rear/pull/3138/files#diff-0cf7e0a3b217a0feaf68c285ada67dbf21b981b3c6525c79d3fc29b9d956ada8R98-R103

image

because it mixes in all the backup methods into a single file instead of using our directory based overlay architecture to separate the concerns for each backup software. If it is OK with @rear/contributors then I'd gladly pull this code apart by introducing a new LD_LIBRARY_PATH_ADDITIONS variable that can be set for every backup tool.

The reason why I didn't do that is that I cannot test this change for all backup tools so that we would have to rely on eyeball inspection and later tests by others.

schlomo commented at 2024-01-26 13:11:

Another thought: I added a feature to backup and restore the ESP because PPDM doesn't backup vfat filesystems. I'm wondering if I should change this code to be more generic so that it would either happen automatically for any recovery that doesn't restore the ESP or that it can be activated for those commercial backup tools that can't backup the ESP themselves.

In my test case the ESP was ~7MB, not much at all.

[Export of Github issue for rear/rear.]