Ubuntu
curtin package

Curtin block.get_blockdev_sector_size incorrectly assumes block._lsblock will return a dictionary with only a single entry

  1. Xenial (16.04)
  2. Bug #1598310
Bug #1598310 reported by Wesley Wiedenmeier
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
curtin
Fix Released
Undecided
Unassigned
curtin (Ubuntu)
Fix Released
Undecided
Unassigned
Xenial
Fix Released
Undecided
Unassigned

Bug Description

[Impact]

 * Curtin block module method `get_blockdev_sector_size` fails when
   pointed at a device with multiple partitions instead of only
   a partition.

   Curtin has been updated to filter the results to find the expected
   device path and return results for that device.

[Test Case]

 * Install proposed curtin package on a system with a block device with
   multiple partitions.
   - python3 -c 'from curtin import block;
                 block.get_blockdev_sector_size("/dev/sda")'

  PASS: method call returns tuple with values
   - (512, 512)

  FAIL: method call fails with stacktrace like
   - Traceback (most recent call last):
   File "<stdin>", line 1, in <module>
   File "curtin/curtin/block/__init__.py", line 426,
    in get_blockdev_sector_size
     [parent] = info
   ValueError: too many values to unpack (expected 1)

[Regression Potential]

 * Third-party consumers of curtin modules may have relied on this failure
   to detect devices which contained partitions.

[Original Description]
The function curtin.get_blockdev_sector_size will not work when called with a devpath which lsblk will return multiple lines of results for.

This means that in some cases formatting a device other than a partition may fail, because block.mkfs checks for logical block size using this function.

For example:
Python 3.5.1+ (default, Mar 30 2016, 22:46:26)
[GCC 5.3.1 20160330] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from curtin import block
>>> block.get_blockdev_sector_size('/dev/sda1')
(512, 512)
>>> block.get_blockdev_sector_size('/dev/sda5')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/magicanus/code/curtin-clean/curtin/curtin/block/__init__.py", line 426, in get_blockdev_sector_size
    [parent] = info
ValueError: too many values to unpack (expected 1)
>>> block.get_blockdev_sector_size('/dev/sda')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/magicanus/code/curtin-clean/curtin/curtin/block/__init__.py", line 426, in get_blockdev_sector_size
    [parent] = info
ValueError: too many values to unpack (expected 1)

See original description

Related branches

lp://staging/~wesley-wiedenmeier/curtin/1598310
Server Team CI bot: Approve (continuous-integration)
Ryan Harper (community): Approve
Diff: 73 lines (+44/-1)
2 files modified
curtin/block/__init__.py (+12/-1)
tests/unittests/test_block.py (+32/-0)
lp://staging/~curtin-dev/curtin/trunk
lp://staging/~smoser/ubuntu/yakkety/curtin/pkg
lp://staging/~raharper/ubuntu/xenial/curtin/pkg-sru-revno425
Scott Moser: Pending requested
Diff: 14513 lines (+10324/-1893)
94 files modified
Makefile (+3/-1)
curtin/__init__.py (+4/-0)
curtin/block/__init__.py (+249/-61)
curtin/block/clear_holders.py (+387/-0)
curtin/block/lvm.py (+96/-0)
curtin/block/mdadm.py (+18/-5)
curtin/block/mkfs.py (+10/-5)
curtin/commands/apply_net.py (+156/-1)
curtin/commands/apt_config.py (+668/-0)
curtin/commands/block_info.py (+75/-0)
curtin/commands/block_meta.py (+134/-263)
curtin/commands/block_wipe.py (+1/-2)
curtin/commands/clear_holders.py (+48/-0)
curtin/commands/curthooks.py (+61/-235)
curtin/commands/main.py (+4/-3)
curtin/config.py (+2/-3)
curtin/gpg.py (+74/-0)
curtin/net/__init__.py (+67/-30)
curtin/net/network_state.py (+45/-1)
curtin/util.py (+278/-81)
debian/changelog (+32/-2)
doc/conf.py (+21/-4)
doc/devel/README-vmtest.txt (+0/-152)
doc/devel/README.txt (+0/-55)
doc/devel/clear_holders_doc.txt (+85/-0)
doc/index.rst (+6/-0)
doc/topics/apt_source.rst (+164/-0)
doc/topics/config.rst (+551/-0)
doc/topics/development.rst (+68/-0)
doc/topics/integration-testing.rst (+245/-0)
doc/topics/networking.rst (+522/-0)
doc/topics/overview.rst (+7/-7)
doc/topics/reporting.rst (+3/-3)
doc/topics/storage.rst (+894/-0)
examples/apt-source.yaml (+267/-0)
examples/network-ipv6-bond-vlan.yaml (+56/-0)
examples/tests/apt_config_command.yaml (+85/-0)
examples/tests/apt_source_custom.yaml (+97/-0)
examples/tests/apt_source_modify.yaml (+92/-0)
examples/tests/apt_source_modify_arches.yaml (+102/-0)
examples/tests/apt_source_modify_disable_suite.yaml (+92/-0)
examples/tests/apt_source_preserve.yaml (+98/-0)
examples/tests/apt_source_search.yaml (+97/-0)
examples/tests/basic.yaml (+5/-1)
examples/tests/basic_network_static_ipv6.yaml (+22/-0)
examples/tests/basic_scsi.yaml (+1/-1)
examples/tests/network_alias.yaml (+125/-0)
examples/tests/network_mtu.yaml (+88/-0)
examples/tests/network_source_ipv6.yaml (+31/-0)
examples/tests/test_old_apt_features.yaml (+11/-0)
examples/tests/test_old_apt_features_ports.yaml (+10/-0)
examples/tests/uefi_basic.yaml (+15/-0)
examples/tests/vlan_network_ipv6.yaml (+92/-0)
setup.py (+2/-2)
tests/unittests/helpers.py (+41/-0)
tests/unittests/test_apt_custom_sources_list.py (+170/-0)
tests/unittests/test_apt_source.py (+1032/-0)
tests/unittests/test_block.py (+210/-0)
tests/unittests/test_block_lvm.py (+94/-0)
tests/unittests/test_block_mdadm.py (+28/-23)
tests/unittests/test_block_mkfs.py (+2/-2)
tests/unittests/test_clear_holders.py (+329/-0)
tests/unittests/test_make_dname.py (+200/-0)
tests/unittests/test_net.py (+54/-13)
tests/unittests/test_util.py (+180/-2)
tests/vmtests/__init__.py (+38/-38)
tests/vmtests/helpers.py (+129/-166)
tests/vmtests/test_apt_config_cmd.py (+55/-0)
tests/vmtests/test_apt_source.py (+238/-0)
tests/vmtests/test_basic.py (+21/-41)
tests/vmtests/test_bcache_basic.py (+5/-8)
tests/vmtests/test_bonding.py (+0/-204)
tests/vmtests/test_lvm.py (+2/-1)
tests/vmtests/test_mdadm_bcache.py (+21/-17)
tests/vmtests/test_multipath.py (+5/-13)
tests/vmtests/test_network.py (+205/-348)
tests/vmtests/test_network_alias.py (+40/-0)
tests/vmtests/test_network_bonding.py (+63/-0)
tests/vmtests/test_network_enisource.py (+91/-0)
tests/vmtests/test_network_ipv6.py (+53/-0)
tests/vmtests/test_network_ipv6_enisource.py (+26/-0)
tests/vmtests/test_network_ipv6_static.py (+42/-0)
tests/vmtests/test_network_ipv6_vlan.py (+34/-0)
tests/vmtests/test_network_mtu.py (+155/-0)
tests/vmtests/test_network_static.py (+44/-0)
tests/vmtests/test_network_vlan.py (+77/-0)
tests/vmtests/test_nvme.py (+2/-3)
tests/vmtests/test_old_apt_features.py (+89/-0)
tests/vmtests/test_raid5_bcache.py (+5/-8)
tests/vmtests/test_uefi_basic.py (+16/-18)
tools/jenkins-runner (+33/-7)
tools/launch (+9/-48)
tools/xkvm (+90/-2)
tox.ini (+30/-13)
Wesley Wiedenmeier (wesley-wiedenmeier)
Changed in curtin:
status: New → Fix Committed
Ryan Harper (raharper)
description: updated
Revision history for this message
Andy Whitcroft (apw) wrote : Please test proposed package

Hello Wesley, or anyone else affected,

Accepted curtin into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/curtin/0.1.0~bzr425-0ubuntu1~16.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

tags: added: verification-needed
Andy Whitcroft (apw)
Changed in curtin (Ubuntu):
status: New → Fix Released
Changed in curtin (Ubuntu Xenial):
status: New → Fix Committed
Revision history for this message
Wesley Wiedenmeier (wesley-wiedenmeier) wrote :

I am marking verification-done, as this fixes the issue for me on xenial.

When running with the curtin build currently in xenial-updates (/dev/vdb3 is used as physical volume for several lvm volumes):

  root@ubuntu:/home/ubuntu# apt-cache policy curtin
    curtin:
      Installed: 0.1.0~bzr399-0ubuntu1~16.04.1
      Candidate: 0.1.0~bzr399-0ubuntu1~16.04.1
  root@ubuntu:/home/ubuntu# python3
    Python 3.5.2 (default, Sep 10 2016, 08:21:44)
    [GCC 5.4.0 20160609] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from curtin import block
    >>> block.get_blockdev_sector_size("/dev/vdb3")
      Traceback (most recent call last):
         File "<stdin>", line 1, in <module>
         File "/usr/lib/python3/dist-packages/curtin/block/__init__.py", line 426, in get_blockdev_sector_size
           [parent] = info
      ValueError: too many values to unpack (expected 1)

When running with proposed (note that 4096 is actual sector size as advanced format disk used):

  root@ubuntu:/home/ubuntu# apt-cache policy curtin
    curtin:
      Installed: 0.1.0~bzr425-0ubuntu1~16.04.1
      Candidate: 0.1.0~bzr425-0ubuntu1~16.04.1

  root@ubuntu:/home/ubuntu# python3
    Python 3.5.2 (default, Sep 10 2016, 08:21:44)
    [GCC 5.4.0 20160609] on linux
    Type "help", "copyright", "credits" or "license" for more information.
    >>> from curtin import block
    >>> block.get_blockdev_sector_size("/dev/vdb3")
      (4096, 4096)

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package curtin - 0.1.0~bzr425-0ubuntu1~16.04.1

---------------
curtin (0.1.0~bzr425-0ubuntu1~16.04.1) xenial-proposed; urgency=medium

  [ Scott Moser ]
  * debian/new-upstream-snapshot: add writing of debian changelog entries.

  [ Ryan Harper ]
  * New upstream snapshot.
    - unittest,tox.ini: catch and fix issue with trusty-level mock of open
    - block/mdadm: add option to ignore mdadm_assemble errors (LP: #1618429)
    - curtin/doc: overhaul curtin documentation for readthedocs.org
      (LP: #1351085)
    - curtin.util: re-add support for RunInChroot (LP: #1617375)
    - curtin/net: overhaul of eni rendering to handle mixed ipv4/ipv6 configs
    - curtin.block: refactor clear_holders logic into block.clear_holders and
      cli cmd
    - curtin.apply_net should exit non-zero upon exception. (LP: #1615780)
    - apt: fix bug in disable_suites if sources.list line is blank.
    - vmtests: disable Wily in vmtests
    - Fix the unittests for test_apt_source.
    - get CURTIN_VMTEST_PARALLEL shown correctly in jenkins-runner output
    - fix vmtest check_file_strippedline to strip lines before comparing
    - fix whitespace damage in tests/vmtests/__init__.py
    - fix dpkg-reconfigure when debconf_selections was provided.
      (LP: #1609614)
    - fix apt tests on non-intel arch
    - Add apt features to curtin. (LP: #1574113)
    - vmtest: easier use of parallel and controlling timeouts
    - mkfs.vfat: add force flag for formating whole disks (LP: #1597923)
    - block.mkfs: fix sectorsize flag (LP: #1597522)
    - block_meta: cleanup use of sys_block_path and handle cciss knames
      (LP: #1562249)
    - block.get_blockdev_sector_size: handle _lsblock multi result return
      (LP: #1598310)
    - util: add target (chroot) support to subp, add target_path helper.
    - block_meta: fallback to parted if blkid does not produce output
      (LP: #1524031)
    - commands.block_wipe: correct default wipe mode to 'superblock'
    - tox.ini: run coverage normally rather than separately
    - move uefi boot knowledge from launch and vmtest to xkvm

 -- Ryan Harper <email address hidden> Mon, 03 Oct 2016 13:43:54 -0500

Changed in curtin (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Martin Pitt (pitti) wrote : Update Released

The verification of the Stable Release Update for curtin has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Scott Moser (smoser) wrote : Fixed in Curtin 17.1

This bug is believed to be fixed in curtin in 17.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.