-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
storaged: support creating btrfs snapshots #20562
base: main
Are you sure you want to change the base?
Conversation
When you're creating snapshots, you do not want to mount them, or set up auto mounting? I don't understand why mounting is half the dialog. Snapshots of subvolumes are intended as "cheap" "backups". I don't even remember mounting or unmounting them when using them before in the past. They also cannot cross into other subvolumes. I don't think they're even treated like subvolumes in a traditional manner. Isn't btrfs snapshotting basically the following:
Nothing related to mounting or even unmounting. (Whenever I've made a snapshot, I'm able to go into that snapshot immediately without having to do anything special.) If someone really wants to mount a snapshot instead of going into it, they can do that with the standard mount dialog afterward, right? |
So I was thinking we can make snapshotting more like a snapshotting task and less like "random generic subvolume". This would include some nice stuff where it makes snapshotting super easy without "crap work" that someone would have to do each time, and instead have the computer remember and figure out that stuff for them, with good and opinionated defaults. Here's what that could look like: The text in the side says:
|
If you're trying to snapshot the parent subvolumes (e.g. Put more concretely, in Fedora's setup, we have the following setup:
You could do something like this:
This would allow you to have the snapshots not visible most of time, and if you need it for recovery or boot-to-snapshot purposes, it's easily available. |
The Cockpit storage page (currently) shows all subvolumes not based on what is mounted (that is an interesting idea though). There is a draft PR to hide snapshots from the overview page and list them under the subvolume you made a snapshot of. #20234 |
Is this a path in the filesystem? I think it should be the name of the parent subvolume. We could have a dropdown for the parent subvol, and then the radio for the name of the new snapshot. |
Open question is, do we add an option for subvolumename + date? |
2b1674a
to
517ef66
Compare
Btrfs supports creating a snapshot of a subvolume, either readonly or write-able. Unlike creating subvolumes, it doesn't make sense to put a snapshot of a subvolume in the subvolume itself users likely have a special `snapshots` subvolume where they collect their snapshots.
517ef66
to
0babff9
Compare
TextInput("subvolume", _("Subvolume"), | ||
{ | ||
value: subvol.pathname, | ||
disabled: true, |
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.
The is confusing, imo. Usually we put the subject of a dialog into the title, like "Permanently delete subvolume foo/bar?" so I would expect "Create snapshot of foo/bar" as the title instead of this disabled text field.
My immediate thought when seeing it in the dialog was "why is it disabled? what can I do to enable it?" and there are no good answers to those questions, I guess.
The mockups have a static text instead of a disabled textinput, which avoids those questions. I think that's much better.
I like this new pattern of putting the subject of a dialog into the list of input fields, as a static text. However, it's still a new pattern and it feels random to introduce it here, and make this dialog different from all others in this regard.
]; | ||
|
||
const get_local_storage_snapshots_locs = () => { | ||
const localstorage_snapshot_data = localStorage.getItem(localstorage_key); |
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.
What about using the location of the most recently made snapshot? By looking at the list of snapshots of this subvolume. That makes this independent from browser storage.
placeholder: cockpit.format(_("Example, $0"), "/.snapshots"), | ||
explanation: (<> | ||
<p>{_("Snapshots must reside within the same btrfs volume.")}</p> | ||
<p>{_("When the snapshot location does not exist, it will be created as btrfs subvolume automatically.")}</p> |
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.
Hmm, why not as a directory? Also, non existing parents should be created as well, no?
value: subvol.pathname, | ||
disabled: true, | ||
}), | ||
TextInput("snapshots_location", _("Snapshots location"), |
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.
I think this should be a autocomplete thing, offering the mount points of all mounted subvolumes.
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.
This seems unused.
Btrfs supports creating a snapshot of a subvolume, either readonly or write-able. Unlike creating subvolumes, it doesn't make sense to put a snapshot of a subvolume in the subvolume itself users likely have a special
snapshots
subvolume where they collect their snapshots.Open questions:
/snapshots or
/.snapshots`, this is not a well-known directory)Follow up / issues:
Support creating btrfs snapshots
Snapshot creation dialog: