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

Code review - destroySnapshots - check that $dataSet is a snapshot before destroying it #401

Closed Beneter closed 5 years ago

Beneter commented 5 years ago

I am currently doing a code review of znapzend, without having much of experience in Perl.

https://github.com/oetiker/znapzend/blob/4c758e6b330a0f8b607630a57e9a196fa61863a9/lib/ZnapZend/ZFS.pm#L243-L247

I was wondering if splitDataSetSnapshot shall be invoked before destroying $dataSet, to be sure it is actually a snapshot and not a complete dataset.

oetiker commented 5 years ago

yes, looking at the code this seems to make sense ... the oracle mode does not get all that much attention I guess :) will you create a PR ?

redmop commented 5 years ago

Please don't remove it. I just needed it about 2 weeks ago. I had too many snapshots and I needed Oracle mode to fix it.

On Fri, Jan 4, 2019, 3:30 AM Tobias Oetiker <notifications@github.com wrote:

yes, looking at the code this seems to make sense ... the oracle mode does not get all that much attention I guess :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/oetiker/znapzend/issues/401#issuecomment-451408030, or mute the thread https://github.com/notifications/unsubscribe-auth/ANsAr_1PtEDd7S51-DZLyL8C-kCcVG_iks5u_y00gaJpZM4Zk-dG .

oetiker commented 5 years ago

not intending to remove it just fixing it :-)

redmop commented 5 years ago

Whew! ;)

Beneter commented 5 years ago

will you create a PR ?

Yes, I will for both issues. Might take me some time though... :-)