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

Bug #1598310 reported by Wesley Wiedenmeier
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
curtin
Undecided
Unassigned
curtin (Ubuntu)
Undecided
Unassigned
Xenial
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)

Related branches

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  Edit
Everyone can see this information.

Other bug subscribers