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.

[mtwebster]

So, it seems like we can update this PR to make snapshots RO but there's no need to advertise or even make it an option is there? It just boils down to adding a -r to the btrfs command.

[cshandley-uk]

There at least needs to be an option for new snapshots to be created read-write (i.e. even if individual snapshots cannot be modified), so that "grub-btrfs" can be used, otherwise it will prevent the ability to boot into a snapshot.

[emanuc]

So, it seems like we can update this PR to make snapshots RO but there's no need to advertise or even make it an option is there? It just boils down to adding a -r to the btrfs command.

Yes, that’s how it was supposed to work from the beginning.

For those using tools like “grub-btrfs” and similar applications to restore snapshots from GRUB, this can still be handled by switching the snapshots back to read-write (RW).

emanu@fedora ~> sudo btrfs property get /.snapshots/472/snapshot
ro=true

emanu@fedora ~> sudo btrfs property set /.snapshots/472/snapshot ro false

emanu@fedora ~> sudo btrfs property get /.snapshots/472/snapshot
ro=false

[SentineI]

For those using tools like “grub-btrfs” and similar applications to restore snapshots from GRUB, this can still be handled by switching the snapshots back to read-write (RW).

I don't see how that would work. The whole point of grub-btrfs is you can choose to boot a snapshot at boot time (e.g. if the OS fails to boot), while the commands suggested need you to have already booted (never mind working out which snapshot number corresponds to which snapshot).

What's the problem with adding a little tick box in the GUI to choose whether or not the user wants read-writeable snapshots to be created?

[emanuc]

For those using tools like “grub-btrfs” and similar applications to restore snapshots from GRUB, this can still be handled by switching the snapshots back to read-write (RW).

I don't see how that would work. The whole point of grub-btrfs is you can choose to boot a snapshot at boot time (e.g. if the OS fails to boot), while the commands suggested need you to have already booted (never mind working out which snapshot number corresponds to which snapshot).

What's the problem with adding a little tick box in the GUI to choose whether or not the user wants read-writeable snapshots to be created?

openSUSE does exactly this: it boots into a read-only snapshot selected from GRUB, starts the session, and then you decide whether to restore it as the default if it matches your expectations.

However, I don’t think this is feasible on a system that isn’t designed around that layout. Thinking about it, you need a separate subvolume for the entire /var, and with Debian-based distributions this isn’t really possible, because the package manager databases are located under /var, specifically /var/lib/dpkg and /var/lib/apt/. openSUSE and Fedora have addressed this by moving those databases into /usr.

Currently, the RPM databases is located in /var. Let's move it to /usr. The move is already under way in rpm-ostree-based installations, and in (open)SUSE.

That said, I’m not against adding an option for this. Ultimately, it’s up to the maintainers to decide.