-
Notifications
You must be signed in to change notification settings - Fork 138
Add snapshots restoring support to snbk #1072
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Thanks a lot for the patch. It look very good. One problem is that snapperd will not know about the restored snapshots (if it is currently running). That should be documented. May you mind explaining your use case? |
client/snbk/Shell.cc
Outdated
|
|
||
| SystemCmd::Args | ||
| shellify_pipe(const SystemCmd::Args& args1, const Shell& shell2, const SystemCmd::Args& args2) | ||
| string _wrap_shell_args(const Shell& shell, const SystemCmd::Args& args) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Names starting with an underscore are problematic - although should be fine in this case. Please move the function (without the underscore) to the anonymous namespace above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your valuable review. I hadn’t even learned about the anonymous namespace before. It’s now fixed.
No problem! It’s my pleasure to contribute.
Got it. I will add some documentation for this in a future commit.
Sure! We use Ubuntu 24.04 on Btrfs for our production server systems. I have successfully restored misconfigured settings several times with Btrfs snapshots, which are really helpful. Recently, we started doing full system backups for our production systems. Since Btrfs is used, I prefer making backups using the method natively supported by Btrfs — Btrfs send/receive. So that I don’t need to worry about any filesystem-level inconsistency between the source and the backups. We store the system backups to iSCSI disks provided by our storage server. With this setup:
After restoring the system, I need to make sure all snapshots are fully restored. Otherwise, the backups might be removed when the scheduled snapper-backup service is triggered. This is the reason why I created this pull request. For 2 and 3 is the reason why I would like to propose a new Btrfs send parent finding algorithm. The parents of snapshots may be cleaned up by the timeline service. Snapshots without explicit parents won’t be sent incrementally, leading to additional disk space usage overhead. Please let me know if you want to know more about the details. |
|
I’ve added the restore command to the |
|
When running restore and there are several backup configs for one snapper config the code tries to restore the snapshot several times. I think a check for |
|
The restore command should also be added to the bash completion. |
|
Thanks for describing your use case. Are you restoring snapshots of the root file system or some data file system? If for the root file system how do you manage the various different UUIDs (e.g. of the root btrfs file system in fstab)? |
|
I’ve added a check to prohibit running |
I’m restoring both the root and the user data file systems ( If a conflict occurs, I may add an extra identifier to the file system labels to resolve it. For example, changing I perform system restoration in a live environment. If the file system is mounted by label, I simply ensure that the file systems on the new storage are formatted with the corresponding labels. Mounting by UUID is also fine. I can modify the |
The pull request adds a snapshot restoring support to
snbk.This allows users to transfer snapshots from the backup target back to the source using the following new command:
This PR provides a foundational piece toward resolving issue #194.
I also personally need this feature for restoring server systems in the event of a disaster.
There is one limitation I have not been able to resolve: after restoring snapshots,
snapperis not showing the restored snapshots unless thesnapperdservice is restarted.I would appreciate any advice on how to address this. If there is anything that needs to be changed in this PR, please let me know!
Details about this PR
Btrfs Send Parent Finding
The algorithm for finding the Btrfs send parent during restoration is slightly different from that used during snapshot transfer.
When transferring snapshots to the backup target,
snbkrequires an explicit parent snapshot to perform an incremental backup. However, these parent snapshots may have been removed by timeline cleanup.To reduce disk usage, the nearest previous available snapshot is used as the send parent if the explicit parent does not exist.
I would like to open another PR to enhance and unify the send parent finding algorithm for both transferring and restoring with a tree-based searching. I would appreciate your feedback.
Shell Module
A
shell1parameter is added to theshellify_pipefunction. I think this change is required for restoring snapshots from an SSH backup target.Potential Duplicated Implementations
The implementations for transferring and restoring snapshots share duplicated logic.
If this PR is accepted, I would like to submit another PR to unify these implementations to reduce maintenance overhead.
Coding Style
From viewing the codebase, it appears that a line width of approximately 90 characters is used.
I have tried to follow the existing style in this PR. If any adjustments are needed or if a specific formatter is used,
please let me know, and I will reformat accordingly.