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: Remount filesystems after resizing them #19418

Merged
merged 1 commit into from
Oct 24, 2023
Merged

storage: Remount filesystems after resizing them #19418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 2 additions & 22 deletions pkg/storaged/fsys-tab.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Flex, FlexItem } from "@patternfly/react-core/dist/esm/layouts/Flex/ind

import cockpit from "cockpit";
import * as utils from "./utils.js";
import { parse_options, unparse_options, extract_option, set_crypto_options, set_crypto_auto_option } from "./utils.js";
import { parse_options, unparse_options, extract_option, set_crypto_options, set_crypto_auto_option, get_fstab_config_with_client } from "./utils.js";

import {
dialog_open, TextInput, PassInput, CheckBoxes, SelectOne,
Expand Down Expand Up @@ -52,27 +52,7 @@ export function is_mounted(client, block) {
}

export function get_fstab_config(block, also_child_config) {
let config = block.Configuration.find(c => c[0] == "fstab");

if (!config && also_child_config && client.blocks_crypto[block.path])
config = client.blocks_crypto[block.path]?.ChildConfiguration.find(c => c[0] == "fstab");

if (config && utils.decode_filename(config[1].type.v) != "swap") {
const mnt_opts = utils.get_block_mntopts(config[1]).split(",");
let dir = utils.decode_filename(config[1].dir.v);
let opts = mnt_opts
.filter(function (s) { return s.indexOf("x-parent") !== 0 })
.join(",");
const parents = mnt_opts
.filter(function (s) { return s.indexOf("x-parent") === 0 })
.join(",");
if (dir[0] != "/")
dir = "/" + dir;
if (opts == "defaults")
opts = "";
return [config, dir, opts, parents];
} else
return [];
return get_fstab_config_with_client(client, block, also_child_config);
}

function find_blocks_for_mount_point(client, mount_point, self) {
Expand Down
15 changes: 11 additions & 4 deletions pkg/storaged/resize.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import cockpit from "cockpit";
import {
block_name, get_active_usage, teardown_active_usage,
is_mounted_synch, get_partitions
undo_temporary_teardown, is_mounted_synch, get_partitions
} from "./utils.js";
import {
existing_passphrase_fields, init_existing_passphrase,
Expand Down Expand Up @@ -255,7 +255,10 @@ export function grow_dialog(client, lvol_or_part, info, to_fit) {
round_size = 1024 * 1024;
}

const usage = get_active_usage(client, block && info.grow_needs_unmount ? block.path : null, _("grow"));
const usage = get_active_usage(client,
block && info.grow_needs_unmount ? block.path : null,
_("grow"), null,
true);

if (usage.Blocking) {
dialog_open({
Expand Down Expand Up @@ -304,6 +307,7 @@ export function grow_dialog(client, lvol_or_part, info, to_fit) {
to_fit ? grow_size : vals.size,
info.grow_needs_unmount,
vals.passphrase || recovered_passphrase)
.then(() => undo_temporary_teardown(client, usage))
.catch(request_passphrase_on_error_handler(dlg, vals, recovered_passphrase, block)));
});
}
Expand Down Expand Up @@ -336,8 +340,10 @@ export function shrink_dialog(client, lvol_or_part, info, to_fit) {
round_size = 1024 * 1024;
}

const usage = get_active_usage(client, block && !to_fit && info.shrink_needs_unmount ? block.path : null,
_("shrink"));
const usage = get_active_usage(client,
block && !to_fit && info.shrink_needs_unmount ? block.path : null,
_("shrink"), null,
true);

if (usage.Blocking) {
dialog_open({
Expand Down Expand Up @@ -413,6 +419,7 @@ export function shrink_dialog(client, lvol_or_part, info, to_fit) {
to_fit ? shrink_size : vals.size,
to_fit ? false : info.shrink_needs_unmount,
vals.passphrase || recovered_passphrase)
.then(() => undo_temporary_teardown(client, usage))
.catch(request_passphrase_on_error_handler(dlg, vals, recovered_passphrase, block)));
});
}
Expand Down
43 changes: 40 additions & 3 deletions pkg/storaged/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,31 @@ export function find_children_for_mount_point(client, mount_point, self) {
return children;
}

export function get_active_usage(client, path, top_action, child_action) {
export function get_fstab_config_with_client(client, block, also_child_config) {
let config = block.Configuration.find(c => c[0] == "fstab");

if (!config && also_child_config && client.blocks_crypto[block.path])
config = client.blocks_crypto[block.path]?.ChildConfiguration.find(c => c[0] == "fstab");

if (config && decode_filename(config[1].type.v) != "swap") {
const mnt_opts = get_block_mntopts(config[1]).split(",");
let dir = decode_filename(config[1].dir.v);
let opts = mnt_opts
.filter(function (s) { return s.indexOf("x-parent") !== 0 })
.join(",");
const parents = mnt_opts
.filter(function (s) { return s.indexOf("x-parent") === 0 })
.join(",");
if (dir[0] != "/")
dir = "/" + dir;
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

if (opts == "defaults")
opts = "";
return [config, dir, opts, parents];
} else
return [];
}

export function get_active_usage(client, path, top_action, child_action, is_temporary) {
function get_usage(usage, path, level) {
const block = client.blocks[path];
const fsys = client.blocks_fsys[path];
Expand All @@ -791,6 +815,9 @@ export function get_active_usage(client, path, top_action, child_action) {
}

function enter_unmount(block, location, is_top) {
const [, mount_point] = get_fstab_config_with_client(client, block);
Copy link
Member

Choose a reason for hiding this comment

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

Seeing that get_fstab_config_with_client can return [], this leads to mount_point being undefined, it might be better to use === below. But looking at it, it should do the right thing currently.

const has_fstab_entry = is_temporary && location == mount_point;

for (const u of usage) {
if (u.usage == 'mounted' && u.location == location) {
if (is_top) {
Expand All @@ -805,8 +832,9 @@ export function get_active_usage(client, path, top_action, child_action) {
block,
usage: 'mounted',
location,
set_noauto: !is_top,
actions: is_top ? get_actions(_("unmount")) : [_("unmount")],
has_fstab_entry,
set_noauto: !is_top && !is_temporary,
actions: (is_top ? get_actions(_("unmount")) : [_("unmount")]).concat(has_fstab_entry ? [_("mount")] : []),
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

blocking: false
});
}
Expand Down Expand Up @@ -967,6 +995,15 @@ export function teardown_active_usage(client, usage) {
));
}

export async function undo_temporary_teardown(client, usage) {
for (let i = usage.length - 1; i >= 0; i--) {
const u = usage[i];
if (u.usage == "mounted" && u.has_fstab_entry) {
await client.mount_at(u.block, u.location);
}
}
}

// TODO - generalize this to arbitrary number of arguments (when needed)
export function fmt_to_array(fmt, arg) {
const index = fmt.indexOf("$0");
Expand Down
8 changes: 1 addition & 7 deletions test/verify/check-storage-resize
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ class TestStorageResize(storagelib.StorageCase):
self.dialog_apply()
self.dialog_wait_close()
self.content_tab_wait_in_info(1, 1, "Size", "398 MB")
if grow_needs_unmount:
self.content_row_action(fsys_row, "Mount")
self.confirm()
with b.wait_timeout(30):
self.wait_mounted(fsys_row, fsys_tab)
size = int(m.execute("df -k --output=size /run/foo | tail -1").strip())
Expand All @@ -100,10 +97,7 @@ class TestStorageResize(storagelib.StorageCase):
self.dialog_apply()
self.dialog_wait_close()
self.content_tab_wait_in_info(1, 1, "Size", "201 MB")
if shrink_needs_unmount:
self.content_row_action(fsys_row, "Mount")
self.confirm()
self.wait_mounted(fsys_row, fsys_tab)
self.wait_mounted(fsys_row, fsys_tab)
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we verify that the mount options are the same?

size = int(m.execute("df -k --output=size /run/foo | tail -1").strip())
self.assertLess(size, 300000)
else:
Expand Down