pyparted icon indicating copy to clipboard operation
pyparted copied to clipboard

Not recognizing change in device capacity

Open jflorian opened this issue 8 years ago • 10 comments

Hi David,

We just stumbled onto a rather quirky issue involving USB a card "reader" with removable Flash-based media. It's easily reproduced with two completely different hardware models (Transcend RDP8 and Lexar Workflow series). Basically parted is not showing any change in geometry when we change cards from one size to another, even though a new instance is gotten each time. Whether the issue is in pyparted or parted itself, I don't know.

Here's the trivial test case:

import parted
while True:
     print('press Enter after inserting a new card and letting things settle')
     input()
     dev = parted.getDevice('/dev/sdh')
     print(dev)

Here's example output where I start with a 2 GB CF card, press Enter, replace with a 16 GB CF card (in the same reader slot), wait a few seconds and press Enter again. I then break out of the script and restart it and press Enter without touching the card this 3rd time. Oddly the change in capacity is only recognized after restarting. Whatsupwithdat?

$ sudo python3 /tmp/huh.py press Enter after inserting a new card and letting things settle

parted.Device instance -- model: Lexar WorkflowCFR1 path: /dev/sdc type: 1 sectorSize: 512 physicalSectorSize: 512 length: 3915072 openCount: 0 readOnly: False externalMode: False dirty: False bootDirty: False host: 0 did: 0 busy: False hardwareGeometry: (7676, 255, 2) biosGeometry: (7676, 255, 2) PedDevice: <_ped.Device object at 0x7f4739af3f28> press Enter after inserting a new card and letting things settle

parted.Device instance -- model: Lexar WorkflowCFR1 path: /dev/sdc type: 1 sectorSize: 512 physicalSectorSize: 512 length: 3915072 openCount: 0 readOnly: False externalMode: False dirty: False bootDirty: False host: 0 did: 0 busy: False hardwareGeometry: (7676, 255, 2) biosGeometry: (7676, 255, 2) PedDevice: <_ped.Device object at 0x7f472ee46268> press Enter after inserting a new card and letting things settle ^CTraceback (most recent call last): File "/tmp/wtf.py", line 4, in input() KeyboardInterrupt $ sudo python3 /tmp/huh.py press Enter after inserting a new card and letting things settle

parted.Device instance -- model: Lexar WorkflowCFR1 path: /dev/sdc type: 1 sectorSize: 512 physicalSectorSize: 512 length: 31293360 openCount: 0 readOnly: False externalMode: False dirty: False bootDirty: False host: 0 did: 0 busy: False hardwareGeometry: (61359, 255, 2) biosGeometry: (61359, 255, 2) PedDevice: <_ped.Device object at 0x7f8ae6d46f28> press Enter after inserting a new card and letting things settle

jflorian avatar Feb 06 '17 19:02 jflorian

It looks like adding dev.removeFromCache() after the print resolves the problem.

I don't know if that means pyparted is buggy or we were just not using it correctly.

jflorian avatar Feb 07 '17 14:02 jflorian

I've written a small test program using libparted in C which also displays the same behavior if the cache is not flushed between device reads. Source code is located at https://gist.github.com/2a2e75c27cc0ca9c5fe313f8b374f01b

Note that this assumes your card reader is /dev/sdc.

blackknight36 avatar Feb 07 '17 14:02 blackknight36

Thanks for the report. It is odd behavior, but as you see through the C program, this is the same as what libparted does. Whether or not this is a bug in libparted is up to them. I do not want to fix libparted problems in pyparted, they should be addressed there. This may be as-defined in libparted, in which case the functionality should remain the same in pyparted.

However, with pyparted we can introduce a helper in the 'parted' module. You'll notice there is the _ped module and the higher level parted module. The _ped module is meant to map more or less 1 to 1 with libparted. The parted module is written in Python on top of that. The parted module also contains some pyparted-specifics that are, in my view, ok to have because they do not override anything that libparted's API is providing.

I'll leave it to you. If you'd like to see a helper function, send a pull request with a proposal. Or if you want to report it upstream to libparted, that's fine too.

dcantrell avatar Feb 08 '17 19:02 dcantrell

@dcantrell , I totally agree that if the behavior needs to change, it should be in libparted so there's consistency everywhere. Beyond that I'm of no strong opinion what's best. It feels like a caching issue, but I can't imagine why caching would be needed here, but some of these low layers are near voodoo from my PoV. My best guess is this is akin to NTP setting the clock backwards -- it's unexpected and requires special handling by the affected things. That gels pretty good in my noggin until I consider this is removable media. Maybe libparted is sort of stuck in the past before that was so common.

Either way, I've a work-around now and if I can ever get caught up with all my fires, I hope to give this some more attention and maybe a PR. I don't know how widely used pyparted is so maybe it's not worth the effort. Regardless, I very much appreciate your feedback and the effort you've put into making pyparted such a nice package to work with.

jflorian avatar Feb 23 '17 15:02 jflorian

I would agree that not diverging from what libparted does is the better approach here. I used to be the upstream maintainer for parted and the code is esoteric. When it comes to interacting with block devices, userspace programs are really at the mercy of the kernel. Behavior of the same version of parted can vary depending on the kernel you're using. But that should be expected and is sort of the right way to handle this sort of thing. The userspace program should not work around a deficiency in the kernel.

Removable block devices present a challenge to Linux, and always have really. libparted does better does fine with this, but the kernel still really has to be told or know when a device is removed or not. Sometimes that information isn't available to userspace so knowing when to call BLKRRPART becomes a guessing game.

pyparted is used in a lot of places. It's a core piece of the installation software stack for RHEL, Fedora, CentOS, and other derivatives. And I know it's used in a lot of other places. It started life inside Red Hat and I inherited it a long time ago. I decided to completely rewrite it and expose the entire libparted API to Python programmers and that has proven to be a really good move because it's enabled a lot of neat Python-based projects to do disk partitioning work. On the other hand, it has increased the rate at which people find deficiencies in libparted. :)

I would be open to starting a contrib/ subdirectory in pyparted that contains workarounds like yours along with a writeup explaining the conditions it may show up and how to use the workaround in your code.

Thanks.

dcantrell avatar Feb 23 '17 20:02 dcantrell

I wouldn't mind adding some info to the contrib directory along with the test program I wrote. I do plan on updating the code to allow different block devices to be specified on the command line since the device name won't always be /dev/sdc.

blackknight36 avatar Feb 28 '17 16:02 blackknight36

You have some contrib/ code and notes to add for this issue? Happy to merge what you have now with notes about the limitations and explaining the problem it addresses.

dcantrell avatar Jul 13 '17 14:07 dcantrell

Please see PR#42.

jflorian avatar Jul 13 '17 15:07 jflorian

@dcantrell The test program I wrote is in C and since this is a python project it may be better to use the one included in PR #42 .

blackknight36 avatar Jul 14 '17 13:07 blackknight36

@blackknight36 It might be good to share your C test program though with parted upstream where it truly originates in case they're not aware of the issue. It's not hard to imagine someone there forgetting for a brief lapse that some devices have removable media. (Sadly, I remember an era of hard disks -- the size of a clothes washer -- which had no capacity until you "loaded" them up with a disk pack. Ah, the bad ol' days.)

jflorian avatar Jul 14 '17 13:07 jflorian