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.

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

storage: Improve handling of LUKS backed btrfs #20070

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 18 additions & 9 deletions pkg/storaged/block/create-pages.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import { make_swap_card } from "../swap/swap.jsx";
import { make_encryption_card } from "../crypto/encryption.jsx";
import { make_btrfs_device_card } from "../btrfs/device.jsx";
import { make_btrfs_filesystem_card } from "../btrfs/filesystem.jsx";
import { make_btrfs_subvolume_pages } from "../btrfs/subvolume.jsx";
import { make_btrfs_subvolume_pages, make_btrfs_subvolume_pages_from_child_config } from "../btrfs/subvolume.jsx";
import { find_btrfs_volume } from "../btrfs/utils.jsx";

import { new_page } from "../pages.jsx";

Expand All @@ -58,8 +59,8 @@ export function make_block_page(parent, block, card) {
const is_btrfs = (fstab_config.length > 0 &&
(fstab_config[2].indexOf("subvol=") >= 0 || fstab_config[2].indexOf("subvolid=") >= 0));

const block_btrfs_blockdev = content_block && client.blocks_fsys_btrfs[content_block.path];
const single_device_volume = block_btrfs_blockdev && block_btrfs_blockdev.data.num_devices === 1;
const btrfs_vol = find_btrfs_volume(block);
const single_device_volume = !btrfs_vol || btrfs_vol.data.num_devices <= 1;

if (client.blocks_ptable[block.path]) {
make_partition_table_page(parent, block, card);
Expand All @@ -85,8 +86,14 @@ export function make_block_page(parent, block, card) {
// can not happen unless there is a bug in the code above.
console.error("Assertion failure: is_crypto == false");
}
if (fstab_config.length > 0 && !is_btrfs) {
card = make_filesystem_card(card, block, null, fstab_config);
if (fstab_config.length > 0) {
if (is_btrfs) {
if (single_device_volume)
card = make_btrfs_filesystem_card(card, block, null);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this requires investigation.

else
card = make_locked_encrypted_data_card(card, block);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition confuses me as a reader, if it's not a single device volume it is encrypted?

I understand that we don't know if a locked without fstab?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always encrypted and locked when we get here (content_block == null, is_crypto == true), but if we know (or guess) that it's a single device btrfs, we can make a filesystem card for it even when it is locked.

} else
card = make_filesystem_card(card, block, null, fstab_config);
} else {
card = make_locked_encrypted_data_card(card, block);
}
Expand All @@ -95,11 +102,11 @@ export function make_block_page(parent, block, card) {
const block_pvol = client.blocks_pvol[content_block.path];
const block_swap = client.blocks_swap[content_block.path];

if (block_btrfs_blockdev) {
if (btrfs_vol) {
if (single_device_volume)
card = make_btrfs_filesystem_card(card, block, content_block);
else
card = make_btrfs_device_card(card, block, content_block, block_btrfs_blockdev);
card = make_btrfs_device_card(card, block, content_block, btrfs_vol);
} else if (is_filesystem) {
card = make_filesystem_card(card, block, content_block, fstab_config);
} else if ((content_block.IdUsage == "raid" && content_block.IdType == "LVM2_member") ||
Expand All @@ -122,8 +129,10 @@ export function make_block_page(parent, block, card) {

if (card) {
const page = new_page(parent, card);
if (block_btrfs_blockdev && single_device_volume)
make_btrfs_subvolume_pages(page, block_btrfs_blockdev);
if (btrfs_vol && single_device_volume)
make_btrfs_subvolume_pages(page, btrfs_vol);
else if (!content_block && is_btrfs && single_device_volume)
make_btrfs_subvolume_pages_from_child_config(page, block);
Fixed Show fixed Hide fixed
return page;
}
}
27 changes: 16 additions & 11 deletions pkg/storaged/btrfs/filesystem.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ import { DescriptionList } from "@patternfly/react-core/dist/esm/components/Desc
import {
new_card, ChildrenTable, StorageCard, StorageDescription
} from "../pages.jsx";
import { format_dialog } from "../block/format-dialog.jsx";
import { StorageUsageBar, StorageLink } from "../storage-controls.jsx";
import { btrfs_device_usage, btrfs_is_volume_mounted } from "./utils.jsx";
import { btrfs_device_actions } from "./device.jsx";
import { rename_dialog } from "./volume.jsx";

const _ = cockpit.gettext;
Expand All @@ -43,17 +43,18 @@ export function make_btrfs_filesystem_card(next, backing_block, content_block) {
return new_card({
title: _("btrfs filesystem"),
next,
actions: btrfs_device_actions(backing_block, content_block),
actions: [
{ title: _("Format"), action: () => format_dialog(client, backing_block.path), danger: true },
],
component: BtrfsFilesystemCard,
props: { backing_block, content_block },
});
}

const BtrfsFilesystemCard = ({ card, backing_block, content_block }) => {
const block_btrfs = client.blocks_fsys_btrfs[content_block.path];
const block_btrfs = content_block && client.blocks_fsys_btrfs[content_block.path];
const uuid = block_btrfs && block_btrfs.data.uuid;
const label = block_btrfs && block_btrfs.data.label;
const use = btrfs_device_usage(client, uuid, block_btrfs.path);

// Changing the label is only supported when the device is not mounted
// otherwise we will get btrfs filesystem error ERROR: device /dev/vda5 is
Expand All @@ -66,18 +67,22 @@ const BtrfsFilesystemCard = ({ card, backing_block, content_block }) => {
<StorageCard card={card}>
<CardBody>
<DescriptionList className="pf-m-horizontal-on-sm">
{ block_btrfs &&
<StorageDescription title={_("Label")}
value={label}
action={
<StorageLink onClick={() => rename_dialog(block_btrfs, label)}
excuse={is_mounted ? _("Btrfs volume is mounted") : null}>
{_("edit")}
</StorageLink>}
value={label}
action={
<StorageLink onClick={() => rename_dialog(block_btrfs, label)}
excuse={is_mounted ? _("Btrfs volume is mounted") : null}>
{_("edit")}
</StorageLink>}
/>
}
{ content_block &&
<StorageDescription title={_("UUID")} value={content_block.IdUUID} />
}
{ block_btrfs &&
<StorageDescription title={_("Usage")}>
<StorageUsageBar key="s" stats={use} />
<StorageUsageBar key="s" stats={btrfs_device_usage(client, uuid, block_btrfs.path)} />
</StorageDescription>
}
</DescriptionList>
Expand Down