From 872713b260d8b969d67443d52eb970448232d123 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Tue, 7 Nov 2023 15:18:02 +0200 Subject: [PATCH] storage: Disable "Apply" in Teardown dialogs after error We want the user to cancel and start over. --- pkg/storaged/content-views.jsx | 2 ++ pkg/storaged/dialog.jsx | 3 ++- pkg/storaged/format-dialog.jsx | 1 + pkg/storaged/fsys-tab.jsx | 1 + pkg/storaged/mdraid-details.jsx | 2 ++ pkg/storaged/resize.jsx | 2 ++ pkg/storaged/stratis-details.jsx | 2 ++ pkg/storaged/vdo-details.jsx | 2 ++ pkg/storaged/vgroup-details.jsx | 2 ++ test/common/storagelib.py | 3 +++ test/verify/check-storage-used | 42 ++++++++++++++++++++++++++++++++ 11 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/storaged/content-views.jsx b/pkg/storaged/content-views.jsx index c88ec2865c6..9f62be6b3a5 100644 --- a/pkg/storaged/content-views.jsx +++ b/pkg/storaged/content-views.jsx @@ -433,6 +433,7 @@ function create_tabs(client, target, options) { Action: { Danger: danger, Title: _("Delete"), + disable_on_error: usage.Teardown, action: function () { return utils.teardown_active_usage(client, usage) .then(function () { @@ -752,6 +753,7 @@ function format_disk(client, block) { Title: _("Initialize"), Danger: _("Initializing erases all data on a disk."), wrapper: job_progress_wrapper(client, block.path), + disable_on_error: usage.Teardown, action: function (vals) { const options = { 'tear-down': { t: 'b', v: true } diff --git a/pkg/storaged/dialog.jsx b/pkg/storaged/dialog.jsx index e437fbdfb63..e5aa408f5f7 100644 --- a/pkg/storaged/dialog.jsx +++ b/pkg/storaged/dialog.jsx @@ -400,6 +400,7 @@ export const dialog_open = (def) => { } errors = errs; update(); + update_footer(); return Promise.reject(); }); }; @@ -414,7 +415,7 @@ export const dialog_open = (def) => { caption: def.Action.Title, style: "primary", danger: def.Action.Danger || def.Action.DangerButton, - disabled: running_promise != null, + disabled: running_promise != null || (def.Action.disable_on_error && errors), clicked: progress_callback => run_action(progress_callback, null), } ]; diff --git a/pkg/storaged/format-dialog.jsx b/pkg/storaged/format-dialog.jsx index 2da510c65df..ce7d2369f43 100644 --- a/pkg/storaged/format-dialog.jsx +++ b/pkg/storaged/format-dialog.jsx @@ -396,6 +396,7 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended, Danger: (create_partition ? null : _("Formatting erases all data on a storage device.")), Variants: [action_variant], wrapper: job_progress_wrapper(client, block.path, client.blocks_cleartext[block.path]?.path), + disable_on_error: usage.Teardown, action: function (vals) { const mount_now = vals.variant != "nomount"; diff --git a/pkg/storaged/fsys-tab.jsx b/pkg/storaged/fsys-tab.jsx index 6b4fe228d3c..2638e4650c7 100644 --- a/pkg/storaged/fsys-tab.jsx +++ b/pkg/storaged/fsys-tab.jsx @@ -414,6 +414,7 @@ export function mounting_dialog(client, block, mode, forced_options) { }, Action: { Title: mode_action[mode], + disable_on_error: usage.Teardown, action: function (vals) { if (mode == "unmount") { return do_unmount(); diff --git a/pkg/storaged/mdraid-details.jsx b/pkg/storaged/mdraid-details.jsx index 9d11a27dd0f..88dd7e8e66e 100644 --- a/pkg/storaged/mdraid-details.jsx +++ b/pkg/storaged/mdraid-details.jsx @@ -180,6 +180,7 @@ function mdraid_stop(client, mdraid) { Teardown: TeardownMessage(usage), Action: { Title: _("Stop device"), + disable_on_error: true, action: function () { return utils.teardown_active_usage(client, usage) .then(function () { @@ -235,6 +236,7 @@ function mdraid_delete(client, mdraid) { Action: { Title: _("Delete"), Danger: _("Deleting erases all data on a RAID device."), + disable_on_error: usage.Teardown, action: function () { return utils.teardown_active_usage(client, usage) .then(delete_) diff --git a/pkg/storaged/resize.jsx b/pkg/storaged/resize.jsx index ff2cb3401df..9711e71b7fc 100644 --- a/pkg/storaged/resize.jsx +++ b/pkg/storaged/resize.jsx @@ -428,6 +428,7 @@ export function grow_dialog(client, lvol_or_part, info, to_fit) { }, Action: { Title: _("Grow"), + disable_on_error: usage.Teardown, action: function (vals) { return teardown_active_usage(client, usage) .then(function () { @@ -541,6 +542,7 @@ export function shrink_dialog(client, lvol_or_part, info, to_fit) { Fields: size_fields.concat(passphrase_fields), Action: { Title: _("Shrink"), + disable_on_error: usage.Teardown, action: function (vals) { return teardown_active_usage(client, usage) .then(function () { diff --git a/pkg/storaged/stratis-details.jsx b/pkg/storaged/stratis-details.jsx index 2fde5b647dc..366a3586fcf 100644 --- a/pkg/storaged/stratis-details.jsx +++ b/pkg/storaged/stratis-details.jsx @@ -408,6 +408,7 @@ export function stratis_content_rows(client, pool, options) { Action: { Danger: _("Deleting a filesystem will delete all data in it."), Title: _("Delete"), + disable_on_error: usage.Teardown, action: function () { return teardown_active_usage(client, usage) .then(() => destroy_filesystem(client, fsys)); @@ -594,6 +595,7 @@ function delete_pool(client, pool) { Action: { Danger: _("Deleting a Stratis pool will erase all data it contains."), Title: _("Delete"), + disable_on_error: usage.Teardown, action: function () { return teardown_active_usage(client, usage) .then(() => destroy_pool(client, pool)) diff --git a/pkg/storaged/vdo-details.jsx b/pkg/storaged/vdo-details.jsx index d22ae17c2c4..0709d1bcb6c 100644 --- a/pkg/storaged/vdo-details.jsx +++ b/pkg/storaged/vdo-details.jsx @@ -138,6 +138,7 @@ export class VDODetails extends React.Component { Teardown: TeardownMessage(usage), Action: { Title: _("Stop"), + disable_on_error: usage.Teardown, action: function () { return teardown_active_usage(client, usage) .then(function () { @@ -199,6 +200,7 @@ export class VDODetails extends React.Component { Action: { Title: _("Delete"), Danger: _("Deleting erases all data on a VDO device."), + disable_on_error: usage.Teardown, action: function () { return (teardown_active_usage(client, usage) .then(teardown_configs) diff --git a/pkg/storaged/vgroup-details.jsx b/pkg/storaged/vgroup-details.jsx index 83b5ab872c6..77841377053 100644 --- a/pkg/storaged/vgroup-details.jsx +++ b/pkg/storaged/vgroup-details.jsx @@ -173,6 +173,7 @@ export function vgroup_delete(client, vgroup) { Action: { Danger: _("Deleting erases all data on a volume group."), Title: _("Delete"), + disable_on_error: usage.Teardown, action: function () { return utils.teardown_active_usage(client, usage) .then(function () { @@ -284,6 +285,7 @@ export class VGroupDetails extends React.Component { Teardown: TeardownMessage(usage), Action: { Title: _("Remove"), + disable_on_error: usage.Teardown, action: function () { return utils.teardown_active_usage(client, usage) .then(function () { diff --git a/test/common/storagelib.py b/test/common/storagelib.py index 021fca12a30..b1672c79144 100644 --- a/test/common/storagelib.py +++ b/test/common/storagelib.py @@ -317,6 +317,9 @@ def dialog_wait_not_present(self, field): def dialog_wait_apply_enabled(self): self.browser.wait_attr('#dialog button.apply:nth-of-type(1)', "disabled", None) + def dialog_wait_apply_disabled(self): + self.browser.wait_visible('#dialog button.apply:nth-of-type(1)[disabled]') + def dialog_apply(self): self.browser.click('#dialog button.apply:nth-of-type(1)') diff --git a/test/verify/check-storage-used b/test/verify/check-storage-used index 0b2e0ee85c6..2e25295aab9 100755 --- a/test/verify/check-storage-used +++ b/test/verify/check-storage-used @@ -133,6 +133,48 @@ ExecStart=/usr/bin/sleep infinity self.assertEqual(m.execute("vgs TEST1 || echo GONE").strip(), "GONE") + def testTeardownRetry(self): + m = self.machine + b = self.browser + + self.login_and_go("/storage") + + disk = self.add_ram_disk() + b.wait_in_text("#drives", disk) + m.execute(f"mke2fs -q -L TEST {disk}") + m.execute(f"mount {disk} /mnt") + + b.click(f'.sidepanel-row:contains("{disk}")') + b.wait_visible("#storage-detail") + self.retry_in_content_tab(1, 1, lambda tab: b.wait_in_text(tab, "The filesystem is currently mounted on /mnt")) + + # Start formatting, and while the dialog is open, make the + # filesystem unmountable. + # + # We have two processes that keep the filesystem busy: one + # that is supposed to be picked up by the dialog, and one that + # is not. The first is only used to figure out when the dialog + # is done initializing. + + m.spawn("cd /mnt; sleep infinity; true", "sleep") + + b.click('button:contains(Create partition table)') + self.dialog_wait_open() + b.wait_visible("#dialog tbody:first-of-type button:contains(Currently in use)") + self.dialog_wait_apply_enabled() + m.spawn("cd /mnt; sleep infinity; true", "sleep") + self.dialog_apply() + b.wait_in_text("#dialog", "umount: /mnt: target is busy") + self.dialog_wait_apply_disabled() + self.dialog_cancel() + self.dialog_wait_close() + + b.click('button:contains(Create partition table)') + self.dialog_wait_open() + b.wait_visible("#dialog button:contains(Currently in use)") + self.dialog_apply() + self.dialog_wait_close() + if __name__ == '__main__': testlib.test_main()