storage: Partition resizing #19385

mvollmer · 2023-09-25T10:47:56Z

Demo: https://www.youtube.com/watch?v=rZ2ZExzkIto

Storage: Partitions can be resized

Cockpit can now also resize partitions, including their content such as filesystems or LUKS containers.

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/resize.jsx

+        block = client.lvols_block[lvol_or_part.path];
+        name = lvol_or_part.Name;
+        orig_size = lvol_or_part.Size;
+        max_size = pool ? pool.Size * 3 : lvol_or_part.Size + vgroup.FreeSize;


This added line is not executed by any test. Details

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/resize.jsx


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

    if (usage.Blocking) {
        dialog_open({
-            Title: cockpit.format(_("$0 is in use"), lvol.Name),
+            Title: cockpit.format(_("$0 is in use"), name),


This added line is not executed by any test. Details

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/resize.jsx

-                                                         info.grow_needs_unmount,
-                                                         vals.passphrase || recovered_passphrase)
+                            return (lvol_or_part_and_fsys_resize(client, lvol_or_part,
+                                                                 to_fit ? grow_size : vals.size,


This added line is not executed by any test. Details

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/resize.jsx


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

    if (usage.Blocking) {
        dialog_open({
-            Title: cockpit.format(_("$0 is in use"), lvol.Name),
+            Title: cockpit.format(_("$0 is in use"), name),


This added line is not executed by any test. Details

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/resize.jsx

+                                                                 to_fit ? shrink_size : vals.size,
+                                                                 to_fit ? false : info.shrink_needs_unmount,


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

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/warnings.jsx

                content_size: fsys.Size
            });
        }

-        if (vdo && (lvol.Size - vdo.physical_size - crypto_overhead) > vgroup.ExtentSize) {
+        if (vdo && (size - vdo.physical_size - crypto_overhead) > min_change) {


This added line is not executed by any test. Details

cockpituous · 2023-09-28T12:18:00Z

pkg/storaged/warnings.jsx

            enter_warning(path, {
                warning: "unused-space",
-                volume_size: lvol.Size - crypto_overhead,
+                volume_size: size - crypto_overhead,


This added line is not executed by any test. Details

jelly · 2023-09-29T09:55:14Z

Unrelated issues:

BTRFS can't set a label, when mounted, can when unmounted. But I can do it, so this seems to be a udisks/libblockdev bug:

btrfs filesystem label /dev/sda lololol

Works fine and changes in cockpit. Libblockdev also uses this but with a mountpoint! So maybe this is a recent change in btrfs-progs.

jelly

This all looks fairly trivial and codewise it should be fine. I do have some concerns with the functionality itself:

For example I grow a ntfs paritition which is mounted:

Why is it not mounted back, at the end? Or do we never do this for a good reason?

Shrinking/growing does give me this "warning" which looks a bit like shrinking or growing failed

While shrinking I am allowed to shrink to a too small amount and I get this rather large error:

I suppose libblockdev does not really record the minimum filesystem size or can calculate that for us? But hmmm I have 180 MB in use at the moment, so maybe we should set the shrink limit to FS size in use + ~ 500 MB for some FS metdata? IIRC XFS requires a minimum of 128 MB?

With shrinking NTFS I would be a bit worried about my windows OS. Should we show a warning sign anywhere? I am already a bit surprised to see resizing working fine especially shrinking!

I tested shrinking/growing a LUKS encrypted FS, this went fine! Maybe this should also be an automated test?

I tested BTRFS, it can't be shrink/grown. So this sounds like something we can implement in libblockdev? Or is that more a udisks thing? Either way growing BTRFS should be safe. Unsure about shrinking. I myself would consider shrinking a filesystem to always be an unsafe operation and would recommend to first make backups (which you already should).

Let's test something obscure like F2FS, this does not seem to be recognized by Cockpit?! Why would that be? Does udisks only know about filesystems libblockdev knows?

[root@fedora-38-127-0-0-2-2201 ~]# udisksctl info --block-device /dev/sda1
/org/freedesktop/UDisks2/block_devices/sda1:
  org.freedesktop.UDisks2.Block:
    Configuration:              []
    CryptoBackingDevice:        '/'
    Device:                     /dev/sda1
    DeviceNumber:               2049
    Drive:                      '/org/freedesktop/UDisks2/drives/QEMU_QEMU_HARDDISK_DISK0'
    HintAuto:                   false
    HintIconName:
    HintIgnore:                 false
    HintName:
    HintPartitionable:          true
    HintSymbolicIconName:
    HintSystem:                 true
    Id:                         by-id-scsi-0QEMU_QEMU_HARDDISK_DISK0-part1
    IdLabel:
    IdType:
    IdUUID:
    IdUsage:
    IdVersion:
    MDRaid:                     '/'
    MDRaidMember:               '/'
    PreferredDevice:            /dev/sda1
    ReadOnly:                   false
    Size:                       1833959424
    Symlinks:                   /dev/disk/by-diskseq/4-part1

Anyway, Cockpit allows to grow the partition, not the filesystem so that's fine.

NILFS2 can't be grown or shrink, there is nilfs-resize but that simply isn't implemented in libblockdev (yet). You can however now not enlarge the partition, is that a problem?

jelly · 2023-09-29T09:44:40Z

pkg/storaged/part-tab.jsx

+                <br />
+                <Alert variant="warning"
+                         isInline
+                         title={_("This partition is not completely used by its content.")}>


This sounds more like a @garrett question, but maybe it should be more like This partition has free space?

This text was already there for logical volumes, so ignore me.

mvollmer · 2023-09-29T12:09:25Z

Why is it not mounted back, at the end? Or do we never do this for a good reason?

Yes, good point. I will check this.

But hmmm I have 180 MB in use at the moment, so maybe we should set the shrink limit to FS size in use + ~ 500 MB for some FS metdata? IIRC XFS requires a minimum of 128 MB?

Good idea!

I tested shrinking/growing a LUKS encrypted FS, this went fine! Maybe this should also be an automated test?

We test a lot more resizing in check-storage-lvm2, more filesystem types and also LUKS.

I tested BTRFS, it can't be shrink/grown. So this sounds like something we can implement in libblockdev? Or is that more a udisks thing? Either way growing BTRFS should be safe.

Yeah, I think it is mostly libblockdev. UDisk2 reports which filesystem types support what, and Cockpit goes along with it.

Let's test something obscure like F2FS, this does not seem to be recognized by Cockpit?!

Cockpit should work with anything that UDisks2 supports. We could test more obscure ones, sure.

jelly · 2023-09-29T12:48:44Z

Why is it not mounted back, at the end? Or do we never do this for a good reason?

Yes, good point. I will check this.

But hmmm I have 180 MB in use at the moment, so maybe we should set the shrink limit to FS size in use + ~ 500 MB for some FS metdata? IIRC XFS requires a minimum of 128 MB?

Good idea!

I tested shrinking/growing a LUKS encrypted FS, this went fine! Maybe this should also be an automated test?

We test a lot more resizing in check-storage-lvm2, more filesystem types and also LUKS.

I tested BTRFS, it can't be shrink/grown. So this sounds like something we can implement in libblockdev? Or is that more a udisks thing? Either way growing BTRFS should be safe.

Yeah, I think it is mostly libblockdev. UDisk2 reports which filesystem types support what, and Cockpit goes along with it.

Let's test something obscure like F2FS, this does not seem to be recognized by Cockpit?!

Cockpit should work with anything that UDisks2 supports. We could test more obscure ones, sure.

Oh I did not suggest that hehe, it seems UDisk2 does not report the filesystem type for F2FS, so that was my remark. Maybe checking if btrfs can't be resized would be good? So we would know when it would work.

Reading libblockdev, it should actually be supported in the fs plugin. So not sure why it's not advertised as such.

There is a test in udisks for it:

        with self._temp_mount(dev.path) as mnt:
            new_size = dev.size - 20 * 1024**2
            dev.obj.Resize(dbus.UInt64(new_size), self.no_options,
                           dbus_interface=self.iface_prefix + '.Filesystem.BTRFS')

jelly · 2023-09-29T12:54:07Z

One thing I forgot to test is, swap? What does Udisks offer for that?

jelly · 2023-09-29T13:23:54Z

For the release-note it would be nice to get a screenshot.

mvollmer added the release-note label Sep 25, 2023

mvollmer force-pushed the storage-resize-partitions branch 2 times, most recently from 111fb36 to 44391ef Compare September 25, 2023 11:11

mvollmer added the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Sep 26, 2023

mvollmer force-pushed the storage-resize-partitions branch 3 times, most recently from f884f75 to 59d8b5e Compare September 26, 2023 13:38

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

mvollmer force-pushed the storage-resize-partitions branch 2 times, most recently from 22b6ac4 to cd48ea2 Compare September 28, 2023 07:18

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

mvollmer force-pushed the storage-resize-partitions branch from cd48ea2 to 8ec73eb Compare September 28, 2023 09:20

mvollmer removed the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Sep 28, 2023

mvollmer force-pushed the storage-resize-partitions branch from 8ec73eb to 796f130 Compare September 28, 2023 10:20

storage: Partition resizing

7243126

mvollmer force-pushed the storage-resize-partitions branch from 796f130 to 7243126 Compare September 28, 2023 11:18

cockpituous reviewed Sep 28, 2023

View reviewed changes

mvollmer requested a review from jelly September 29, 2023 08:26

jelly reviewed Sep 29, 2023

View reviewed changes

mvollmer requested a review from jelly September 29, 2023 12:11

jelly approved these changes Sep 29, 2023

View reviewed changes

mvollmer merged commit e65330b into cockpit-project:main Oct 2, 2023
97 checks passed

martinpitt removed the release-note label Oct 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

storage: Partition resizing #19385

storage: Partition resizing #19385

mvollmer commented Sep 25, 2023 •

edited

Loading

cockpituous Sep 28, 2023

cockpituous Sep 28, 2023

cockpituous Sep 28, 2023

cockpituous Sep 28, 2023

cockpituous Sep 28, 2023

cockpituous Sep 28, 2023

cockpituous Sep 28, 2023

jelly commented Sep 29, 2023

jelly left a comment

jelly Sep 29, 2023

jelly Sep 29, 2023

mvollmer commented Sep 29, 2023

jelly commented Sep 29, 2023 •

edited

Loading

jelly commented Sep 29, 2023

jelly commented Sep 29, 2023

		to_fit ? shrink_size : vals.size,
		to_fit ? false : info.shrink_needs_unmount,

storage: Partition resizing #19385

storage: Partition resizing #19385

Conversation

mvollmer commented Sep 25, 2023 • edited Loading

Storage: Partitions can be resized

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

jelly commented Sep 29, 2023

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

mvollmer commented Sep 29, 2023

jelly commented Sep 29, 2023 • edited Loading

jelly commented Sep 29, 2023

jelly commented Sep 29, 2023

mvollmer commented Sep 25, 2023 •

edited

Loading

jelly commented Sep 29, 2023 •

edited

Loading