storage: Allow creation of swap and maintain fstab entries #19843

mvollmer · 2024-01-15T11:30:17Z

Storage: improved support for swap

Cockpit can now format block devices as swap and will maintain a fstab entry for them.

mvollmer · 2024-01-15T11:33:13Z

@marusak For excellent swap support also in the normal Cockpit mode, we would need to write entries for swap devices into fstab, maintain their "noauto" option depending on whether the device is active, do some "misswapping" warnings, and remove the entry during teardown. That's entirely possible, but might not be worth the effort. Opinions?

jelly · 2024-01-15T13:04:44Z

@marusak For excellent swap support also in the normal Cockpit mode, we would need to write entries for swap devices into fstab, maintain their "noauto" option depending on whether the device is active, do some "misswapping" warnings, and remove the entry during teardown. That's entirely possible, but might not be worth the effort. Opinions?

In my own opinion I would like this to be fully supported in Cockpit (even if it is dying tech most distro's nowadays only use zram/zswap). Keeping the amount of custom Anaconda features low would a good thing to strive for in my opinion.

mvollmer · 2024-01-15T16:51:10Z

In my own opinion I would like this to be fully supported in Cockpit (even if it is dying tech most distro's nowadays only use zram/zswap).

Yes, that's the right thing to do(tm), let's do it.

jelly · 2024-01-17T09:19:54Z

Some issues I found:

Creating a swap partition on /dev/sda does not work (no partition table) this should be a valid use case:

[root@fedora-39-127-0-0-2-2201 ~]# mkswap /dev/sda
Setting up swapspace version 1, size = 4 GiB (4294963200 bytes)
no label, UUID=934afe18-e205-4bfe-96f2-ec4a76edd783

Swap on LVM suffers the same issue
Swap on stratis?

mvollmer · 2024-01-17T11:10:13Z

Creating a swap partition on /dev/sda does not work (no partition table)

Oh, you mean there is a JavaScript exception? Good catch! Fixed!

mvollmer · 2024-01-17T11:10:56Z

Swap on stratis?

That's not possible, stratis only has filesystems, no?

jelly · 2024-01-17T11:18:31Z

Swap on stratis?

That's not possible, stratis only has filesystems, no?

Ah ok, then nvm!

jelly · 2024-01-17T11:18:55Z

Creating a swap partition on /dev/sda does not work (no partition table)

Oh, you mean there is a JavaScript exception? Good catch! Fixed!

Yup, blocks_ptable doesn't exist for a plain /dev/sda. Thanks!

jelly

Looks good, one comment about a potential missing test.

test/verify/check-storage-swap

jelly · 2024-01-18T10:03:49Z

Just realized this is release note worthy so marked it as such.

jelly · 2024-01-18T10:33:45Z

test/verify/check-storage-swap

+        m.execute(f"mkswap -f {disk}")
+        b.click(self.card_button("Swap", "Start"))
+        b.wait_text(self.card_desc("Swap", "Used"), "0")
+        self.assertIn("defaults", m.execute(f"findmnt --fstab -n -o OPTIONS {disk}"))


This check seems to be a bit racy fails on both Arch and debian-testing

Yes, clicking "Start" will first activate the Swap and then add the fstab entry. So it is feasible that the "Used" number appears before the fstab entry. I changed this into a testlib.wait.

jelly

Unfortunately one part of the test seems to be flaky.

jelly

Approved, re-triggered rhel-8-10-distropkg/storage hopefully it was just a busy test vm.

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/block/format-dialog.jsx

@@ -423,6 +429,8 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
            else if (trigger == "type") {
                if (dlg.get_value("type") == "empty") {
                    dlg.update_actions({ Variants: action_variants_for_empty });
+                } else if (dlg.get_value("type") == "swap") {


This added line is not executed by any test. Details

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/block/format-dialog.jsx

@@ -450,6 +458,12 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
                    partition_type = "21686148-6449-6e6f-744e-656564454649";
                }

+                if (type == "swap") {
+                    partition_type = (block_ptable && block_ptable.Type == "dos"
+                        ? "0x82"


This added line is not executed by any test. Details

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/block/format-dialog.jsx

+                    if (keep_keys) {
+                        const content_block = client.blocks_cleartext[path];
+                        return client.blocks_swap[content_block.path];
+                    } else if (is_encrypted(vals))


These 4 added lines are not executed by any test. Details

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/block/format-dialog.jsx

+                        return client.blocks_swap[content_block.path];
+                    } else if (is_encrypted(vals))
+                        return (client.blocks_cleartext[path] &&
+                                client.blocks_swap[client.blocks_cleartext[path].path]);


This added line is not executed by any test. Details

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/block/format-dialog.jsx

+                    if (type == "swap" && mount_now)
+                        return (client.wait_for(() => block_swap_for_block(path))
+                                .then(block_swap => block_swap.Start({})));
+                    if (is_encrypted(vals) && (is_filesystem(vals) || type == "swap") && !mount_now)


This added line is not executed by any test. Details

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/block/resize.jsx

@@ -284,6 +284,12 @@ export function get_resize_info(client, block, to_fit) {
                grow_needs_unmount: false
            };
            shrink_excuse = _("VDO backing devices can not be made smaller");
+        } else if (client.blocks_swap[block.path]) {


This added line is not executed by any test. Details

cockpituous · 2024-01-19T08:08:49Z

pkg/storaged/swap/swap.jsx

+        ["fstab", {
+            dir: { t: 'ay', v: encode_filename("none") },
+            type: { t: 'ay', v: encode_filename("swap") },
+            opts: { t: 'ay', v: encode_filename(noauto ? "noauto" : "defaults") },


This added line is not executed by any test. Details

cockpituous · 2024-01-19T08:08:50Z

pkg/storaged/utils.js

@@ -910,6 +911,15 @@ export function get_active_usage(client, path, top_action, child_action, is_temp
                    enter_unmount(children[c], c, false);
                enter_unmount(block, mp, true);
            });
+        } else if (swap) {


This added line is not executed by any test. Details

mvollmer force-pushed the storage-make-swap branch from fa2fa91 to 1143d40 Compare January 15, 2024 11:30

mvollmer force-pushed the storage-make-swap branch from 1143d40 to f90ef17 Compare January 15, 2024 12:43

mvollmer marked this pull request as ready for review January 15, 2024 12:43

mvollmer requested a review from marusak January 15, 2024 12:44

mvollmer marked this pull request as draft January 15, 2024 16:50

mvollmer force-pushed the storage-make-swap branch 2 times, most recently from a81d732 to 3ab49c7 Compare January 16, 2024 09:07

mvollmer changed the title ~~storage: Allow formatting as swap in Anaconda mode~~ storage: Allow creation of swap and maintain fstab entries Jan 16, 2024

mvollmer force-pushed the storage-make-swap branch from 3ab49c7 to 80fc371 Compare January 16, 2024 11:49

mvollmer marked this pull request as ready for review January 16, 2024 13:17

mvollmer force-pushed the storage-make-swap branch 3 times, most recently from db745eb to 6aab047 Compare January 17, 2024 08:41

mvollmer force-pushed the storage-make-swap branch from 6aab047 to 70e1404 Compare January 17, 2024 11:15

jelly previously approved these changes Jan 17, 2024

View reviewed changes

test/verify/check-storage-swap Show resolved Hide resolved

mvollmer dismissed jelly’s stale review via b4f40dc January 18, 2024 08:34

mvollmer force-pushed the storage-make-swap branch 2 times, most recently from b4f40dc to ffd6b83 Compare January 18, 2024 08:55

jelly added the release-note label Jan 18, 2024

jelly reviewed Jan 18, 2024

View reviewed changes

jelly requested changes Jan 18, 2024

View reviewed changes

storage: Allow creation of swap and maintain fstab entries

e6fc1c3

mvollmer force-pushed the storage-make-swap branch from ffd6b83 to e6fc1c3 Compare January 18, 2024 10:43

mvollmer requested a review from jelly January 18, 2024 12:30

jelly approved these changes Jan 18, 2024

View reviewed changes

cockpituous reviewed Jan 19, 2024

View reviewed changes

jelly merged commit 42522f9 into cockpit-project:main Jan 19, 2024
92 of 93 checks passed

mvollmer removed the release-note label Jan 31, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storage: Allow creation of swap and maintain fstab entries #19843

storage: Allow creation of swap and maintain fstab entries #19843

mvollmer commented Jan 15, 2024 •

edited

Loading

mvollmer commented Jan 15, 2024

jelly commented Jan 15, 2024

mvollmer commented Jan 15, 2024

jelly commented Jan 17, 2024 •

edited

Loading

mvollmer commented Jan 17, 2024

mvollmer commented Jan 17, 2024

jelly commented Jan 17, 2024

jelly commented Jan 17, 2024

jelly left a comment

jelly commented Jan 18, 2024

jelly Jan 18, 2024

mvollmer Jan 18, 2024

jelly left a comment

jelly left a comment

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

cockpituous Jan 19, 2024

storage: Allow creation of swap and maintain fstab entries #19843

storage: Allow creation of swap and maintain fstab entries #19843

Conversation

mvollmer commented Jan 15, 2024 • edited Loading

Storage: improved support for swap

mvollmer commented Jan 15, 2024

jelly commented Jan 15, 2024

mvollmer commented Jan 15, 2024

jelly commented Jan 17, 2024 • edited Loading

mvollmer commented Jan 17, 2024

mvollmer commented Jan 17, 2024

jelly commented Jan 17, 2024

jelly commented Jan 17, 2024

jelly left a comment

Choose a reason for hiding this comment

jelly commented Jan 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jelly left a comment

Choose a reason for hiding this comment

jelly left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvollmer commented Jan 15, 2024 •

edited

Loading

jelly commented Jan 17, 2024 •

edited

Loading