Cannot resize partitions on NVME devices due to bad device name parsing

Bug #1733276 reported by Trent Lloyd
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
partman-partitioning
Fix Released
Unknown
partman-partitioning (Ubuntu)
Fix Released
Undecided
Dimitri John Ledkov
Trusty
Fix Released
Medium
Eric Desrochers
Xenial
Fix Released
Medium
Eric Desrochers
Zesty
Won't Fix
Undecided
Unassigned
Artful
Fix Released
Medium
Eric Desrochers
Bionic
Fix Released
Undecided
Dimitri John Ledkov

Bug Description

[Impact]

It is not possible to resize NVME partitions in the Ubiquity installer.
NVME devices have an unusual device format, specifically e.g. /dev/nvme0n1p1 that has two 'sections' that potentially look like partitions.

Based on the error message, it seems that this device name is being parsed incorrectly and it attempts to use the device path /dev/nvme0n1 instead of /dev/nvme0n1p1

** See attachment (screenshot) took in action, illustrating the problem:
- resize.png
- resize_impossible.png
- console_log.png

[Test Case]

To test component(s) in -proposed
--------------------
https://wiki.ubuntu.com/Testing/EnableProposed
[Section: Installation testing using -proposed]

In order to install successfully from these images, you will normally need to tell the installer to fetch its own components from -proposed as well, which is not the default. To do this, add the following boot parameter:

apt-setup/proposed=true
--------------------

You can test this using qemu and a virtual NVME device. If you toggle the same device/partition between a virtual SCSI/IDE and an NVME device - the issue appears and disappears.

Commands you can use to replicate the issue - unpack netboot.tar.gz into the local directory and then

dd if=/dev/zero of=nvme.disk bs=1M count=16384

NVME:
qemu-system-x86_64 -drive file=nvme.disk,if=none,id=drv0,format=raw -device nvme,drive=drv0,serial=foo -enable-kvm -smp 2 -m 1024 -kernel xenial/ubuntu-installer/amd64/linux -initrd xenial/ubuntu-installer/amd64/initrd.gz

SCSI:
qemu-system-x86_64 -drive file=nvme.disk,if=none,id=drv0,format=raw -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drv0,id=virtio-disk0,bootindex=1 -enable-kvm -smp 2 -m 1024 -kernel ubuntu-installer/amd64/linux -initrd ubuntu-installer/amd64/initrd.gz

** See attachment (screenshot) took in action, illustrating how the installer should react with the fix :
xenial_with_fix1.png
xenial_with_fix2.png
xenial_with_fix3.png
xenial_console_with_fix.png

[Regression Potential]

 * Low risk of regression, with this patch, partman, will be more robust and align with what disk_name() in the kernel does (linux.git/tree/block/partition-generic.c). It is sufficient to check
whether the last character is a digit.

* The patch may also fixes (or at least prevent for the future) other potential similar devices issue and not only explicitly fixing and/or benefit NVMe device types.

[Other Info]

* Debian upstream bug :
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=820818

* Debian upstream fix :
https://anonscm.debian.org/cgit/d-i/partman-partitioning.git/commit/?id=01087125e07a7b22da589e8116f9ef7a26275006

* Commit [01087125]
Make get_real_device() both simpler and more generic (See: #820818).
Looking at the implementation of the disk_name() function in the kernel
(linux.git/tree/block/partition-generic.c), it is sufficient to check
whether the last character is a digit.

* $ git describe --contains 01087125e07a7b22da589e8116f9ef7a26275006
116~2

* $ rmadison partman-partitioning
 partman-partitioning | 85ubuntu2 | precise/main/debian-installer
 partman-partitioning | 99ubuntu1 | trusty/main/debian-installer
 partman-partitioning | 110ubuntu4.1 | xenial-updates/main/debian-installer
 partman-partitioning | 114ubuntu2 | zesty/main/debian-installer
 partman-partitioning | 114ubuntu2 | artful/main/debian-installer
 partman-partitioning | 120ubuntu1 | bionic/main/debian-installer

[Original Description]

It is not possible to resize NVME partitions in the Ubiquity installer. This appears to affect multiple filesystem types including NTFS and ext4.

NVME devices have an unusual device format, specifically e.g. /dev/nvme0n1p1 that has two 'sections' that potentially look like partitions.

Based on the error message, it seems that this device name is being parsed incorrectly and it attempts to use the device path /dev/nvme0n1 instead of /dev/nvme0n1p1

You can test this using qemu and a virtual NVME device. If you toggle the same device/partition between a virtual SCSI/IDE and an NVME device - the issue appears and disappears.

Commands you can use to replicate the issue - unpack netboot.tar.gz into the local directory and then

dd if=/dev/zero of=nvme.disk bs=1M count=16384

NVME:
qemu-system-x86_64 -drive file=nvme.disk,if=none,id=drv0,format=raw -device nvme,drive=drv0,serial=foo -enable-kvm -smp 2 -m 1024 -kernel xenial/ubuntu-installer/amd64/linux -initrd xenial/ubuntu-installer/amd64/initrd.gz

SCSI:
qemu-system-x86_64 -drive file=nvme.disk,if=none,id=drv0,format=raw -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drv0,id=virtio-disk0,bootindex=1 -enable-kvm -smp 2 -m 1024 -kernel ubuntu-installer/amd64/linux -initrd ubuntu-installer/amd64/initrd.gz

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in debian-installer (Ubuntu):
status: New → Confirmed
Frode Nordahl (fnordahl)
tags: added: sts
Changed in partman-partitioning:
status: Unknown → Fix Released
Revision history for this message
Frode Nordahl (fnordahl) wrote :

Added debdiff for Bionic.

Will add SRU template and debdiffs for the other releases as soon as we get it landed in the current development release.

Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

The attachment "partman-partitioning-bionic.debdiff" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

tags: added: patch
Revision history for this message
Frode Nordahl (fnordahl) wrote :
tags: added: sts-sru-needed
Changed in partman-partitioning (Ubuntu Bionic):
assignee: nobody → Dimitri John Ledkov (xnox)
milestone: none → ubuntu-17.11
Revision history for this message
Dan Streetman (ddstreet) wrote :

This has been fixed in Bionic by pulling partman-partitioning-120 from debian (artful has version -114). The newer version has a more comprehensive fix, i.e.:

--- partman-partitioning-114/lib/resize.sh 2014-09-04 01:45:57.000000000 -0400
+++ partman-partitioning-120/lib/resize.sh 2017-06-24 23:04:17.000000000 -0400
@@ -18,15 +18,12 @@
   num=$(sed 's/^[^0-9]*\([0-9]*\)[^0-9].*/\1/' $backupdev/$oldid/view)
   bdev=$(cat $backupdev/device)
   case $bdev in
- */disc)
- bdev=${bdev%/disc}/part$num
+ /dev/*[0-9])
+ bdev=${bdev}p$num
    ;;
- /dev/[hsv]d[a-z]|/dev/xvd[a-z])
+ /dev/*)
    bdev=$bdev$num
    ;;
- /dev/cciss/c[0-9]d[0-9]|/dev/cciss/c[0-9]d[0-9][0-9]|/dev/ida/c[0-9]d[0-9]|/dev/ida/c[0-9]d[0-9][0-9]|/dev/mmcblk[0-9])
- bdev=${bdev}p$num
- ;;
       *)
    log "get_real_device: strange device name $bdev"
    return

Revision history for this message
Dan Streetman (ddstreet) wrote :

newer patch is https://anonscm.debian.org/cgit/d-i/partman-partitioning.git/commit/?id=01087125e07a7b22da589e8116f9ef7a26275006

while older, simpler patch is https://anonscm.debian.org/cgit/d-i/partman-partitioning.git/commit/lib/resize.sh?id=348a501524e7a2cdd3e04d5ec1c9f9d2aead3743

the newer, more complete patch might be better as it should cover more devices than just nvme, but for now the older simpler patch should work for this bug too.

Eric Desrochers (slashd)
description: updated
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :

[01087125] is already in place in our current development release (bionic) and it indeed look more complete and align with disk_name() kernel function.

A/X/T are affected.

lib/resize.sh
     21 case $bdev in
     22 /dev/*[0-9])
     23 bdev=${bdev}p$num
     24 ;;
     25 /dev/*)
     26 bdev=$bdev$num
     27 ;;
     28 *)
     29 log "get_real_device: strange device name $bdev"
     30 return
     31 ;;
     32 esac

- Eric

Changed in partman-partitioning (Ubuntu Bionic):
status: New → Fix Released
Eric Desrochers (slashd)
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :

attachment to show an example of what the bug does in real life
[resize.png]
[resize_impossible.png]

Revision history for this message
Eric Desrochers (slashd) wrote :

attachment to show an example of what the bug does in real life
[resize.png]
[resize_impossible.png]

description: updated
Eric Desrochers (slashd)
Changed in partman-partitioning (Ubuntu Artful):
assignee: nobody → Eric Desrochers (slashd)
Changed in partman-partitioning (Ubuntu Zesty):
status: New → Won't Fix
Changed in partman-partitioning (Ubuntu Xenial):
assignee: nobody → Eric Desrochers (slashd)
Changed in partman-partitioning (Ubuntu Trusty):
assignee: nobody → Eric Desrochers (slashd)
Changed in partman-partitioning (Ubuntu Artful):
status: New → In Progress
importance: Undecided → Medium
Changed in partman-partitioning (Ubuntu Xenial):
importance: Undecided → Medium
status: New → Fix Released
Changed in partman-partitioning (Ubuntu Trusty):
importance: Undecided → Medium
status: New → In Progress
description: updated
Eric Desrochers (slashd)
Changed in partman-partitioning (Ubuntu Xenial):
status: Fix Released → In Progress
Revision history for this message
Eric Desrochers (slashd) wrote :

debdiff for xenial

Revision history for this message
Eric Desrochers (slashd) wrote :

debdiff for artful

Eric Desrochers (slashd)
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :

attachment: console_log.png

description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :

I have built partman-partionning (.udeb) and then extract ubuntu 16.04.3 iso to replace ./pool/main/p/partman-partitioning/partman-partitioning_110ubuntu4.1_amd64.udeb by ./pool/main/p/partman-partitioning/partman-partitioning_110ubuntu4.2_amd64.udeb and then create a custom ISO image to validate that the fix work as expected.

and it does, with the fix, the installer is now able to go ahead with the resize and not complaining nor generating errors.

I'll attach screenshot in a few seconds.

- Eric

Revision history for this message
Eric Desrochers (slashd) wrote :
Revision history for this message
Eric Desrochers (slashd) wrote :
Revision history for this message
Eric Desrochers (slashd) wrote :
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :

Uploaded for X/A, it is now waiting on SRU team approval to start building in -proposed.

Will test Trusty and proceed with Trusty upload soon.

- Eric

tags: added: sts-sponsor-slashd-done
no longer affects: debian-installer (Ubuntu)
no longer affects: debian-installer (Ubuntu Trusty)
no longer affects: debian-installer (Ubuntu Xenial)
no longer affects: debian-installer (Ubuntu Zesty)
no longer affects: debian-installer (Ubuntu Artful)
no longer affects: debian-installer (Ubuntu Bionic)
Revision history for this message
Eric Desrochers (slashd) wrote :

Trusty uploaded as well.

Revision history for this message
Chris J Arges (arges) wrote : Please test proposed package

Hello Trent, or anyone else affected,

Accepted partman-partitioning into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/partman-partitioning/110ubuntu4.2 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 on 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-xenial to verification-done-xenial. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-xenial. In either case, without details of your testing we will not be able to proceed.

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

Changed in partman-partitioning (Ubuntu Xenial):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-xenial
Changed in partman-partitioning (Ubuntu Artful):
status: In Progress → Fix Committed
tags: added: verification-needed-artful
Revision history for this message
Chris J Arges (arges) wrote :

Hello Trent, or anyone else affected,

Accepted partman-partitioning into artful-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/partman-partitioning/114ubuntu3 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 on 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-artful to verification-done-artful. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-artful. In either case, without details of your testing we will not be able to proceed.

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

Changed in partman-partitioning (Ubuntu Trusty):
status: In Progress → Fix Committed
tags: added: verification-needed-trusty
Revision history for this message
Chris J Arges (arges) wrote :

Hello Trent, or anyone else affected,

Accepted partman-partitioning into trusty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/partman-partitioning/99ubuntu2 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 on 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-trusty to verification-done-trusty. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-trusty. In either case, without details of your testing we will not be able to proceed.

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

Eric Desrochers (slashd)
description: updated
Revision history for this message
Eric Desrochers (slashd) wrote :

[VERIFICATION ARTFUL]

I have tested the latest artful-proposed mini.iso by enabling the installer to fetch its own components from -proposed, in this case partman-partitioning.

It is now possible to resize a NMVe drive at installation.

- Eric

Revision history for this message
Eric Desrochers (slashd) wrote :

[VERIFICATION XENIAL]

I have tested the latest xenia-proposed mini.iso by enabling the installer to fetch its own components from -proposed, in this case partman-partitioning.

It is now possible to resize a NMVe drive at installation.

- Eric

tags: added: verification-done-artful verification-done-xenial
removed: verification-needed-artful verification-needed-xenial
Revision history for this message
Eric Desrochers (slashd) wrote :

[VERIFICATION TRUSTY]

I have tested the latest trusty-proposed mini.iso by enabling the installer to fetch its own components from -proposed, in this case partman-partitioning.

It is now possible to resize a NMVe drive at installation.

- Eric

tags: added: verification-done-trusty
removed: verification-needed-trusty
Revision history for this message
Aydin Doyak (aydintd) wrote :

Hello all,

I've also tested below procedure to see if there's any impacts on resizing "non-NVMe" devices by using the mini.iso enabling the installer to fetch necessarry components from -proposed.

Here's the mini.iso URL I've tested: http://archive.ubuntu.com/ubuntu/dists/xenial-proposed/main/installer-amd64/20101020ubuntu451.18/images/netboot/mini.iso

Test phases:

1. Created a QEMU-KVM virtual machine with qcow2 formatted IDE BUS disk (/dev/sda)
2.I installed a Ubuntu 16.04 system on that qemu-kvm virtual machine via passing "apt-setup/proposed=true" value to the installers boot parameters without problem. I also used guided partitioning on the disks and continued with the installation.
3. I ensured that the VM is working without problems after installation.
4. I rebooted the VM with that mini.iso again with the same "apt-setup/proposed=true" parameter value to the ubuntu installer and resized the disk succesfully to an another size. Installed the system again on the new partitions without problems.
5. Ensured that the VM is working without problems after installation.

-
Aydin

Revision history for this message
Eric Desrochers (slashd) wrote :

Attaching a screenshot proving the installer installed partman-partitioning version "110ubuntu4.2" for Xenial (as additional information) as it should when using the boot param: apt-setup/proposed=true.

# rmadison
 partman-partitioning | 110ubuntu4.2 | xenial-proposed | source
 partman-partitioning | 110ubuntu4.2 | xenial-proposed/main/debian-installer | amd64, arm64, armhf, i386, powerpc, ppc64el, s390x

tags: added: verification-done
removed: verification-needed
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for partman-partitioning 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
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package partman-partitioning - 110ubuntu4.2

---------------
partman-partitioning (110ubuntu4.2) xenial; urgency=medium

  * Make get_real_device() both simpler and more generic.
    (LP: #1733276) (Closes: #820818).

 -- Eric Desrochers <email address hidden> Mon, 05 Feb 2018 12:04:22 -0500

Changed in partman-partitioning (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package partman-partitioning - 99ubuntu2

---------------
partman-partitioning (99ubuntu2) trusty; urgency=medium

  * Make get_real_device() both simpler and more generic.
    (LP: #1733276) (Closes: #820818).

 -- Eric Desrochers <email address hidden> Tue, 06 Feb 2018 12:08:28 -0500

Changed in partman-partitioning (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package partman-partitioning - 114ubuntu3

---------------
partman-partitioning (114ubuntu3) artful; urgency=medium

  * Make get_real_device() both simpler and more generic.
    (LP: #1733276) (Closes: #820818).

 -- Eric Desrochers <email address hidden> Mon, 05 Feb 2018 12:12:45 -0500

Changed in partman-partitioning (Ubuntu Artful):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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