storaged: btrfs: show btrfs filesystem "root" subvolume as "top-level" #21127
Conversation
|
If we want this italic, it might not be too easy and we have to special case it for btrfs. |
|
Two different naive approaches, without much code modification:
This is assuming that "name" is not translated here, however, and that subvolumes are not able to be called "top-level" manually, and that someone has done so. Is this the best way to do it? I'm sure it's not. It's a way without modifying anything. It might spark some other idea too (perhaps a better way to determine this, as you know the code better), so I'm sharing it. |
There was a problem hiding this comment.
Thanks!
Also, I'm fine if we merge this as-is and treat styling as a follow-up. This is already an improvement.
However: One quick question: Is top-level ever supposed to be translated? It's the btrfs internal name for it, right? Do btrfs docs translate it? Do the command line tools translate it?
|
It's still in English as "top-level", at least in https://wiki.debianforum.de/Btrfs : |
d5ff3ae
to
18b510e
Compare
18b510e
to
ac37d11
Compare
Cockpit renamed the btrfs "root subvolume" to "top-level" to avoid confusion with "/". cockpit-project/cockpit#21127
|
And anaconda PR rhinstaller/anaconda-webui#492 |
ac37d11
to
a52afcd
Compare
There was a problem hiding this comment.
Looks good to me.
Styling "top-level" differently than other names might need some work in pages.jsx. I think we assume that page.name is a string, and can't be a React component. We might need to allow that, or invent page.name_className to put some custom classes into the DOM.
Showing this as "/" confuses users as they might think it is the root partition and official btrfs terminology names it "top-level". https://btrfs.readthedocs.io/en/latest/Subvolumes.html Relates: cockpit-project#19920
When re-trying non-destructive tests the testFstabOptions test fails as /etc/fstab is not restored to its pristine state. CoreOS does not run storage tests so should not restore fstab/crypttab and the ws-container scenario should not stop cockpit as it isn't installed on the host itself.
a52afcd
to
1ed3f41
Compare
| @@ -452,7 +462,7 @@ const BtrfsSubvolumeCard = ({ card, volume, subvol, snapshot_origin, mismount_wa | |||
| backing_block={block} content_block={block} subvol={subvol} />}> | |||
| <CardBody> | |||
| <DescriptionList className="pf-m-horizontal-on-sm"> | |||
| <StorageDescription title={_("Name")} value={subvol.pathname} /> | |||
| <StorageDescription title={_("Name")} value={subvol.id === 5 ? "top-level" : subvol.pathname} /> | |||
There was a problem hiding this comment.
This added line is not executed by any test.
Showing this as "/" confuses users as they might think it is the root partition and official btrfs terminology names it "top-level".
https://btrfs.readthedocs.io/en/latest/Subvolumes.html
Relates: #19920