Add Read-Only Btrfs Snapshots Option (issue #332)

[CloverGit]

Adds an option to create read-only Btrfs snapshots by default. Resolves #332

[ygerlach]

Do we need a warning if someone tries to use --readonly in an rsync setup?

[CloverGit]

The shell argument description already clarifies that this feature is specifically for Btrfs, and I believe that users familiar with rsync will understand that it doesn't have a "read-only" concept. However, if necessary, perhaps consider changing the argument name to '--btrfs-readonly'?

[cshandley-uk]

Please do NOT make snapshots read-only, as this will basically break "grub-btrfs", and prevent the ability to boot into a snapshot. Or if you DO make them read-only, then there should be a separate set of snapshots that are read-write, but I think that would be over-complicating things. At best make this change optional please.

[agusdmb]

Please do NOT make snapshots read-only, as this will basically break "grub-btrfs", and prevent the ability to boot into a snapshot. Or if you DO make them read-only, then there should be a separate set of snapshots that are read-write, but I think that would be over-complicating things. At best make this change optional please.

i think having it optional would be ideal

[CloverGit]

Please do NOT make snapshots read-only, as this will basically break "grub-btrfs", and prevent the ability to boot into a snapshot. Or if you DO make them read-only, then there should be a separate set of snapshots that are read-write, but I think that would be over-complicating things. At best make this change optional please.

I agree with making it optional, but I still recommend keeping read-only as the default behavior, a "writable snapshot" contradicts the design purpose of snapshot (frozen disk state).
For read-only snapshots, you should use grub-btrfs-overlayfs instead of grub-btrfs.

[ygerlach]

One can always change the "read-only-nes" of a snapshot by either

• creating a new writeable snapshot of the given read-only snapshot (btrfs subvolume snapshot /path/to/snapshot /path/to/snapshot_rw
  or
• actually changing the flag btrfs property set -ts /path/to/snapshot ro false

I think having an option to set the value for all new snapshots (default to ro) and having the option to toggle a snapshot with the context menu would be the best option for everybody.
Users often booting into snapshots can set the default to be writable and other users are protected against accidentally destroying their snapshots, but still have the option to edit a snapshot for what ever reason they may need.

At best make this change optional please.

commit 797d8c2 makes it an option in the settings.

[mtwebster]

I'd prefer it keep the current behavior by default, or else there should be a way in the ui to change it (context menu), and something other than the comment field indicating that it's read-only (Maybe an additional tag?)

[mtwebster]

This will need to be guarded by if (opt_btrfs.active) {} or else it throws a runtime warning. What I'd prefer is to always add the widget, but mark it insensitive (grey it out) - like we already do for the btrfs radio button:
https://github.com/linuxmint/timeshift/pull/373/files#diff-4a9255872e366bab354c5ddc6d435d03d84ae8b736856a284092dc137ff21257R101-R104

[emanuc]

This would be a nice improvement in terms of security, as it would make snapshots immutable and protected from accidental writes.

On openSUSE, with Snapper and Btrfs Assistant (which partially relies on Snapper), snapshots are read-only by default.

As for how to present this in the GUI, Btrfs Assistant could be used as a reference:

The user should not need to interact with these settings, because Timeshift should handle the logic of removing the read-only flag when the user restores the selected snapshot.

Ideally, with something like this:

# mv @ @_old
# btrfs subvolume snapshot /mnt/toplevel/patch/snapshot_X /mnt/toplevel/@
# reboot

However, I still ran a quick test:

emanu@Fedora ~ [1]> sudo btrfs subvolume create test
Create subvolume './test'

emanu@Fedora ~> sudo btrfs subvolume snapshot -r test test_ro_snap
Create readonly snapshot of 'test' in 'test_ro_snap'

emanu@Fedora ~> sudo btrfs subvolume snapshot test_ro_snap test_rw
Create snapshot of 'test_ro_snap' in 'test_rw'

emanu@Fedora ~> sudo btrfs property get  test_rw
ro=false

emanu@Fedora ~> sudo btrfs property get test_ro_snap/
ro=true

[mtwebster]

Thanks for the info. It sounds like read-only should just be the norm.

Is there any argument to let a user see this state or modify it in timeshift? Btrfs-assistant seems a bit power-user-oriented, and timeshift is more about simple backups and restores. It doesn't need to become a btrfs management tool (also, it looks like btrfs-assistant is becoming available on more distributions, including Mint).

[emanuc]

Is there any argument to let a user see this state or modify it in timeshift?

Since Timeshift is aimed at ease of use, I see no reason to expose this detail to the user. What a typical Timeshift user does is simply create and restore snapshots.

For the user, this change would have zero impact, while internally it would make snapshots safer.