curtin

Curtin fails if a partition with bootable flag is present

Bug #1875903 reported by seahawk1986
34
This bug affects 6 people
Affects Status Importance Assigned to Milestone
curtin
Fix Released
High
Unassigned

Bug Description

I became aware of this problem by reading this forum post: https://www.vdr-portal.de/forum/index.php?thread/133200-yavdr-experimental-f%C3%BCr-ubuntu-20-04-yavdr-ansible-focal/&postID=1325428#post1325428 so I am not able to do an explicit debug run.

curtain parses the output of a subprocess call like "sfdisk --dump /dev/sdb", which can look like this:

label: dos
label-id: 0x0007ca9c
device: /dev/sdb
unit: sectors
/dev/sdb1 : start= 2048, size= 29294592, type=83, bootable
/dev/sdb2 : start= 29296640, size= 28682240, type=83
/dev/sdb3 : start= 57985022, size= 2872291330, type=5
/dev/sdb5 : start= 2927134720, size= 3141632, type=82
/dev/sdb6 : start= 57985024, size= 29294592, type=83
/dev/sdb7 : start= 87281664, size= 2839851008, type=83

The method _sfdisk_parse_lines(lines) (https://github.com/canonical/curtin/blob/master/curtin/block/__init__.py#L251 ff.) expects key-value pairs in the style key=value, which isn't the case for any partition flags.

As a side node: The output of sfdisk for gpt-formatted disks can contain partition names, so splitting the right hand side string of the colon by ',' (https://github.com/canonical/curtin/blob/master/curtin/block/__init__.py#L259 ff.) without proper parsing of quotes (e.g. using shlex.split() https://docs.python.org/3/library/shlex.html) could cause additional issues if the partition name contains one or more commata.

Related branches

~raharper/curtin:ubuntu-devel-new-upstream-snapshot-20200507
Server Team CI bot: Needs Fixing (continuous-integration)
Chad Smith: Approve
Diff: 10726 lines (+6948/-918)
83 files modified
Makefile (+15/-6)
curtin/__init__.py (+2/-0)
curtin/block/__init__.py (+190/-12)
curtin/block/bcache.py (+242/-1)
curtin/block/clear_holders.py (+34/-51)
curtin/block/lvm.py (+43/-6)
curtin/block/mdadm.py (+10/-25)
curtin/block/mkfs.py (+8/-2)
curtin/block/multipath.py (+90/-11)
curtin/block/schemas.py (+13/-2)
curtin/commands/apt_config.py (+1/-5)
curtin/commands/block_discover.py (+11/-2)
curtin/commands/block_meta.py (+597/-474)
curtin/commands/clear_holders.py (+44/-9)
curtin/commands/curthooks.py (+225/-38)
curtin/distro.py (+18/-1)
curtin/storage_config.py (+64/-36)
curtin/udev.py (+25/-2)
curtin/util.py (+13/-2)
debian/changelog (+58/-0)
dev/null (+0/-6)
doc/topics/storage.rst (+108/-19)
examples/tests/bcache-ceph-nvme-simple.yaml (+2/-2)
examples/tests/bcache-ceph-nvme.yaml (+2/-2)
examples/tests/filesystem_battery.yaml (+1/-0)
examples/tests/mirrorboot-uefi.yaml (+17/-1)
examples/tests/multipath-lvm-part-wipe.yaml (+125/-0)
examples/tests/multipath-lvm.yaml (+121/-0)
examples/tests/multipath.yaml (+6/-0)
examples/tests/preserve-bcache.yaml (+82/-0)
examples/tests/preserve-lvm.yaml (+77/-0)
examples/tests/preserve-partition-wipe-vg-simple.yaml (+62/-0)
examples/tests/preserve-partition-wipe-vg.yaml (+116/-0)
examples/tests/preserve-raid.yaml (+4/-2)
examples/tests/reuse-lvm-member-partition.yaml (+94/-0)
examples/tests/reuse-msdos-partitions.yaml (+77/-0)
examples/tests/reuse-raid-member-wipe-partition.yaml (+2/-0)
examples/tests/reuse-raid-member-wipe.yaml (+2/-1)
examples/tests/uefi_reuse_esp.yaml (+4/-2)
examples/tests/vmtest_defaults.yaml (+19/-1)
helpers/common (+11/-5)
tests/data/probert_storage_bogus_wwn.json (+1258/-0)
tests/data/probert_storage_nvme_multipath.json (+310/-0)
tests/data/udevadm_info_sandisk_cruzer.txt (+54/-0)
tests/unittests/helpers.py (+52/-3)
tests/unittests/test_apt_custom_sources_list.py (+2/-0)
tests/unittests/test_apt_source.py (+2/-0)
tests/unittests/test_block.py (+104/-5)
tests/unittests/test_block_lvm.py (+31/-1)
tests/unittests/test_block_mdadm.py (+8/-6)
tests/unittests/test_block_mkfs.py (+9/-0)
tests/unittests/test_block_multipath.py (+45/-0)
tests/unittests/test_block_zfs.py (+24/-20)
tests/unittests/test_clear_holders.py (+51/-37)
tests/unittests/test_commands_block_meta.py (+885/-7)
tests/unittests/test_commands_clear_holders.py (+24/-0)
tests/unittests/test_commands_collect_logs.py (+2/-0)
tests/unittests/test_curthooks.py (+483/-8)
tests/unittests/test_distro.py (+51/-2)
tests/unittests/test_gpg.py (+50/-44)
tests/unittests/test_make_dname.py (+3/-3)
tests/unittests/test_storage_config.py (+189/-3)
tests/unittests/test_udev.py (+43/-6)
tests/unittests/test_util.py (+25/-2)
tests/vmtests/__init__.py (+53/-17)
tests/vmtests/releases.py (+20/-3)
tests/vmtests/test_basic.py (+33/-19)
tests/vmtests/test_fs_battery.py (+7/-0)
tests/vmtests/test_mdadm_bcache.py (+34/-0)
tests/vmtests/test_multipath.py (+80/-3)
tests/vmtests/test_multipath_lvm.py (+76/-0)
tests/vmtests/test_preserve_bcache.py (+67/-0)
tests/vmtests/test_preserve_lvm.py (+80/-0)
tests/vmtests/test_preserve_partition_wipe_vg.py (+59/-0)
tests/vmtests/test_reuse_lvm_member.py (+34/-0)
tests/vmtests/test_reuse_msdos_partitions.py (+31/-0)
tests/vmtests/test_reuse_raid_member.py (+1/-1)
tests/vmtests/test_ubuntu_core.py (+9/-0)
tools/block-discover-to-config (+1/-1)
tools/schema-validate-storage (+1/-0)
tools/vmtest-filter (+2/-1)
tools/vmtest-sync-images (+1/-0)
tox.ini (+19/-0)
Revision history for this message
seahawk1986 (seahawk1986-hotmail) wrote :
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for filing the bug and including the sfdisk table.

Changed in curtin:
importance: Undecided → High
status: New → Triaged
Revision history for this message
Paride Legovini (paride) wrote :

Hi Ryan,

sfdisk has a --json option that makes it output the partitions in json format. Do you think we could use it to make the parsing more solid?

Revision history for this message
seahawk1986 (seahawk1986-hotmail) wrote :

This seems to be a bug with the same cause: https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1875560

Using json output sounds good, flags are represented as boolean values:
{
   "partitiontable": {
      "label":"dos",
      "id":"0x27690b00",
      "device":"/dev/sda",
      "unit":"sectors",
      "partitions": [
         {"node":"/dev/sda1", "start":2048, "size":116660224, "type":"83", "bootable":true},
         {"node":"/dev/sda2", "start":116664318, "size":8380418, "type":"5"},
         {"node":"/dev/sda5", "start":116664320, "size":8380416, "type":"82"}
      ]
   }
}

Revision history for this message
Paride Legovini (paride) wrote :

sfdisk in <= Trusty does not have --json, but the version in Xenial has it already.

Revision history for this message
Ryan Harper (raharper) wrote :

Yes; already implementing that.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

This bug is fixed with commit 8ab48f64 to curtin on branch master.
To view that commit see the following URL:
https://git.launchpad.net/curtin/commit/?id=8ab48f64

Changed in curtin:
status: Triaged → Fix Committed
Revision history for this message
Michael Wesley (electr0) wrote :

Has this bug been confirmed as fixed?

I'm still experiencing this error, even when using the new installer as prompted during the update.

https://bugs.launchpad.net/subiquity/+bug/1878890

Unless something else is going on in with my installation...?

Revision history for this message
Ryan Harper (raharper) wrote :

@electr0

This bug is fixed, your crash file, we can see sfdisk --json being used:

Verifying /dev/sda2 exists
 Running command ['sfdisk', '--json', '/dev/sda'] with allowed return codes [0] (capture=True)
 Verifying /dev/sda2 size, expecting 86564143104 bytes, found 86564143104 bytes
 Device /dev/sda2 ptable entry: {
  "node": "/dev/sda2",
  "size": 169070592,
  "start": 61442048,
  "type": "83"
 }
 Verifying /dev/sda2 partition flag, expecting linux, found None
 An error occured handling 'partition-sda2': RuntimeError - Verifying /dev/sda2 partition flag, expecting linux, found None

This path is a new, we'll fix this in https://bugs.launchpad.net/ubuntu/+source/curtin/+bug/1878890

Revision history for this message
Kai Harrekilde-Petersen (kaihp) wrote :

@raharper

I can confirm that this fixes the original problem I reported under https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1875560

Unfortunately other bugs werre hiding behind this for me: curtin now barfs on /dev/sda2 (if it's Type=0x83) or on /dev/sda3 (if /dev/sda2 type=0x00). Seems like that curtin is very sensitive (too sensitive!)to how preexisting partition layouts look like.

I added the bugs to my subiquity bug report including logs.

Revision history for this message
Ryan Harper (raharper) wrote :

@Kai

Thanks for the confirmation and following up in the other bug reports. In my comment #9, I believe LP: #1878890 will resolve your current issue.

Revision history for this message
Kai Harrekilde-Petersen (kaihp) wrote :

@Ryan

Agreed, LP: #1878890 definititely smells like fish.

Revision history for this message
Ryan Harper (raharper) wrote : Fixed in curtin version 20.1.

This bug is believed to be fixed in curtin in version 20.1. If this is still a problem for you, please make a comment and set the state back to New

Thank you.

Changed in curtin:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.