After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 639393 - Redux: Destination partition smaller than source partition
Redux: Destination partition smaller than source partition
Status: RESOLVED FIXED
Product: gparted
Classification: Other
Component: application
0.7.1
Other Linux
: Normal major
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2011-01-13 04:41 UTC by Justin Gottula
Modified: 2011-01-15 23:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gparted_details.htm from GParted 0.6.2 (1.13 KB, text/html)
2011-01-13 04:41 UTC, Justin Gottula
  Details
Patch to add debug printf's to src/Dialog_Base_Partition.cc (1.96 KB, patch)
2011-01-13 20:45 UTC, Justin Gottula
none Details | Review
First attempt to improve logic for MiB alignment (2.96 KB, patch)
2011-01-13 22:04 UTC, Curtis Gedak
none Details | Review
Second attempt to improve logic for MiB alignment (2.97 KB, patch)
2011-01-13 22:09 UTC, Curtis Gedak
none Details | Review
Third attempt to improve logic for MiB alignment (4.82 KB, patch)
2011-01-14 19:01 UTC, Curtis Gedak
none Details | Review
Fourth attempt to improve logic for MiB alignment (4.74 KB, patch)
2011-01-14 20:16 UTC, Curtis Gedak
none Details | Review

Description Justin Gottula 2011-01-13 04:41:45 UTC
Created attachment 178203 [details]
gparted_details.htm from GParted 0.6.2

I have a 1 TB SATA 3Gbps drive with a GPT table and a single, drive-filling Btrfs partition. Today I attempted to copy this partition to the beginning of an empty, GPT-formatted 2 TB USB 2.0 drive using GParted 0.6.2 (selecting to align the destination to mebibytes) and encountered the "destination is smaller than the source partition" error immediately upon execution of the copy operation; clearly, the destination was large enough for the source partition (see attachment for gparted_details.htm).

Relevant partition information from parted:

Model: Hitachi HDS722020ALA330 (scsi)
Disk /dev/sdb: 2000GB
Sector size (logical/physical): 512B/512B
Partition Table: gpt

Number  Start  End  Size  File system  Name  Flags


Model: ATA WDC WD10EALS-00Z (scsi)
Disk /dev/sdi: 1000GB
Sector size (logical/physical): 512B/512B
Partition Table: gpt

Number  Start   End     Size    File system  Name    Flags
 1      17.4kB  1000GB  1000GB  btrfs        Backup


At this point, I seemed to remember having this problem some weeks before. I can't remember exactly when, or what kind of partitions I was dealing with then; but in any case, I had probably just given up and used dd back at that time. In this case, however, I decided to look through the bugzilla listings to find anyone else with a similar issue and came across bug #626946. Upon discovering that this was indeed a known issue with 0.6.2, the very version I was using, I promptly downloaded the 0.7.1 source, compiled it, and attempted to run the copy again. To my surprise, the error persisted (producing exactly the same gparted_details.htm, save for the version number).

I can verify that the "add one MiB" workaround is still effective. I most likely originally created the source partition using mkfs.btrfs (this was before the time of GParted/btrfs compatibility), so it's probable that the source partition was aligned to cylinders, not mebibytes.

I next looked into the source from which I compiled 0.7.1 and verified that the patch from the relevant git commit (b77a5e229dae407b40fdf6661483a3808797251d) was in effect in my 0.7.1 build (just to assure myself that major source code weirdness wasn't in effect).

At this point, I checked a few other things: setting up the destination as an MBR partition, then copying (error); aligning to cylinders on the copy (success, as expected); using an MBR-formatted 2 GB USB 2.0 flash drive with a drive-filling Btrfs partition as the source and running the copy (success; major WTF?! moment followed). At this point, I tried the original copy operation over again... error, as always.

At this point, I am aware that I can relatively easily work around the problem by adding a mebibyte or aligning to cylinders on the copy (the latter of which happens to be 100% fine for my situation). Still, I'm intensely perplexed by my consistent inability to reproduce the error in any situation except the exact copy operation I originally intended to run.

Stuff I didn't try, for full bug reporter integrity/honesty:
- Running the 0.7.1 LiveCD
- Uninstalling the 0.6.2 version of GParted from my distribution prior to running 0.7.1 (though I did verify that I was running the correct version)
- Cloning the git repo and compiling from the head revision (figured this wouldn't change anything as the relevant commit/patch was already in place as of 0.6.3)
Comment 1 Justin Gottula 2011-01-13 04:48:33 UTC
For additional clarity, the Btrfs partition I created on the flash drive to try to reproduce the problem was aligned to cylinders; hence my surprise, since the new source partition appeared to be set up in exactly the same manner as the original source partition, yet yielded a different result.
Comment 2 Curtis Gedak 2011-01-13 17:56:50 UTC
Thank you Justin for reporting this problem.

Would you be able to provide the output from the following commands?

   fdisk -l -u

where one of the options is a lower case "L" and not the number one.

   parted /path-to-your-device unit s print

where /path-to-your-device is something like /dev/sda.


With these sector values I can run through the calculations to try to see why this error is occurring.
Comment 3 Justin Gottula 2011-01-13 18:33:26 UTC
sudo fdisk -l -u /dev/sdb

WARNING: GPT (GUID Partition Table) detected on '/dev/sdb'! The util fdisk doesn't support GPT. Use GNU Parted.


Disk /dev/sdb: 1999.9 GB, 1999926984704 bytes
255 heads, 63 sectors/track, 243143 cylinders, total 3906107392 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: 0x00000000

   Device Boot      Start         End      Blocks   Id  System
/dev/sdb1               1  3906107391  1953053695+  ee  GPT



sudo fdisk -l -u /dev/sdi

WARNING: GPT (GUID Partition Table) detected on '/dev/sdi'! The util fdisk doesn't support GPT. Use GNU Parted.


Disk /dev/sdi: 1000.2 GB, 1000204886016 bytes
255 heads, 63 sectors/track, 121601 cylinders, total 1953525168 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: 0x00000000

   Device Boot      Start         End      Blocks   Id  System
/dev/sdi1               1  1953525167   976762583+  ee  GPT



sudo parted /dev/sdb unit s print

Model: Hitachi HDS722020ALA330 (scsi)
Disk /dev/sdb: 3906107392s
Sector size (logical/physical): 512B/512B
Partition Table: gpt

Number  Start  End          Size         File system  Name  Flags



sudo parted /dev/sdi unit s print

Model: ATA WDC WD10EALS-00Z (scsi)
Disk /dev/sdi: 1953525168s
Sector size (logical/physical): 512B/512B
Partition Table: gpt

Number  Start  End          Size         File system  Name    Flags
 1      34s    1953520064s  1953520031s  btrfs        Backup
Comment 4 Curtis Gedak 2011-01-13 18:35:58 UTC
Sorry, my bad.  I believe "gdisk" is used for gpt partition tables.

The output from parted should be sufficient for me to run the calculations.
Comment 5 Justin Gottula 2011-01-13 18:38:18 UTC
That makes a little more sense. Here's the gdisk output in case it does turn out to be useful:

sudo gdisk -l /dev/sdb

Found valid GPT with protective MBR; using GPT.
Disk /dev/sdb: 3906107392 sectors, 1.8 TiB
Disk identifier (GUID): 0287D60B-356B-4460-8714-1F9F488F5311
Partition table holds up to 128 entries
First usable sector is 34, last usable sector is 3906107358
Total free space is 1952587294 sectors (931.1 GiB)

Number  Start (sector)    End (sector)  Size       Code  Name



sudo gdisk -l /dev/sdi

Found valid GPT with protective MBR; using GPT.
Disk /dev/sdi: 1953525168 sectors, 931.5 GiB
Disk identifier (GUID): 25A37D4F-6D6C-4A66-B462-141AA9854E75
Partition table holds up to 128 entries
First usable sector is 34, last usable sector is 1953525134
Total free space is 5070 sectors (2.5 MiB)

Number  Start (sector)    End (sector)  Size       Code  Name
   1              34      1953520064   931.5 GiB   0700  Backup
Comment 6 Curtis Gedak 2011-01-13 19:11:39 UTC
Hmmm...  I'm not sure but we could be encountering a problem with loss of precision in integer math.

Would you be able to retry the operation by compiling code that wraps integers with "Sector(#)"?

The updated code can be downloaded at:
http://gparted.org/curtis/gparted-0.7.1-git.tar.bz2
Comment 7 Justin Gottula 2011-01-13 19:19:48 UTC
Running 0.7.1-git with the code change in place, the operation still trips the error.
Comment 8 Curtis Gedak 2011-01-13 19:44:13 UTC
Thank you for the prompt test response.  It would appear that I will need to dig deeper for the cause of the problem.
Comment 9 Justin Gottula 2011-01-13 20:45:34 UTC
Created attachment 178254 [details] [review]
Patch to add debug printf's to src/Dialog_Base_Partition.cc

I added some debug printf's to the critical part of Dialog_Base_Partition::Get_New_Partition in order to shed a little light on how the math is working out. On my partitions, the output is as follows:

DEBUG: diff = 2048
DEBUG: selected_partition .sector_start % (MEBIBYTE / selected_partition .sector_size ) = 0
DEBUG: selected_partition .sector_end - START + Sector(1) + diff = 1953523712
DEBUG: total_length = 3906107392
DEBUG: selected_partition .sector_end was 1953521663
DEBUG: if statement NOT executed
DEBUG: selected_partition .sector_end is now 1953521663
Comment 10 Curtis Gedak 2011-01-13 21:03:35 UTC
Thanks for the debug printf patch and the output from running on your system.  I think you are on the right track to solving this problem.

I can see that the reason the if statement is not executed is because the partition does start on a MiB boundary:

DEBUG: selected_partition .sector_start % (MEBIBYTE / selected_partition
.sector_size ) = 0

This check was to determine if the starting sector was not MiB aligned (and assumed to be Cylinder aligned).


My initial thoughts are that this logic needs to be reworked to round up the partition size to the nearest full MiB.  I now think that the existing logic doesn't quite do this exactly.  Instead it tries to round up the partition end to the nearest full MiB, but only if the partition start is not already on a MiB boundary.
Comment 11 Curtis Gedak 2011-01-13 21:12:23 UTC
It is important to note that we cannot simply remove the line that checks the start sector alignment.  This line was added due to the following problem.

Bug #632478 - Move/Copy using MiB alignment results in destination being 1 MiB larger

We will need improved logic that takes into account both of these problems.
Comment 12 Justin Gottula 2011-01-13 21:26:42 UTC
So, as I understand it, this particular source partition, which was cylinder-aligned, just happened to begin on a mebibyte boundary by coincidence, yet did not end on one, and so the code, detecting that it did start on a MiB boundary, concluded that it must be a MiB-aligned partition; as such, it tried to copy it to a destination that was slightly too small because it rounded the partition's last sector down to the nearest MiB based on the assumption that the partition was MiB-aligned on both sides anyway. Instead, in this case (partition starts on MiB boundary but does not end on a MiB boundary), the code should round UP to the nearest MiB boundary on the right end of the partition. The central problem here seems to be that a partition that starts on a MiB boundary is not necessarily a MiB-aligned partition; it could have been aligned to some other metric, but just happened to have its left end on a MiB boundary by chance; and if this is the case, it is fairly unlikely that it will end on a MiB boundary by similar chance.

Does this explanation check out? I made an assumption or two in there based on my limited understanding of the way GParted handles partition alignment.
Comment 13 Curtis Gedak 2011-01-13 21:39:40 UTC
Yes, I believe you understand the essence of this problem.

There is another piece of code in GParted_Core.cc called snap_to_mebibyte which will round a start sector up, and/or an end sector down to ensure that these values are aligned to MiB boundaries.  This can result in a partition shrinking such that the destination partition is smaller than the source partition.

The code in Dialog_Base_Partition.cc tries to handle this situation by growing the end of the destination partition if there is room (e.g. more unallocated space).

I am now trying to rework this into simpler logic to handle your situation as well.
Comment 14 Curtis Gedak 2011-01-13 22:04:47 UTC
Created attachment 178265 [details] [review]
First attempt to improve logic for MiB alignment

Attached is a first attempt to improve the logic needed to handle the situations where MiB alignment can result in a destination partition being smaller than the source partition.

Please note that much more testing is required to be sure that this change to Dialog_Base_Partition.cc does not adversely affect other operations like create new partition or move/resize partition.
Comment 15 Curtis Gedak 2011-01-13 22:09:58 UTC
Created attachment 178266 [details] [review]
Second attempt to improve logic for MiB alignment
Comment 16 Justin Gottula 2011-01-13 22:26:33 UTC
The first patch works for me (notwithstanding any possible regressions):

 sector_size: 512
sector_start: 2048
  sector_end: 1953521663
       START: 0
total_length: 3906107392
partition_size: 1953519616
sectors_in_mib: 2048
          diff: 0
DEBUG:  if statement executed

The second patch, though, does not:

 sector_size: 512
sector_start: 2048
  sector_end: 1953521663
       START: 0
total_length: 3906107392
partition_size: 1953519616
sectors_in_mib: 2048
          diff: 0
DEBUG:  if statement not executed
Comment 17 Curtis Gedak 2011-01-13 22:30:44 UTC
Thank you for the prompt testing of both patches.

The first patch had a bug when determining if the end sector was on a MiB boundary.  To do this calculation one has to add 1 sector to the end sector value before performing the modulus operation.

Hence the first patch would almost always have added an additional MiB to the partition size.


The information you have provided on the second patch tells me that I haven't quite got the logic right just yet.
Comment 18 Justin Gottula 2011-01-13 22:36:57 UTC
I started over fresh from git HEAD, applying the second patch, to ensure that my patch-repatch-repatching wasn't interfering with the results; second patch still fails.
Comment 19 Curtis Gedak 2011-01-13 22:50:14 UTC
I will need to think on this, perhaps overnight.  I can see that somehow the partition size in the debug statements (1,953,519,616) is already
smaller than the source partition size (1,953,520,031).
Comment 20 Curtis Gedak 2011-01-14 19:01:47 UTC
Created attachment 178341 [details] [review]
Third attempt to improve logic for MiB alignment

Attached is a third attempt to address this problem.  I think that I found the source of the problem.

Justin, would you be able to test using this third patch?
Comment 21 Curtis Gedak 2011-01-14 20:16:28 UTC
Created attachment 178351 [details] [review]
Fourth attempt to improve logic for MiB alignment

Just caught another problem that I discovered when copying a 48 MiB cylinder aligned partition to a new MiB aligned partition on the same drive.  This fourth patch addresses this problem.

The key item added in the third patch was to ensure that the COPIED_LENGTH_MB value was set to the next highest full MiB, and not simply rounded to the nearest MiB.

If you have a chance to test this fourth patch it would be appreciated.
Comment 22 Justin Gottula 2011-01-14 22:59:42 UTC
Using patch #4:

- Original test case (the problematic Btrfs partition copy) ran without error.

Created two 100 MiB partitions, one MiB-aligned (204800-sector), and one cylinder-aligned (~101.98 MiB):

- Copying MiB-aligned to MiB-aligned worked properly (size stayed the same).
- Copying cylinder-aligned to cylinder-aligned worked properly (size stayed the same).

- Copying MiB-aligned to cylinder-aligned worked properly (grew to next highest cylinder size; math checks out).
- Copying cylinder-aligned to MiB-aligned worked properly (grew to next MiB; math checks out).

Created a 100 MiB (204800-sector) partition using gdisk with arbitrary (non-MiB, non-cylinder) alignment:

- Copying to MiB-aligned worked properly (size stayed the same).
- Copying to cylinder-aligned worked properly (size grew to 101.98 MiB as expected).

Created a <100 MiB (204799-sector) partition using gdisk with arbitrary alignment:

- Copying to MiB-aligned worked properly (size grew by a sector to 204800).
- Copying to cylinder-aligned worked properly (size grew to next cylinder size).

Created a >100 MiB (204801-sector) prtition using gdisk with arbitrary alignment:

- Copying to MiB-aligned worked properly (size grew to 101 MiB).
- Copying to cylinder-aligned worked properly (size grew to next cylinder size).

Copies were performed between partitions within one GPT disk. I made sure to check for fencepost/off-by-one errors with partition sector sizes. No odd behavior of any kind occurred, which is encouraging.
Comment 23 Curtis Gedak 2011-01-15 00:12:50 UTC
Thank you Justin for the extensive testing you performed.

I tested copies on the same physical disk, and also to a new physical disk.  I tried cylinder and MiB aligned copies, and ones that changed the alignment.  Each one of my tests (using patch #4) worked correctly.  Hence I think we might have finally solved this problem.

I plan soon to remove the debug statements and then to commit this enhanced code to the git repository.
Comment 24 Curtis Gedak 2011-01-15 18:09:22 UTC
Thanks again Justin for all of your work.  Your support was a huge factor in being able to diagnose and fix this bug in a prompt manner.

A fix for this problem has been committed to the git repository for inclusion in the next release of GParted (0.8.0).

The relevant git commit can be viewed at the following link:
http://git.gnome.org/browse/gparted/commit/?id=6ae39268f21d35679764817be23fceefd371a583
Comment 25 Justin Gottula 2011-01-15 23:48:20 UTC
Glad to have been of assistance.