Skip to content
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.

Sign up for GitHub

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

Jump to bottom

storage: Action redesign #19882

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from
Draft

storage: Action redesign #19882

wants to merge 15 commits into from

Conversation

mvollmer
Copy link
Member

@mvollmer mvollmer commented Jan 24, 2024 •
edited
Loading

  • Erase dialog: No overwriting
  • Partition table dialog: Use radio buttons
  • Everywhere: drop "/dev/" prefix
  • New demo
    - Start with encrypted xfs on partition
    - Lock/Mount/Unmount a bit
    - Erase the lot
    - Show how to do a expand the "Create partition" dialog step wise to do a lot in one go.
    - Point out that "Unformatted data" and "Locked data" are no longer a thing.
    - Format as ext4 with keeping the encryption.
  • tests, coverage

Action redesign for Storage

Demo: https://youtu.be/BkDCAOvOPVc

Goals:

  • Simplify the "Format" dialog. It does too much.

    Especially, don't require people to figure out that they need to
    use it with type "No filesystem" to wipe a block device before they
    can use it as a LVM2 physical volume, etc.

Design:

  • Have "primary" actions. Only those will be a button in a non-mobile card. Everything else is in the menu.
  • Move "Format" to the card that will not change, like Partition, Drive, Logical volume. This makes more sense.
  • Move "Lock" and "Unlock" to "Encryption" card. This also makes more sense.
  • Remove "Unformatted data" and "Locked data" cards.
  • New "Add encryption" action to create empty LUKS devices. This would only be offered for empty block devices, you can't add encryption to existing filesystems.
  • Offer "Format" also on "Encryption" card and remove "keep" logic from format dialog. (Formatting from Encryption card would not offer encryption options at all.)
  • Move "no partition table" and "no filesystem" into new "Erase" dialog. Don't use "initialize" term anymore. Keep "no filesystem" for "create partition" dialog only, and move it to the top of the dropdown and make it default. Move "Type" after "Size".
  • Only offer "Format" and "Create partition table" for unformatted block devices, offer "Erase" else. Make Format and CreateP non-dangerous and primary actions.
  • Also offer "Create partition" on PartitionCard. This will pick the first free space that is big enough for the requested size. Make it a primary action.
  • Add "Format as swap" action.

@mvollmer mvollmer added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Jan 24, 2024
@garrett garrett mentioned this pull request Jan 25, 2024
Open
@mvollmer mvollmer mentioned this pull request Apr 5, 2024
Open
@mvollmer mvollmer force-pushed the storage-action-redesign branch 3 times, most recently from fa9ef75 to 21ded11 Compare April 5, 2024 14:48
@mvollmer mvollmer requested a review from garrett April 8, 2024 06:42
@mvollmer
Copy link
Member Author

mvollmer commented Apr 8, 2024 •
edited
Loading

@garrett, what do you think of this? There is a demo: https://youtu.be/BkDCAOvOPVc .

One point I didn't make in the demo: The awkward "Unformatted data" and "Locked data" cards are gone.

@jelly
Copy link
Member

jelly commented Apr 8, 2024 •
edited
Loading

Some things from my side:

It looks interesting!

  • Instead of Format as swap, maybe, Create swap partition? Although I guess you can format a whole /dev/sda as swap?

  • I would list both Format actions next each other.

  • Add encryption feels a bit weird, I understand what you mean, but I am not sure if others do. It feels that this action would be to "add encryption" to a Filesystem. Maybe something easier? Encrypt disk, Add disk encryption, I understand that this acts on a disk so maybe it is logical after all. (Also this can handles LVM logical volumes as well so we can't put disk in there). So in the end, maybe it is just fine as is :)

  • Format cleartext device - This is feels like a lot of technical jargon, for a user this just means format $encrypted_device so maybe we can keep it as Format device?

@mvollmer
Copy link
Member Author

mvollmer commented Apr 9, 2024

Here is a mockup from Garrett:

image

@mvollmer
Copy link
Member Author

mvollmer commented Apr 9, 2024 •
edited
Loading

It looks interesting!

Thanks, yes! And I exactly share your concerns!

  • Instead of Format as swap, maybe, Create swap partition? Although I guess you can format a whole /dev/sda as swap?

Hmm. We could add "swap" as a type to the "Create partition dialog". It can do more than filesystems anyway, like "biosboot" etc.

I really wanted to have "Format as filesystem" as the action title, and then we can't have anything else but filesystems in it. And if we want to still allow people to make swap on arbitrary block devices, we need a "Format as swap" action.

  • I would list both Format actions next each other.

Yes, I was going back and forth on the menu order many times, exactly because I wanted the two "Format as ..." next to each other. But "Add encryption" is argueably more important than formatting swap, so... I really don't know.

  • Add encryption feels a bit weird,

Absolutely. I am not happy about the wording.

I understand what you mean, but I am not sure if others do. It feels that this action would be to "add encryption" to a Filesystem. Maybe something easier? Encrypt disk, Add disk encryption, I understand that this acts on a disk so maybe it is logical after all. (Also this can handles LVM logical volumes as well so we can't put disk in there). So in the end, maybe it is just fine as is :)

"Format with encryption only"? This could give the idea that other actions can also use encryption.

  • Format cleartext device - This is feels like a lot of technical jargon, for a user this just means format $encrypted_device so maybe we can keep it as Format device?

"Format as encrypted filesystem"?

"Erase encrypted device"?

This is a good PR for a user study...

@mvollmer mvollmer mentioned this pull request Apr 11, 2024
Merged
@mvollmer
Copy link
Member Author

So I start to have doubts about making "Erase" and "Format" mutually exclusive. It just forces people to do in two steps what the tool can actually do in one. I still think it's good to have a dedicated "Erase" action and dialog, but we shouldn't hide "Format as ..." until people have done the Erase.

github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 17, 2024
View reviewed changes
pkg/storaged/block/erase-dialog.jsx Fixed Show fixed Hide fixed
pkg/storaged/block/erase-dialog.jsx Fixed Show fixed Hide fixed
pkg/storaged/swap/format-dialog.jsx Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 17, 2024
View reviewed changes
pkg/storaged/block/erase-dialog.jsx
Comment on lines +27 to +32
import {
dialog_open,
CheckBoxes,
BlockingMessage, TeardownMessage,
init_teardown_usage,
} from "../dialog.jsx";

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import CheckBoxes.
mvollmer and others added 11 commits September 11, 2024 15:22
@mvollmer
storage: Move all actions into the menu, except primary ones
2618d8f 
And make "Mount" and "Create new logical volume" primary actions.
@mvollmer
storage: Move "Format" action one card up
804b79b 
Offer it on the "Partition" card instead of the "Filesystem" card, for
example.

This makes more sense. One formats a partition, but mounts the
filesystem.
@mvollmer
storage: Move "Lock" and "Unlock" actions to "Encryption" card
4dfcf04 
That's where they belong. Filesystems are still automatically locked
and unlocked when mounting and unmounting them.
@mvollmer
storage: Remove "Unrmatted Data" and "Locked Data" cards
37602fc 
They have no actions anymore.
@mvollmer
storage: New "Add encryption" action for unformatted blocks
1b188d9
@mvollmer
storage: Offer "Format" on "Encryption" card
71603fc
@mvollmer
storage: Remove "Reuse existing encryption" option
02854f7
@mvollmer
storage: Split "Erase" out from "Format" dialogs
cec67e7 
This allows us to make the format dialogs non-dangerous and primary,
and give them more descriptive names.

The "Create partition" version of the "Format" dialog now defaults to
"empty", and "empty" can not be encrypted. This makes it a very simple
dialog.
@mvollmer
storage: Offer global "Create partition" action
c7f54e3
@mvollmer
storage: Move swap creation out of filesystem format dialog
138a1e7
@mvollmer
storage: Remove "Overwrite" option in Erase dialog
0f5983c 
It's not a secure erase by any means.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garrett garrett Awaiting requested review from garrett

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
no-test For doc/workflow changes, or experiments which don't need a full CI run,
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mvollmer @jelly