Bug 464017

Summary: Cannot shrink an ext2/3/4 filesystem
Product: [Applications] partitionmanager Reporter: Ian Turner <vectro>
Component: generalAssignee: Andrius Štikonas <andrius>
Status: RESOLVED WORKSFORME    
Severity: normal    
Priority: NOR    
Version: 20.12.2   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Ian Turner 2023-01-08 20:37:43 UTC 
SUMMARY
Attempting to shrink an ext2/3/4 filesystem fails with an error because kpmcore does not pass the correct arguments to resize2fs.


STEPS TO REPRODUCE
1. Mark an ext2/3/4 for resizing. Specify a smaller size
2. Execute the operatior

OBSERVED RESULT
Job: Resize file system on partition ‘/dev/sdc1’ to 1,827,692,544 sectors 
Resizing file system from 1,953,523,119 to 1,827,692,544 sectors. 

Command: resize2fs /dev/mapper/luks-edfeab04-9935-4f0c-b019-805ceb4f8df8 -125830575s 

Resizing encrypted file system on partition ‘/dev/sdc1’ failed. 
Resize file system on partition ‘/dev/sdc1’ to 1,827,692,544 sectors: Error

EXPECTED RESULT
Operation would succeed

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: Debian 11 "Bullseye"
(available in About System)
KDE Plasma Version: 5.20.5
KDE Frameworks Version: 5.78.0
Qt Version: 5.15.2

ADDITIONAL INFORMATION
Running the resize2fs command in the output reveals the actual problem:
$ sudo resize2fs /dev/mapper/luks-edfeab04-9935-4f0c-b019-805ceb4f8df8 -125830575s
[sudo] password for vectro: 
resize2fs 1.46.2 (28-Feb-2021)
resize2fs: invalid option -- '1'
Usage: resize2fs [-d debug_flags] [-f] [-F] [-M] [-P] [-p] device [-b|-s|new_size] [-S RAID-stride] [-z undo_file]

Annoyingly, it is necessary to quote the negative number so that it is not interpreted as a flag. The command line should be:
resize2fs /dev/mapper/luks-edfeab04-9935-4f0c-b019-805ceb4f8df8 -- -125830575s
(note the extra --)
Comment 1 Ian Turner 2023-01-08 20:38:35 UTC 
I think the bug is right here: https://github.com/KDE/kpmcore/blob/ce39e149040bce9782b91e532d041ca0c7548a77/src/fs/ext2.cpp#L181
There should be an extra "--" as the first parameter.
Comment 2 Ian Turner 2023-01-08 21:07:20 UTC 
Actually, looking more closely at the resize2fs documentation, I think the problem is that we are passing a negative number at all. And unfortunately, I'm now unable to reproduce this issue.
Comment 3 Andrius Štikonas 2023-01-08 21:16:31 UTC 
(In reply to Ian Turner from comment #2)
> Actually, looking more closely at the resize2fs documentation, I think the
> problem is that we are passing a negative number at all. And unfortunately,
> I'm now unable to reproduce this issue.

Yeah, I was just going to say that it shouldn't be negative number.

Strangely that negative number is equal to 1,953,523,119 - 1,827,692,544, so the difference between final and current size.

Do you remember whether you were trying to reduce it by a bit (1,953,523,119 -> 1,827,692,544) or by a lot 1,953,523,119 to 125830575 sectors? I.e. my guess is that maybe something went wrong and newLength() was -1 https://invent.kde.org/system/kpmcore/-/blob/master/src/jobs/resizefilesystemjob.cpp#L82
Comment 4 Ian Turner 2023-01-08 22:00:28 UTC 
I was trying to reduce it by 60 gibibytes (to 892428 mebibytes).

While I was playing around with it I reduced it by a much smaller amount (< 1GB) and then reduced it a second time to the desired size, and that worked. So possible there is some mathy issue.

I suppose that one possible step forward would be to change some of these types to unsigned, or to add asserts.
Comment 5 Andrius Štikonas 2023-01-08 22:05:33 UTC 
(In reply to Ian Turner from comment #4)
> I suppose that one possible step forward would be to change some of these
> types to unsigned

I suspect this is easier said than done. Probably both API and ABI would break.

> or to add asserts.

Asserts might be useful though.