Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Partition re-size bug for 64GB sd cards #25

Closed
agrez opened this issue Feb 26, 2016 · 11 comments
Closed

Partition re-size bug for 64GB sd cards #25

agrez opened this issue Feb 26, 2016 · 11 comments

Comments

@agrez
Copy link

agrez commented Feb 26, 2016

Hi David

We have a Fedora remix (specifically for the Rasppberry Pi 2B) that uses a python script (rootfs-resize) to automatically resize the rootfs partition on first boot. rootfs-resize itself relies on pyparted to make all the relevant partitioning modifications (i.e to grow the last partition on the sd card). See: https://github.com/ctyler/rootfs-resize

One of our users recently describe a problem whereby the resize script does not work correctly when used on sd cards that are > 32 GB. See: fedberry/fedberry#8

I haven't delved into the relevant pyparted functions but as mentioned in the above issue, I can see when root.getMaxGeometry(constraint).end is called, it returns an incorrect value for my card. I'm not exactly sure if there is a problem with defining the constraint or if there is a deeper issue within pyparted.

When you get a chance, would you mind taking a quick look and offering your opinion please?

If you need any more info (in addition to what is already posted to fedberry/fedberry#8, then please just ask and I will try to help where I can.

Best Regards
Vaughan

@dcantrell
Copy link
Owner

I finally have a 64 GB microSDXC card that I can use now to work on this.

@dcantrell
Copy link
Owner

OK, at first glance it appears that the device's bios_geom is not initialized properly, which is why the constraint is not the same as the device. I'm still digging in to this, but that's my theory right now.

libparted uses the HDIO_GETGEO ioctl to initialize the bios_geom field on Linux. That gives cylinders, heads, and sectors and then those values are later used by things like getOptimalAlignedConstraint. Likewise, getMaxGeometry(constraint).end would be incorrect too because the bios_geom is initialized incorrectly.

Over on the fedberry thread, someone mentioned it possibly being an integer overflow in pyparted. Seems plausible, but the values are stored as PedSectors which are long long ints in C. So we have room there. I really think this is the ioctl not working, so I'm going to keep going down that road and see where it goes. Just wanted to post an update here for you.

@agrez
Copy link
Author

agrez commented Mar 17, 2016

Thanks for looking into this and the update, its appreciated. Sorry but I haven't been able to dig any deeper with with issue but I have both time and knowledge constraints (pardon the pun) hampering me.

@dcantrell
Copy link
Owner

What I suspected is correct. libparted (which is part of GNU parted and sits underneath pyparted....I used to be the upstream owner of GNU parted before doing pyparted) is using HDIO_GETGEO which has long since been deprecated. It's still there and works for most common cases, but returns useless info for more modern devices. The idea of cylinders, heads, and sectors is not really relevant anymore since everything is LBA now. But a sector count is useful for libparted, which we can still get. Working on a candidate patch right now for libparted.

Unfortunately this is not something I can fix in pyparted. For now, continue using your workaround. The optimalAlignedConstraint will contain bogus values until libparted is fixed.

I'll leave this ticket open and keep you updated with libparted and when/if the problem is fixed there.

@dcantrell
Copy link
Owner

And now I think I have a candidate patch. I'm using BLKSSZGET to get the sector size, but falling back on the deprecated HDIO_GETGEIO ioctl just in case. libparted tracks cylinders and heads too, which are lies, but I don't want to gut the API with this patch. Local tests show it working for me, so I'll see about getting this upstream in libparted.

@dcantrell
Copy link
Owner

I have a candidate patch to fix this in parted (specifically libparted) and am working with upstream on it. Here's the patch I am currently working with:

https://dcantrel.fedorapeople.org/patches/parted/0001-Use-BLKSSZGET-to-get-device-sector-size-in-linux.c.patch

If anyone wants to take that, add it to the parted SRPM, rebuild locally, and try it out on a Pi... please do and report whether or not it works. This appears to solve the problem for me and optimalAlignedConstraint works as expected.

@dcantrell
Copy link
Owner

Updated patch for parted:

https://dcantrel.fedorapeople.org/patches/parted/0001-Use-BLKSSZGET-to-get-device-sector-size-in-_device_p.patch

Same instructions as above. Add that to the parted SRPM in Fedora, rebuild, and see if it resolves the problem. Thanks.

@agrez
Copy link
Author

agrez commented Mar 17, 2016

Thanks for your sleuthing on this. I will push out an update for parted and test it out.

@dcantrell
Copy link
Owner

No problem. FYI, there is a new revision of the patch at the same URL. The parted test suite had one failing test on MSDOS extended partitions and it was due to this patch. The updated patch fixes that.

@agrez
Copy link
Author

agrez commented Mar 19, 2016

ok, I finally found some time to test this out. I patched parted using your latest patch but we still have problems. It is somewhat improved in that the root partition is now growing to use most of the available free space, however unfortunately the start sector of the root partition is still being moved (eg. from sector 2541568 to 261120 in my case).

The ramifications of this are obviously bad as we no longer have a valid start to our root partition. So we seem to still have issues defining constraints. Refer to the attached 'before' and 'after' txt files to see what happens to the partitions.

after_resize.txt
before_resize.txt

bcl pushed a commit to bcl/parted that referenced this issue Apr 27, 2016
Seen on certain newer devices (such as >32G SDHC memory cards), the
HDIO_GETGEO ioctl does not return useful information.  The libparted
code records hardware and bios reported geometry information, but all of
that is largely unusable these days.  The information is used in the
PedConstraint code for aligning partitions.  The sector count is most
useful.  Rather than only trying HDIO_GETGIO, first initialize the
bios_geom fields to 0 and then use BLKSSZGET to capture the sector size.
If that fails, try HDIO_GETGEO.  And if that fails, raise a warning and
fall back on the library's default sector size macro.

This problem showed up on Raspberry Pi devices where users were
attempting to grow a partition to fill the SDHC card.  Using the
optimal_aligned_constraint returned invalid geometry information
(98703359 instead of 124735488 sectors).  The issue was reported here:

    fedberry/fedberry#8

And to the pyparted project:

    dcantrell/pyparted#25

I've applied this patch locally to parted, rebuilt, and reinstalled it
and it is working correctly for the problem SDHC cards.

Signed-off-by: Brian C. Lane <bcl@redhat.com>
bcl pushed a commit to bcl/parted that referenced this issue Oct 4, 2016
Seen on certain newer devices (such as >32G SDHC memory cards), the
HDIO_GETGEO ioctl does not return useful information.  The libparted
code records hardware and bios reported geometry information, but all of
that is largely unusable these days.  The information is used in the
PedConstraint code for aligning partitions.  The sector count is most
useful.  Rather than only trying HDIO_GETGIO, first initialize the
bios_geom fields to 0 and then use BLKSSZGET to capture the sector size.
If that fails, try HDIO_GETGEO.  And if that fails, raise a warning and
fall back on the library's default sector size macro.

This problem showed up on Raspberry Pi devices where users were
attempting to grow a partition to fill the SDHC card.  Using the
optimal_aligned_constraint returned invalid geometry information
(98703359 instead of 124735488 sectors).  The issue was reported here:

    fedberry/fedberry#8

And to the pyparted project:

    dcantrell/pyparted#25

I've applied this patch locally to parted, rebuilt, and reinstalled it
and it is working correctly for the problem SDHC cards.

Signed-off-by: Brian C. Lane <bcl@redhat.com>
@dcantrell
Copy link
Owner

Since this change is present in the upstream parted source, I am closing it. The fix is not in parted-3.2, but it is in the parted git repo.

In Fedora rawhide, I have bumped the BuildRequires on the parted package to 3.2-18, which includes this fix from git. That does not enforce a runtime requirement on the parted-3.2-18 package or later, but basically you should be good in recent versions of Fedora.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants