[Bug 2051299] Re: nvme-cli: fguid is printed as binary data and causes MAAS to fail erasing NVME disks

[Matthew Ruffell]

Performing verification for jammy.

I started a fresh jammy VM, that had an attached 50gb NVMe device:

$ lsblk
nvme0n1      259:0    0 46.6G  0 disk 
nvme1n1      259:1    0    8G  0 disk 
├─nvme1n1p1  259:2    0  7.9G  0 part /
├─nvme1n1p14 259:3    0    4M  0 part 
└─nvme1n1p15 259:4    0  106M  0 part /boot/efi

I installed nvme-cli 1.16-3ubuntu0.1 from -updates, and ran:

$ sudo nvme id-ctrl /dev/nvme0n1 | grep fguid
fguid     : 

$ sudo nvme id-ctrl -o json /dev/nvme0n1 | grep fguid
$

The fguid on this VM is set to all zeros, so they are being interpreted
as a null byte and we get no output for this field.

I then enabled -proposed and installed nvme-cli 1.16-3ubuntu0.2, and re-
ran the following:

$ sudo nvme id-ctrl /dev/nvme0n1 | grep fguid
fguid     : 00000000-0000-0000-0000-000000000000

$ sudo nvme id-ctrl -o json /dev/nvme0n1 | grep fguid
  "fguid" : "00000000-0000-0000-0000-000000000000",
  
Now we are actually getting correct output, and see the actual UUID value being printed.

I also ran the following python script that more or less does what
maas_wipe.py does:

import subprocess

output = subprocess.check_output(["nvme", "id-ctrl", "/dev/nvme0n1"])
output = output.decode()

It completes successfully under 1.16-3ubuntu0.2, so I think MAAS is good
to go now.

The package in -proposed fixes the problem. Happy to mark verified for
jammy.

** Tags removed: verification-needed verification-needed-jammy
** Tags added: verification-done-jammy

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to nvme-cli in Ubuntu.
https://bugs.launchpad.net/bugs/2051299

Title:
  nvme-cli: fguid is printed as binary data and causes MAAS to fail
  erasing NVME disks

Status in MAAS:
  Triaged
Status in nvme-cli package in Ubuntu:
  Fix Released
Status in nvme-cli source package in Jammy:
  Fix Committed

Bug description:
  [Impact]

  When a user tries to release a system deployed with MAAS, that has
  erase disks on release set, erasing NVME disks fails on Jammy.

  Traceback (most recent call last):
  File "/tmp/user_data.sh.jNE4lC/bin/maas-wipe", line 542, in <module>
  main()
  File "/tmp/user_data.sh.jNE4lC/bin/maas-wipe", line 522, in main
  disk_info = get_disk_info()
  File "/tmp/user_data.sh.jNE4lC/bin/maas-wipe", line 165, in get_disk_info
  return {kname: get_disk_security_info(kname) for kname in list_disks()}
  File "/tmp/user_data.sh.jNE4lC/bin/maas-wipe", line 165, in <dictcomp>
  return {kname: get_disk_security_info(kname) for kname in list_disks()}
  File "/tmp/user_data.sh.jNE4lC/bin/maas-wipe", line 158, in get_disk_security_info
  return get_nvme_security_info(disk)
  File "/tmp/user_data.sh.jNE4lC/bin/maas-wipe", line 64, in get_nvme_security_info
  output = output.decode()
  UnicodeDecodeError: 'utf-8' codec can't decode byte 0x89 in position 385: invalid start byte

  This is due to maas_wipe.py running "nvme id-ctrl <device>" and
  parsing the results. This should be human readable data, in string
  format, so utf-8 should be appropriate for MAAS to use.

  Instead, the "fguid" field is being printed as binary data, and is not
  parsable as utf-8.

  e.g. From comment #8.

  The user sees:

  `fguid : 2.`

  on closer inspection, the hex is:

  x32,0x89,0x82,0x2E

  Note it is cut off early, likely because the next byte would be 0x00,
  and is being interprested as a null byte.

  Fix nvme-cli such that we print out the fguid as a correct utf-8
  string, so MAAS works as intended.

  [Testcase]

  Deploy Jammy onto a system that has a NVME device.

  $ sudo apt install nvme-cli

  Run the 'id-ctrl' command and look at the fguid entry:

  $ sudo nvme id-ctrl /dev/nvme1n1 | grep fguid
  fguid     :

  Due to the UUID being all zeros, this was interpreted as a null byte,
  and the UUID was not printed correctly.

  There is a test package available in the following ppa:

  https://launchpad.net/~mruffell/+archive/ubuntu/sf387274-test

  If you install the test package, the fguid will be printed as a proper
  string:

  $ sudo nvme id-ctrl /dev/nvme1n1 | grep fguid
  fguid     : 00000000-0000-0000-0000-000000000000

  Also check that json output works as expected:

  $ sudo nvme id-ctrl -o json /dev/nvme1n1 | grep fguid
    "fguid" : "00000000-0000-0000-0000-000000000000",

  Additionally, also test that the new package allows a MAAS deployed system to
  be released correctly with the erase option enabled, as maas_wipe.py should now
  complete successfully.

  [Where problems could occur]

  We are changing the output of the 'id-ctrl' subcommand. No other
  subcommands are changed. Users who for some reason rely on broken,
  incomplete binary data that is printed might be impacted. For users
  doing a hard diff of the command output, the output will now change to
  reflect the actual fguid, and might need a change. The fguid is now
  supplied in json output for 'id-ctrl', and might change programs
  parsing the json object.

  There are no workarounds, and if a regression were to occur, it would
  only affect the 'id-ctrl' subcommand, and not change anything else.

  [Other info]

  Upstream bug:
  https://github.com/linux-nvme/nvme-cli/issues/1653

  This was fixed in the below commit in version 2.2, found in mantic and
  later:

  commit 78b7ad235507ddd59c75c7fcc74fc6c927811f87
  From: Pierre Labat <plabat at micron.com>
  Date: Fri, 26 Aug 2022 17:02:08 -0500
  Subject: nvme-print: Print fguid as a UUID
  Link: https://github.com/linux-nvme/nvme-cli/commit/78b7ad235507ddd59c75c7fcc74fc6c927811f87

  The commit required a minor backport. In later versions, a major
  refactor occurred that changed nvme_uuid_to_string() among numerous
  other functions, that is not appropriate to backport. Instead, just
  take the current implementation of nvme_uuid_to_string() and move it
  like the patch suggests, so json output works correctly.

To manage notifications about this bug go to:
https://bugs.launchpad.net/maas/+bug/2051299/+subscriptions