search

dcantrell / pyparted

Python bindings for GNU parted (libparted)
GNU General Public License v2.0
86 stars 43 forks source link

Partition table wiped if backup GPT table not found #72

Closed jake2184 closed 4 years ago

jake2184 commented 4 years ago

I've found an odd destructive issue. It may be this is an issue with libparted rather than the python wrapper (suspect so), but some mitigation would be good.

Scenario: If a disk is expanded (virtual disk on a VM), and pyparted cannot find a backup GPT table, it wipes the existing partiition table, and writes a single partition, with a dos label type. 

$~ fdisk -l /dev/sda 
WARNING: fdisk GPT support is currently new, and therefore in an experimental phase. Use at your own discretion.

Disk /dev/sda: 107.4 GB, 107374182400 bytes, 209715200 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 label type: gpt
Disk identifier: 3E80CBCB-B1ED-430C-A36F-7C59A8A1A5FF

#         Start          End    Size  Type            Name
 1         2048       411647    200M  EFI System      EFI System Partition
 2       411648      2459647   1000M  Microsoft basic 
 3      2459648      2461695      1M  BIOS boot       
 4      2461696    209713151   98.8G  Linux LVM  

$~ python -c "import parted; dev='/dev/sda'; device=parted.Device(dev); disk=parted.Disk(device)"

# No problem. Now expand the disk (outside of VM) and rescan the device

$~ echo 1 > /sys/class/scsi_device/0\:0\:0\:0/device/rescan 
$~ python -c "import parted; dev='/dev/sda'; device=parted.Device(dev); disk=parted.Disk(device)"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib64/python2.7/site-packages/parted/decorators.py", line 41, in new
    ret = fn(*args, **kwds)
  File "/usr/lib64/python2.7/site-packages/parted/disk.py", line 51, in __init__
    self.__disk = _ped.Disk(device.getPedDevice())
_ped.DiskLabelException: The backup GPT table is not at the end of the disk, as it should be.  This might mean that another operating system believes the disk is smaller.  Fix, by moving the backup to the end (and removing the old backup)?

$~ fdisk -l /dev/sda 

Disk /dev/sda: 118.1 GB, 118111600640 bytes, 230686720 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 label type: dos
Disk identifier: 0x00000000

   Device Boot      Start         End      Blocks   Id  System
/dev/sda1               1   209715199   104857599+  ee  GPT

Throwing the exception is understandable, but why does this overwrite the partition table in such an aggressive manner?

dcantrell commented 4 years ago

Odd. The code path there in pyparted has been there for a long time (I'm seeing 2009 and earlier in logs). But that doesn't mean it's not at fault.

What's happening here is eventually _ped_Disk_init() is invoked which when you take away the Python module wrappings is really just calling ped_disk_new().

@bcl thoughts on this one? pyparted or parted?

bcl commented 4 years ago

In general I don't trust fdisk to diagnose parted problems :)

When I try this with a disk.img it works just fine for me. Running parted on it prompts me to fix the backup GPT header, and running pyparted doesn't prompt me at all and I do not see a traceback. Also when I run fdisk on the resized and unfixed disk image it warns me about the disk size change as well.

What versions of the various tools are you using? I have:

jake2184 commented 4 years ago

This is on a Centos 7.7 build.

Sorry, this was probably a user error/concern rather than a destructive bug.I was trying to script this all automatically, and failed to separate the resizing of the disk (echo 1 > /sys/...) and running pyparted.

After resizing the disk, and rescanning the SCSI device, but BEFORE running pyparted, fdisk gives the output I posted above (single GPT partition), whereas parted correctly prompts straight away about the GPT backup table not being at the end of the disk.

$~  fdisk -l /dev/sda 
Disk /dev/sda: 322.1 GB, 322122547200 bytes, 629145600 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 label type: dos
Disk identifier: 0x00000000

   Device Boot      Start         End      Blocks   Id  System
/dev/sda1               1   419430399   209715199+  ee  GPT

$~  parted -l /dev/sda
Error: The backup GPT table is not at the end of the disk, as it should be.
This might mean that another operating system believes the disk is smaller.
Fix, by moving the backup to the end (and removing the old backup)?
Fix/Ignore/Cancel?

If I fix with parted/gdisk, then fdisk correctly shows the original partitions.

This is an issue with fdisk not nicely handling the GPT table being missing, and mis-reporting the partitions. I was running the resize and then pyparted straight away, so assumed the latter was being disruptive (as why would extending a disk break the partition table that fdisk was reading from what I assumed was the start of the disk).

Therefore, I feel the exception is valid (and fairly self explanatory). I had wondered if pyparted could somehow improve its handling this situation, but I think throwing an exception for the lib user to handle is perfectly valid.

Thanks for your time.

bcl commented 4 years ago

Ah! Ok, much older versions of things. That version of fdisk doesn't properly support GPT so it's just showing the protective MBR that GPT uses to keep tools like old-fdisk from unexpectedly clobbering things.