search

oetiker / znapzend

zfs backup with remote capabilities and mbuffer integration.
www.znapzend.org
GNU General Public License v3.0
608 stars 137 forks source link

Recursive destroy #386

Closed jimklimov closed 4 years ago

jimklimov commented 5 years ago

This should take care of #385, but so far has not been tested on real systems ;)

coveralls commented 5 years ago

Coverage Status

Coverage increased (+1.8%) to 86.467% when pulling 980e3f1248015105763af93871293f5471e7acb9 on jimklimov:recursive-destroy into c319917faac0cea712f7deaab23a9d42623b654b on oetiker:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.9%) to 87.518% when pulling 1dfbb4c89a2d7fef62a2b3bb1d2f95d77254ea30 on jimklimov:recursive-destroy into 1154bc279c0fdc586bf7cd15e15b187a0cdffaed on oetiker:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+1.8%) to 86.467% when pulling 980e3f1248015105763af93871293f5471e7acb9 on jimklimov:recursive-destroy into c319917faac0cea712f7deaab23a9d42623b654b on oetiker:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+1.8%) to 86.467% when pulling 980e3f1248015105763af93871293f5471e7acb9 on jimklimov:recursive-destroy into c319917faac0cea712f7deaab23a9d42623b654b on oetiker:master.

jimklimov commented 5 years ago

Tested on local system, wrinkled out some issues and optimized a bit, seems to work for me. Details about test setup and routine in #385.

oetiker commented 5 years ago

what would be really cool would be if you enhanced the test suite accordingly

jimklimov commented 5 years ago

I am not actually sure how to go about auto-testing it at this time :\

I did git grep destroySnapshot and found one more use-case I missed before :) but otherwise there were no apparent hits under tests. A gmake check fails for me regardless - in the master branch as well...

jimklimov commented 4 years ago

Bumped to align with current master.

Also read up today on ability to have resursive mode but certain datasets not enabled.

jimklimov commented 4 years ago

Updated to align with current master. Every PR wants some love one year or another ;)

Also read today about an ability to have recursive backup plan, but have it not enabled on some child datasets. I wonder if this recursive mass-removal would contradict anything (e.g. cleaning up datasets that we did not intend to actively back up? good... To touch? bad...)

oetiker commented 4 years ago

any thoughts on how to add testing ?

jimklimov commented 4 years ago

Probably a few lines in random files under t/ directory should do it. Just gotta carve them out :)

jimklimov commented 4 years ago

So I somewhat cheated at first and covered the previously existing but not-covered code ;)

And went from there to cover some of the new paths as well.

jimklimov commented 4 years ago

I anticipate that after merging #439, the coverage would also include large blocks about (cleaning) child datasets that inherit a backup plan but are not directly configured. I did not port it here because it largely depends on changes made for that PR, in both mock zfs and maybe the production code.

jimklimov commented 4 years ago

Idea for after merging these PRs (and so taking advantage of the logical structure in that one) - also add tank/source child datasets that would have org.znapzend:enabled==off into t/zfs output, and so test -cover one more family of codepaths.

UPDATE: Added this feature in that PR.

oetiker commented 4 years ago

let me know when you deem the whole thing complete :) I will then review!

jimklimov commented 4 years ago

I hope the two PRs are now complete (and beyond, with the testing cleanups) and should hopefully not conflict when merging :)