GNOME Bugzilla – Bug 723543
Shrink ext2/3/4 results in attempt to set partition smaller than file system
Last modified: 2014-02-19 17:50:51 UTC
Created attachment 267974 [details] gparted_details.htm log file from failed shrink ext2 operation This bug report was originally opened for the Debian distribution at the following link: Debian Bug #737247 - gparted resizes partition to smaller than the filesystem http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737247 Original description follows: I tried to shrink-and-move a partition on an external hard drive. It seems that gparted shrunk the parition to 93768726 4K blocks, but then resized the parition to only 750149807 512-byte sectors (93768725.875 4K blocks). The post-shrink e2fsck then errored out before the partition could be moved.
Following is a synopsis of the original responses to date. If my summary is incorrect then this is entirely on me and not the fault of the original contributors to the Debian bug report. Anomie: Raised initial problem and included gparted_details.htm log. Phillip: This appears to be an off-by-one error in GParted. Curtis: There was a recent git commit in this area for GParted 0.16.2. Shrink file systems to exact size (#701075) 3461413d283f1bac77e541b1054e775ec105212f Phillip: That commit dealt with FS resizing. This problem is with getting the wrong partition size. Curtis: Tried to suggest that FS too large, or partition too small are two different ways to look at the same problem. Raised potential issue with 4k sector drives. Seeking Mike Fleetwood's input. Mike: Will have a closer look. Phillip: Agree that partition size should be an even multiple of 4k. Does GParted have alignment options other than MiB?
Analysis of initial setup ------------------------- http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=gparted_details.htm;att=1;bug=737247 Partition /dev/sdb1 Start: 2048 End: 3907029166 # Divide by 2048 gives remainder 174. End not # 1 MiB aligned. Unusual. Size: 3907027119 # Not a multiple of 1 MiB. e2fsck -f -y -v -C 0 /dev/sdb1 .. 81366570 blocks used (16.66%, out of 488378389) 1) Start of 2048 sectors looks like 1 MiB alignment with 512 byte sectors. 2) Size of 3907027119 sectors / 2048 gives 1907728 MiB remainder 175 sectors. Not a multiple of 1 MiB. Unusual. 3) File system size of 488378389 (4K blocks) = 3907027112 sectors is 7 sectors smaller than the partition. Perhaps would have expected less than 4K but this is OK. Recreating setup (on Fedora 19) ------------------------------- # truncate -s 2199023255040 /tmp/loop0.img # losetup -f --show /tmp/loop0.img /dev/loop0 # echo 2048,3907027119 | sfdisk -uS --force /dev/loop0 ... # sfdisk -l -uS /dev/loop0 sfdisk: Disk /dev/loop0: cannot get geometry Disk /dev/loop0: 267349 cylinders, 255 heads, 63 sectors/track Units: sectors of 512 bytes, counting from 0 Device Boot Start End #sectors Id System /dev/loop0p1 2048 3907029166 3907027119 83 Linux /dev/loop0p2 0 - 0 0 Empty /dev/loop0p3 0 - 0 0 Empty /dev/loop0p4 0 - 0 0 Empty # partx -a /dev/loop0 # cat /proc/partitions major minor #blocks name ... 7 0 2147483647 loop0 259 0 1953513559 loop0p1 # mkfs.ext4 /dev/loop0p1 mke2fs 1.42.7 (21-Jan-2013) ... 122101760 inodes, 488378389 blocks Test GParted shrinking to 386284 MiB ------------------------------------ # gpartedbin /dev/loop0 End up with file system being resize to 93768704 4K blocks and the partition size being 750149632 512 byte sectors, which exactly matches. At the moment I'm not sure how to reproduce this problem. Thanks, Mike
Thank you Mike and Phillip for your thoughts and investigation into this report. To continue with this report I will start by answering Phillip's question, and then ask another question to myself regarding alignment. The Too Long; Didn't Read (TL;DR) version is that I ruled out MiB, and None alignment, and with Cylinder alignment I do not know what values were used for number of cylinders and number of heads. Question from Phillip: I didn't think gparted *had* any other options for alignment other than MiB? Answer: Currently GParted supports three different alignment options. These are MiB, Cylinder, and None as described in the GParted Help Manual. See Specifying Partition Alignment: http://gparted.org/display-doc.php?name=help-manual#gparted-specify-partition-alignment Question Two: What Alignment was used? From the GParted details log, the partition shrink details are as follows: old start: 2048 old end: 3907029166 old size: 3907027119 (1.82 TiB) new start: 2048 new end: 750151854 new size: 750149807 (357.70 GiB) With all GParted alignment schemes, only the changed partition boundaries must satisfy the alignment scheme chosen. Boundaries that are not changed do not need to meet the alignment conditions. Since in this bug report only the end partition boundary was changed, this is the only partition boundary that must satisfy the chosen alignment scheme. OBSERVATIONS ------------ a) The partition start sector value remains the same. b) The new end is not MiB aligned. - To be *MiB* aligned the end sector must be an integer multiple of MiB minus 1 sector. - The end sector value is even so the new end is not MiB aligned. c) The new end is not Cylinder aligned (based on assumption) - Assume 255 Heads 63 Sectors per Track - To be *Cylinder* aligned the end sector must be an integer multiple of cylinders minus 1 sector. - Calculation follows: End Sector = Number of Cylinders * ( Heads * Sectors per Track) - 1 750,151,854 = Z * ( 255 Heads * 63 Sectors per Track ) - 1 Rearranging this equation: Z = ( 750,151,854 + 1 ) / ( 255 * 63 ) Z = 750,151,855 / 16,065 Z = 46,694.7933396 This value is not an integer value and hence the new end is not cylinder aligned. NOTE: This is based on the above assumption most often used for large disks. d) The new end is not None aligned. - To be *None* aligned the end sector should be larger than the start sector by an integer multiple of MiB minus 1 sector. - This is not the case because both Start and End sectors are even numbers Based on the above observations, the only theory I can come up with at this time is that cylinder alignment was used and that my assumption of 255 heads and 63 sectors was wrong. BACKGROUND ON SOURCE CODE FOR CYLINDER, MIB, AND NONE ALIGNMENT --------------------------------------------------------------- Following is a description of the code used for alignment. Initial partition boundaries are set by the respective Dialogs. For resizing a primary partition this is the method Dialog_Partition_Resize_Move::Resize_Move_Normal https://git.gnome.org/browse/gparted/tree/src/Dialog_Partition_Resize_Move.cc?id=GPARTED_0_17_0#n64 The start and sector values are initially set from the dialog in the method Dialog_Base_Partition::Get_New_Partition https://git.gnome.org/browse/gparted/tree/src/Dialog_Base_Partition.cc?id=GPARTED_0_17_0#n140 More specifically for the new end sector see this section https://git.gnome.org/browse/gparted/tree/src/Dialog_Base_Partition.cc?id=GPARTED_0_17_0#n140 All operations are added by method Win_GParted::Add_Operation https://git.gnome.org/browse/gparted/tree/src/Win_GParted.cc?id=GPARTED_0_17_0#n691 All alignment tweaking is performed by method GParted_Core::snap_to_alignment https://git.gnome.org/browse/gparted/tree/src/GParted_Core.cc?id=GPARTED_0_17_0#n625 For Cylinder alignment the extra code is in method GParted_Core::snap_to_cylinder https://git.gnome.org/browse/gparted/tree/src/GParted_Core.cc?id=GPARTED_0_17_0#n403 For MiB alignment extra code is in method GParted_Core::snap_to_mebibyte https://git.gnome.org/browse/gparted/tree/src/GParted_Core.cc?id=GPARTED_0_17_0#n453 For None alignment there are no "safety" checks and the boundaries are set based on MiB values relative to existing partitions on the disk device. Hence these are controlled by the above mentioned Dialogs.
(In reply to comment #2) > 3) File system size of 488378389 (4K blocks) = 3907027112 sectors is > 7 sectors smaller than the partition. Perhaps would have expected > less than 4K but this is OK. Of course 7 512 byte sectors is less than 4K.
Created attachment 268197 [details] [review] 0001-Remove-incorrect-rounding-in-filesystem-resize-72354.patch This should fix it.
Hi Phillip, The description for the patch could use some enhancement by mentioning which commit introduced the problem. Also, the use of the floor() method makes it clear that the size should not be rounded up. If the floor() method is removed I know that the behaviour is the same, but this is not as clear to someone reading the code. I think there is benefit to keeping the floor() method as a replacement for Utils::round(). Curtis
Created attachment 268207 [details] [review] 0001-Remove-incorrect-rounding-in-filesystem-resize-72354.patch I think KISS says it is easier to read when there is less of it. When I see a call like that my brain immediately figures it must be there for a reason and I start trying to figure out what it is actually doing, but if you like it better that way, here goes.
I agree with the KISS philosophy too. Mike, Any preference on "To use floor() or not use floor()"? Curtis
For reference, the link to the original problem recreation steps according to Anomie is: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=737247#55 And a summary of the steps to recreate the problem is as follows: $ truncate -s 2000398933504 /tmp/loop0.img $ sudo losetup -f --show /tmp/loop0.img $ echo -e "o\nn\np\n\n\n\nw\nq\n" | sudo fdisk /dev/loop0 <--snip--> $ sudo fdisk -l /dev/loop0 Disk /dev/loop0: 2000.4 GB, 2000398933504 bytes 81 heads, 62 sectors/track, 777982 cylinders, total 3907029167 sectors Units = sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disk identifier: 0x1987fd01 Device Boot Start End Blocks Id System /dev/loop0p1 2048 3907029166 1953513559+ 83 Linux $ sudo partprobe -s /dev/loop0 $ sudo gparted /dev/loop0 * Select the partition * Click "Resize/Move" * Drag the left end of the slider to the right. This time it says free space preceeding is 1556706, size is 351022, free space following is 0, align is MiB. * Hit "Ok" and confirm. Then apply. * The output is roughly the same as in the original attachment: the resize2fs step reports the new filesystem is 89861654 blocks, the partition shrink takes it down to 718893231 sectors, then the e2fsck complains that the filesystem is 89861654 blocks while the device is only 89861653. Using the above steps I was able to confirm the problem using ext4. However the problem is not obvious when using ntfs or reiserfs because the file system checks for these file systems do not appear to check file system size relative to the partition size. The problem is introduced by the following commit: Shrink file systems to exact size (#701075) https://git.gnome.org/browse/gparted/commit/?id=3461413d283f1bac77e541b1054e775ec105212f I believe the original code assumed a sector size of 512 bytes and used a round operation in combination with subtracting 1 kiB to ensure that the file system size was an integer value of kiB and was not larger than the partition. I think a safer method is to replace the Utils::round() logic with a floor() method. I have tested the patch in comment #7 and can confirm that it fixes the problem. Phillip, If you are under time deadlines to get this fix into Debian/Ubuntu then feel free to apply the patch to Debian/Ubuntu. With the GParted git repository I would like to hear from Mike first regarding the question of "To use floor() or not use floor()" because we should make all of the file system resize code similar. Curtis
Created attachment 268315 [details] [review] 0001-Remove-incorrect-rounding-in-filesystem-resize-72354.patch (v3) The attached patch contains updates to the comment text only. The updates are in an effort to add more description to the problem so that a person reviewing the commit log can more readily understand the problem. Of course a user could also use the link to the bug report to learn more. Mike, When you have a chance please review the patch. Also your thoughts on whether we should use floor() or not would be appreciated. My thoughts are that we should look toward including this in an upcoming release soon. In the meantime I have at least one other minor documentation fix that I plan to address. Thanks, Curtis
Hi All, I think floor() should be used as it is better to be explicit than implicit. Also floor() shouts use of floating point numbers and nothing else does in the computation of sizes being passed to file system resize programs. (I think the use of floating point numbers to convert from sectors to KiB or bytes is wrong because it risks conversion errors. Doubles only hold 15-17 decimal digits of precision, which is only good for about 1000 TiB sizes. Also there is the int->float->int conversion which can introduce rounding issues. This is a fix for another time). Commentary on previous fix: Shrink file systems to exact size (#701075) https://git.gnome.org/browse/gparted/commit/?id=3461413d283f1bac77e541b1054e775ec105212f Sorry for missing the the fact that the round() and -1 KiB was actually keeping a resized ext[234] file systems within the partition when it's size was an odd number of 512 byte sectors. Managed to produce the bug following Curtis' instructions from comment #9 on Fedora 19. Applied patch v3 from comment #10. Tested successfully. Passed reviewed. Thanks, Mike
Thank you Mike for reviewing the patch in comment #10 and sharing your thoughts on using floor(). Also thank you Phillip for working to provide patches to address this problem. RE: Commentary on previous fix in comment #11. I should have caught the introduction of this error back when reviewing bug 701075. I guess we are all human. :-) In our defense there is much functionality in GParted and it is virtually impossible to test all combinations and permutations of operations possible with GParted. We do our best to fix problems as they arise. The patch in comment #10 has been applied to the master branch of the GNOME git repository for inclusion in the next release of GParted. The relevant git commit can be viewed at the following link: Remove incorrect rounding in file system resize (#723543) https://git.gnome.org/browse/gparted/commit/?id=5f9a55fdcb37adc15655753dce1e714a3ab50075
This enhancement was included in the GParted 0.18.0 release on February 19, 2014.