Hi @slashd and @chiluk,

As @chiluk mentioned, this feature is implemented as a standalone plugin
which has no influence over any of the existing source code, and instead
adds code which is only reachable through a new subcommand. Even if a
regression was introduced, it would be limited to users of the "nvme
micron" subcommand only.

I did a diff of the proposed patch to the current upstream master
branch, and the new changes are all more or less cosmetic:

https://paste.ubuntu.com/p/QPkVkNrzVf/

If you would like to review each commit individually, I have provided
them below:

1d37bc9 nvme-cli: Code cleanup
507ded5 nvme-vendor: fix c99 declarations in vendor plugins
c36e848 nvme: Unify min(), max() macro as a common one
661ad0f micron-nvme: Replace direct use of ioctl
f392181 Refactor plugins in a file hierarchy

None of these are direct bug fixes, and are not necessary to carry in
this SRU.

I went and performed autopkgtest on the updated package, but
unfortunately there are no autopkgtests included with this package:

https://paste.ubuntu.com/p/Ky7V4Mr8fM/

The code being introduced is compatible across kernels, as we are only
opening a device interface up for writing, and is compatible across the
default and subsequent HWE kernels.

As for "Other safe cases", I believe that this SRU fulfils the first
three cases for applicability while also improving the user experience
of those with Micron NVMe devices.

For case one:
1.) "Has an obviously safe patch" - we are adding a standalone plugin which has 
no impact on existing code, since it implements a new subcommand, specifically 
for use of Micron hardware.
2.) "affects an application" - changes are limited to nvme-cli only. Kernel 
support is already present.

For case two:
This SRU will enable the use of Micron NVMe devices, while not affecting any 
existing NVMe devices from other manufacturers, since the subcommand is 
implemented alongside other manufactures subcommands, and does not modify any 
existing code.

For case three:
The changes are unintrusive, again, to being limited to plugin which implements 
a new subcommand. The code itself is simplistic, and has not been dramatically 
modified since it first landed in upstream compared to the present day. The 
feature landed in nvme-cli version 1.6, which appears in cosmic onwards, 
meaning upgrading from bionic to a newer release will not cause upgrade 
regressions.

The test package in the SRU template has been tested by the customer on
a Micron NVMe device, although I do not have any actual output of the
commands ran. The customer ran the "nvme micron select-download" command
to update the device firmware, and managed to format a drive to 4K block
size.

When the package hits -proposed, I can ask the customer for explicit
output of commands ran if that is necessary, during the verification
process.

Thanks!

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

Title:
  nvme-cli 1.5 in Bionic does not support Micron NVME drives

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/nvme-cli/+bug/1838555/+subscriptions

-- 
ubuntu-bugs mailing list
[email protected]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to