Save old subvolume as a new snapshot #5
Conversation
…heck next next snapshot number from /.snapshot if it's a dry-run
There was a problem hiding this comment.
Thanks for the contribution. This is a great idea!
For consistency reasons: could you please format these changes with black and use snake_case instead of camelCase wherever possible?
One consideration I have when seeing these changes, I'm assuming that this script might have to run in situations where the system is completely hosed: the snapper subvolume is read-only, or btrfs is messed up somehow.
What if there was a try/catch around the snapshot creation code, and if any exception was thrown, it reverted back to the current behavior?
I'm afraid my system is currently not online, so I can't create a new release right away, but I'll get to it as soon as I can.
snapper-rollback.py
Outdated
| if dry_run: | ||
| LOG.info("Creating directory at {}".format(mountpoint / config.get("root", "subvol_snapshots") / dest)) | ||
| else: | ||
| os.mkdir(mountpoint / config.get("root", "subvol_snapshots") / dest ) |
There was a problem hiding this comment.
there's an ensure_dir function which will handle a few edge cases when creating directories
There was a problem hiding this comment.
I changed os.mkdir in createNextSubvolumeNumber to ensure_dir
snapper-rollback.py
Outdated
| @@ -48,6 +49,39 @@ def parse_args(): | |||
| args = parser.parse_args() | |||
| return args | |||
|
|
|||
| def generateXML(fileName,type,num,date,description,cleanup,dry_run=False) : | |||
There was a problem hiding this comment.
a lot of these parameters are either hardcoded or could be determined inside this function (like date param).
Let's also have a description or userdata which shows this was an active rootfs the user rolled back from
snapper-rollback.py
Outdated
| subvol_main_newname = pathlib.Path(f"{subvol_main}") | ||
| date = datetime.now().strftime("%Y-%m-%d %H:%M:%S") |
There was a problem hiding this comment.
These 2 variables are / should be unused now
snapper-rollback.py
Outdated
| if dry_run: | ||
| subvol_main_snapshot_number = str(int(os.listdir(path= "/.snapshots")[-1])+1) | ||
| else: | ||
| subvol_main_snapshot_number = str(int(os.listdir(path= | ||
| mountpoint / config.get("root", "subvol_snapshots") | ||
| )[-1])+1 |
There was a problem hiding this comment.
I think this should be the same whether dry-run or not.
Also, beware files not being listed in alphanumerical order. This should be sorted
…os.mkdir, reduced generateXML's parameter
…ion instead of whole description and now show source snapshot's creation date
Instead of moving old subvolume to
/btrfsroot/<subvol_main><date>we move old subvolume to/btrfsroot/<subvol_snapshots>/<new_snapshot_number>/snapshotand generate it'sinfo.xml