storage: Remount filesystems after resizing them #19418

mvollmer · 2023-10-02T08:08:17Z

No description provided.

jelly · 2023-10-02T08:32:15Z

# test/verify/check-storage-mounting:462:56: E261 at least two spaces before inline comment

jelly · 2023-10-17T09:16:45Z

I just noticed that we support creating a partitionless filesystem so basically mkfs.xfs /dev/sda, but there is no option to resize this filesystem. Is this a missing feature?

NVM: you can't seem to create a half formatted partitionless disk, this would be insanely silly :)

Resizing grow to full (created a separate issue for this)

Resizing XFS to the max gave an error (which I closed...) about the sizes mismatching. It turns out the partition is now resized but the FS is not so a user will end up with:

I can't grow this now, a user will get:

Error resizing filesystem on /dev/sda1: Process reported exit code 1: data size 524028 too large, maximum is 524027

jelly · 2023-10-17T09:57:36Z

Comments related to the PR:

Shrinking /boot/efi does succeed? But re-mounting does not work

fstab looks like:

UUID=7B77-95E7  /boot/efi       vfat    defaults,uid=0,gid=0,umask=077,shortname=winnt  0       2

Actions list does not show it will `mount` after growing

If an error happens during resizing (not enough space or so) we already umounted so the next grow/shrink button click will result in:

Unrelated, we can't grow/shrink a vfat partition to arbitrary sizes

Danger alert:Error resizing filesystem on /dev/vda2: Failed to resize the filesystem on '/dev/vda2' (GNU Parted cannot resize this partition to this size. We're working on it!)

It seems this is about shrinking to a specific size which errors out.

This happens on libblockdev 2.28 on RHEL 9. And the resize code is different from what I am looking at for libblockdev master. So let's re-test this on Fedora 39.

Reproduced on Fedora-39, this is a libblockdev bug, as vfat-resize comes from libblockdev-tools (which we might want on our images). So this error comes from libparted which makes it seems it can't detect the fat type of the partition?

The issue here is that you can only resize vfat in certain cluster sizes. Depending on if it's Fat 16 or Fat 32.

We can shrink to zero,this never makes sense (should create a separate issue for this)

jelly · 2023-10-17T11:40:28Z

test/verify/check-storage-resize

-                self.content_row_action(fsys_row, "Mount")
-                self.confirm()
-                self.wait_mounted(fsys_row, fsys_tab)
+            self.wait_mounted(fsys_row, fsys_tab)


Ideally we verify that the mount options are the same?

jelly

When any resize action fails, either due to space constraints or something else we already umounted the FS and changing the size and then trying to resize will fail as Cockpit errors out with "FS is already umounted"

mvollmer · 2023-10-18T06:33:51Z

Shrinking /boot/efi does succeed? But re-mounting does not work

/dev/vda2 is mounted on both /boot/efi and /efi before the resize, but there is no fstab entry for /efi. Do you know why that is? (Why is it mounted twice?)

But we should fix Cockpit in any case:

refuse to resize in the first place when there are mounts without fstab
ignore mounts without fstab entry during unmounting, resizing will likely fail.
do the resize, but ignore failures from remounting
...

I think the first option is best.

mvollmer · 2023-10-18T06:36:11Z

When any resize action fails, either due to space constraints or something else we already umounted the FS and changing the size and then trying to resize will fail as Cockpit errors out with "FS is already umounted"

Yes, that's a old bug...

mvollmer · 2023-10-18T06:39:36Z

We can shrink to zero,this never makes sense (should create a separate issue for this)

Well, this is caught in the dialog before starting any action. But yeah, very small sizes will likely never succeed and Cockpit could prevent that. But since in the case of shrinking, the filesystem is resized first, and if that fails immediately, no harm has been done, no? The only thing to improve is the error message or user experience.

mvollmer · 2023-10-18T06:44:53Z

Reproduced on Fedora-39, this is a libblockdev bug

Thanks for investigating this. Can we do anything about it in Cockpit? Maybe refusing to resize vfat is the best option.

jelly · 2023-10-18T07:19:28Z

TL;DR

On RHEL-9-4 we have this

findmnt
├─/efi systemd-1 autofs rw,relatime,fd=47,pgrp=1,timeout
│ └─/efi /dev/vda2 vfat rw,relatime,fmask=0077,dmask=007
└─/boot /dev/vda3 xfs rw,relatime,seclabel,attr2,inode
└─/boot/efi /dev/vda2 vfat rw,relatime,fmask=0077,dmask=007

Shrinking /boot/efi does succeed? But re-mounting does not work

/dev/vda2 is mounted on both /boot/efi and /efi before the resize, but there is no fstab entry for /efi. Do you know why that is? (Why is it mounted twice?)

This was with a standard Fedora-39 image.

/dev/vda3 on /boot/efi type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=ascii,shortname=winnt,errors=remount-ro)

I recall what I did. I was trying to resize /boot which umount's both /boot/efi and /boot

[root@fedora-39-127-0-0-2-2201 ~]# mount | grep boot
/dev/vda2 on /boot type ext4 (rw,relatime,seclabel)
/dev/vda3 on /boot/efi type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=ascii,shortname=winnt,errors=remount-ro)
[root@fedora-39-127-0-0-2-2201 ~]# grep boot /etc/fstab
UUID=19a5c8b5-13c3-43d2-a59d-c78e275ba9e2 /boot                   ext4    defaults        1 2
UUID=6558-48D7          /boot/efi               vfat    defaults,uid=0,gid=0,umask=077,shortname=winnt 0 2

But now I can't reproduce it on Fedora -39. But on RHEL-9-4 this problem does happen:

[root@rhel-9-4-127-0-0-2-2201 ~]# grep boot /etc/fstab
UUID=5b68f570-1c4e-4953-9308-4bc9f6a3c8f6       /boot   xfs     defaults        0       0
UUID=7B77-95E7  /boot/efi       vfat    defaults,uid=0,gid=0,umask=077,shortname=winnt  0       2
[root@rhel-9-4-127-0-0-2-2201 ~]# grep boot /etc/fstab
[root@rhel-9-4-127-0-0-2-2201 ~]# mount | grep boot
/dev/vda3 on /boot type xfs (rw,relatime,seclabel,attr2,inode64,logbufs=8,logbsize=32k,noquota)
/dev/vda2 on /boot/efi type vfat (rw,relatime,fmask=0077,dmask=0077,codepage=437,iocharset=ascii,shortname=winnt,errors=remount-ro)

Note that the only different is /boot being xfs instead of ext4? This is a rather interesting bug

But we should fix Cockpit in any case:
* refuse to resize in the first place when there are mounts without fstab

Or show a warning that it won't be remounted?

* ignore mounts without fstab entry during unmounting, resizing will likely fail.

* do the resize, but ignore failures from remounting

* ...

I think the first option is best.

jelly · 2023-10-18T07:39:28Z

Reproduced on Fedora-39, this is a libblockdev bug

Thanks for investigating this. Can we do anything about it in Cockpit? Maybe refusing to resize vfat is the best option.

It's not a libblockdev bug per se, well it depends on what you expect from an API. FAT16/32 can only be resized to certain block sizes, maybe xfs too? But libblockdev hard errors if you propose to resize it to 15.5MB or so. I created a follow up issue for this in Cockpit.

mvollmer · 2023-10-23T13:46:36Z

Or show a warning that it won't be remounted?

Yes, that's what I did now, kinda Mounts without fstab entry will be unmounted, but not be remounted. There is no big warning about that, the dialog just doesn't mention that they will be remounted.

mvollmer · 2023-10-23T13:47:54Z

Actions list does not show it will mount after growing

Fixed, see #19418 (comment)

cockpituous · 2023-10-23T14:52:12Z

pkg/storaged/utils.js

+                .filter(function (s) { return s.indexOf("x-parent") === 0 })
+                .join(",");
+        if (dir[0] != "/")
+            dir = "/" + dir;


This added line is not executed by any test. Details

cockpituous · 2023-10-23T14:52:12Z

pkg/storaged/utils.js

-                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")] : []),


This added line is not executed by any test. Details

mvollmer · 2023-10-24T07:16:01Z

Thanks for investigating this. Can we do anything about it in Cockpit? Maybe refusing to resize vfat is the best option.

It's not a libblockdev bug per se, well it depends on what you expect from an API. FAT16/32 can only be resized to certain block sizes, maybe xfs too? But libblockdev hard errors if you propose to resize it to 15.5MB or so. I created a follow up issue for this in Cockpit.

Cockpit should try to predict errors as much as possible and steer the user away from triggering them, but it's always an option to just let the tool fail in obscure ways. So if VFAT has baroque restrictions on what sizes it can be resized to, which depend on the version of parted that we are ultimately using, then I think we can just shrug, and expose the ugly belly of the beast.

jelly · 2023-10-24T08:24:34Z

pkg/storaged/utils.js

@@ -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);


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.

mvollmer added the blocked Don't land until something else happens first (see task list) label Oct 2, 2023

mvollmer requested a review from jelly October 2, 2023 08:19

mvollmer force-pushed the storage-remount-after-resize branch from 7a8bb8a to e8510ce Compare October 5, 2023 07:06

mvollmer removed the blocked Don't land until something else happens first (see task list) label Oct 5, 2023

mvollmer force-pushed the storage-remount-after-resize branch from e8510ce to d1ea16d Compare October 17, 2023 07:33

mvollmer temporarily deployed to gh-cockpituous October 17, 2023 07:33 — with GitHub Actions Inactive

jelly mentioned this pull request Oct 17, 2023

Resizing XFS does not always succeed by an off by one calculation #19493

Open

jelly reviewed Oct 17, 2023

View reviewed changes

jelly requested changes Oct 17, 2023

View reviewed changes

storage: Remount filesystems after resizing them

b6ea463

mvollmer force-pushed the storage-remount-after-resize branch from d1ea16d to b6ea463 Compare October 23, 2023 13:44

mvollmer temporarily deployed to gh-cockpituous October 23, 2023 13:44 — with GitHub Actions Inactive

mvollmer requested a review from jelly October 23, 2023 13:47

cockpituous reviewed Oct 23, 2023

View reviewed changes

jelly approved these changes Oct 24, 2023

View reviewed changes

jelly merged commit 4e34a3c into cockpit-project:main Oct 24, 2023
100 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storage: Remount filesystems after resizing them #19418

storage: Remount filesystems after resizing them #19418

mvollmer commented Oct 2, 2023 •

edited

Loading

jelly commented Oct 2, 2023

jelly commented Oct 17, 2023 •

edited

Loading

jelly commented Oct 17, 2023 •

edited

Loading

jelly Oct 17, 2023

jelly left a comment

mvollmer commented Oct 18, 2023

mvollmer commented Oct 18, 2023

mvollmer commented Oct 18, 2023

mvollmer commented Oct 18, 2023

jelly commented Oct 18, 2023 •

edited

Loading

jelly commented Oct 18, 2023

mvollmer commented Oct 23, 2023

mvollmer commented Oct 23, 2023

cockpituous Oct 23, 2023

cockpituous Oct 23, 2023

mvollmer commented Oct 24, 2023 •

edited

Loading

jelly Oct 24, 2023

storage: Remount filesystems after resizing them #19418

storage: Remount filesystems after resizing them #19418

Conversation

mvollmer commented Oct 2, 2023 • edited Loading

jelly commented Oct 2, 2023

jelly commented Oct 17, 2023 • edited Loading

Resizing grow to full (created a separate issue for this)

jelly commented Oct 17, 2023 • edited Loading

Shrinking /boot/efi does succeed? But re-mounting does not work

Actions list does not show it will mount after growing

If an error happens during resizing (not enough space or so) we already umounted so the next grow/shrink button click will result in:

Unrelated, we can't grow/shrink a vfat partition to arbitrary sizes

We can shrink to zero,this never makes sense (should create a separate issue for this)

jelly Oct 17, 2023

Choose a reason for hiding this comment

jelly left a comment

Choose a reason for hiding this comment

mvollmer commented Oct 18, 2023

mvollmer commented Oct 18, 2023

mvollmer commented Oct 18, 2023

mvollmer commented Oct 18, 2023

jelly commented Oct 18, 2023 • edited Loading

jelly commented Oct 18, 2023

mvollmer commented Oct 23, 2023

mvollmer commented Oct 23, 2023

cockpituous Oct 23, 2023

Choose a reason for hiding this comment

cockpituous Oct 23, 2023

Choose a reason for hiding this comment

mvollmer commented Oct 24, 2023 • edited Loading

jelly Oct 24, 2023

Choose a reason for hiding this comment

mvollmer commented Oct 2, 2023 •

edited

Loading

jelly commented Oct 17, 2023 •

edited

Loading

jelly commented Oct 17, 2023 •

edited

Loading

Actions list does not show it will `mount` after growing

jelly commented Oct 18, 2023 •

edited

Loading

mvollmer commented Oct 24, 2023 •

edited

Loading