SRU ndctl into Bionic

Bug #1781268 reported by Andreas Hasenack
20
This bug affects 2 people
Affects Status Importance Assigned to Milestone
ndctl (Ubuntu)
Fix Released
Undecided
Andreas Hasenack
Bionic
Fix Released
Undecided
Unassigned

Bug Description

[Impact]
The support for Intel NVDIMM technology requires both kernel and userspace components. The kernel components have landed in 4.15 and are available in Bionic. In order for users to take advantage of NVDIMMs in their deployments, the following userspace packages are also needed in Bionic:

ndctl -- Utility library for managing the libnvdimm (non-volatile memory device) sub-system in the Linux kernel.
Upstream project: https://github.com/pmem/ndctl

PMDK -- persistent memory toolkit
Upstream project: https://github.com/pmem/pmdk/

This SRU is about ndctl; pmdk is being handled in bug #1781269. Both should be accepted at the same time.

This SRU falls under the hardware enablement category.

[Test Case]
* Install daxctl and ndctl
sudo apt install daxctl ndctl

* download the two dep8 test scripts:
wget https://git.launchpad.net/~usd-import-team/ubuntu/+source/ndctl/plain/debian/tests/daxctl-commands https://git.launchpad.net/~usd-import-team/ubuntu/+source/ndctl/plain/debian/tests/ndctl-commands

* execute them and verify exit status, which should be zero:
sh ./daxctl-commands; echo $?
sh ./ndctl-commands; echo $?

More detailed testing can be executed if the appropriate hardware is available.

[Regression Potential]
These are NEW packages for bionic. They were accepted into the archive after a lengthy review. It is lintian clear, with appropriate overrides or an explanation in README.source.

This package includes libraries, which could inadvertently become new dependencies. If a package has support for ndctl, for example, but it was always disabled because there was no ndctl in the archive, it could now be inadvertently enabled merely by having the lib*-dev package(s) installed.

Similarly, code paths in existing packages that would never have been traversed because of the lack of ndctl support, now could be exposed.

[Other Info]
Since the package doesn't exist in bionic, I don't think I can make an MP.

Here is the branch that represents the bionic source package, though:
https://code.launchpad.net/~ahasenack/ubuntu/+source/ndctl/+git/ndctl/+ref/bionic-ndctl. It's just cosmic with a changelog update for this sru.

You should be able to build a source package from that. Alternatively, you can download it from ppa:canonical-server/nvdimm, but the packages there have a ~ppaN suffix, so keep that in mind.

The git master branch at https://git.launchpad.net/~ahasenack/ubuntu/+source/ndctl/log/?h=master shows all the work that was put into this package to make it suitable for Ubuntu inclusion.
The bug at https://bugs.launchpad.net/bugs/1752378 also contains a lengthy history and discussions about some of the finer details, and is linked to from files in debian/* where further explanation was necessary about some behavior.

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

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

Changed in ndctl (Ubuntu):
status: New → Confirmed
description: updated
description: updated
description: updated
description: updated
description: updated
Changed in ndctl (Ubuntu):
milestone: ubuntu-18.04.1 → bionic-updates
status: Confirmed → In Progress
description: updated
description: updated
description: updated
description: updated
Revision history for this message
Robie Basak (racb) wrote :

Sponsored. Now in Bionic NEW.

Revision history for this message
Łukasz Zemczak (sil2100) wrote : Please test proposed package

Hello Andreas, or anyone else affected,

Accepted ndctl into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/ndctl/61.2-0ubuntu1~18.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 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-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. 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 ndctl (Ubuntu Bionic):
status: New → Fix Committed
tags: added: verification-needed verification-needed-bionic
Revision history for this message
Michael Reed (mreed8855) wrote :

I built and installed the packages and executed the tests. Both tests resulted in a return code of zero.

Here is the output:

ubuntu@R740xd-nvme-57:~/tests/ndtl_test$ sh ./daxctl-commands; echo $?
Available commands from daxctl --list-cmds:
version
list
help
Checking we have a working version command:
Version: 61.2+
0

ubuntu@R740xd-nvme-57:~/tests/ndtl_test$ sh ./ndctl-commands; echo $?
Available commands from ndctl --list-cmds:
version
enable-namespace
disable-namespace
create-namespace
destroy-namespace
check-namespace
enable-region
disable-region
enable-dimm
disable-dimm
zero-labels
read-labels
write-labels
init-labels
check-labels
inject-error
update-firmware
inject-smart
wait-scrub
start-scrub
list
help
Checking we have a working version command:
Version: 61.2+
0

Revision history for this message
Michael Reed (mreed8855) wrote :

This was verified without the hardware

tags: added: verification-done-bionic
removed: verification-needed verification-needed-bionic
Revision history for this message
pragyansri.pathi@intel.com (pragyan) wrote :

Michael - Canonical has access to Apache Pass hardware.
Please work with Bradd Fig to get testing access.
Thanks

Revision history for this message
pragyansri.pathi@intel.com (pragyan) wrote :

ndctl v62 will be the appropriate version to be updated.
Thanks

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Waiting for hardware-related verification.

Revision history for this message
Chris Halse Rogers (raof) wrote :

(Marking the non-Bionic task as Fix Released - ndctl 61.2-0ubuntu1 is now in cosmic/universe)

Changed in ndctl (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I got access to hardware.

Revision history for this message
Amit Bhutani (amit.bhutani) wrote :

As mentioned in comment #7 by Pragya @Intel, we should be targeting ndctl v62 for the update. Can we get Canonical to comment on the feasibility of that?

v62 - https://github.com/pmem/ndctl/releases/tag/v62

Revision history for this message
Andreas Hasenack (ahasenack) wrote : Re: [Bug 1781268] Re: SRU ndctl into Bionic
Download full text (5.5 KiB)

This was my email exchange with <email address hidden> on
September 10th. I haven't heard back since.

On Sat, Sep 8, 2018 at 6:32 PM Pathi, Pragyansri
<email address hidden> wrote:
>
> Thank you
> We will test both in AEP environment.

Thanks, please update the bugs when you have done so.

>
> Andreas ndctl v62 would be an important version for Canonical to update.

Ubuntu 18.10 is in feature freeze mode since Aug 23rd (see
https://wiki.ubuntu.com/CosmicCuttlefish/ReleaseSchedule), and I can't
update the ndctl version there without a justification in the form of
a Feature Freeze Exception
(https://wiki.ubuntu.com/FreezeExceptionProcess). Furhtermore, that
ndctl v62 update has a new daemon which fails to start if the hardware
isn't available, and that fails the package installation, which is
something we cannot have. I will need to find ways to handle this in a
more graceful manner.

That being said, are there important bug fixes that exist in ndctl v62
that make v61.2 unsuitable for release in both Ubuntu 18.10 and Ubuntu
18.04?

Thanks

On Tue, Sep 25, 2018, 14:05 Amit Bhutani <email address hidden> wrote:

> As mentioned in comment #7 by Pragya @Intel, we should be targeting
> ndctl v62 for the update. Can we get Canonical to comment on the
> feasibility of that?
>
>
> v62 - https://github.com/pmem/ndctl/releases/tag/v62
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1781268
>
> Title:
> SRU ndctl into Bionic
>
> Status in ndctl package in Ubuntu:
> Fix Released
> Status in ndctl source package in Bionic:
> Fix Committed
>
> Bug description:
> [Impact]
> The support for Intel NVDIMM technology requires both kernel and
> userspace components. The kernel components have landed in 4.15 and are
> available in Bionic. In order for users to take advantage of NVDIMMs in
> their deployments, the following userspace packages are also needed in
> Bionic:
>
> ndctl -- Utility library for managing the libnvdimm (non-volatile memory
> device) sub-system in the Linux kernel.
> Upstream project: https://github.com/pmem/ndctl
>
> PMDK -- persistent memory toolkit
> Upstream project: https://github.com/pmem/pmdk/
>
> This SRU is about ndctl; pmdk is being handled in bug #1781269. Both
> should be accepted at the same time.
>
> This SRU falls under the hardware enablement category.
>
> [Test Case]
> * Install daxctl and ndctl
> sudo apt install daxctl ndctl
>
> * download the two dep8 test scripts:
> wget
> https://git.launchpad.net/~usd-import-team/ubuntu/+source/ndctl/plain/debian/tests/daxctl-commands
> https://git.launchpad.net/~usd-import-team/ubuntu/+source/ndctl/plain/debian/tests/ndctl-commands
>
> * execute them and verify exit status, which should be zero:
> sh ./daxctl-commands; echo $?
> sh ./ndctl-commands; echo $?
>
> More detailed testing can be executed if the appropriate hardware is
> available.
>
> [Regression Potential]
> These are NEW packages for bionic. They were accepted into the archive
> after a lengthy review. It is lintian clear, with appropriate overrides or
> an explanation in README.source....

Read more...

Revision history for this message
Dan Williams (djbw) wrote :

The only fix in ndctl v62 that would rise to the level of a feature freeze exception is a workaround for a kernel bug:

    a91dff022cd7 ndctl: Work around kernel memory corruption

However, it would be better to ensure that the kernel bug is fixed. This is the upstream commit and I can confirm it has been backported to 4.14-stable and 4.9-stable.

    286e87718103 libnvdimm: fix ars_status output length calculation

I have recorded the the problem with the monitor failing to start if no hardware is present, we'll look to get that rolled into the upcoming v63 release.

Revision history for this message
pragyansri.pathi@intel.com (pragyan) wrote :

Andreas

In response to your comment: (This was my email exchange with <email address hidden> on September 10th. I haven't heard back since)- Intel plans to test Ubuntu 18.10 Beta image. I assumed this will be released Sep 27 - Has this changed?
If it is available let us know.

Thanks
Pragyan

Revision history for this message
Chris Halse Rogers (raof) wrote :

From what I can tell ndctl 61.2-0ubuntu1~18.04.1 is a candidate for release into bionic-updates, but from the commentary here it is unclear whether it is worth releasing that or whether we should wait for a v62 (or v63?) upload.

Should we release ndctl 61.2-0ubuntu1~18.04.1 into bionic-updates?

Revision history for this message
pragyansri.pathi@intel.com (pragyan) wrote :

Chris (Canonical team)
Please proceed with ndctl v61.2 as planned for Ubuntu 18.04.1.
Thank you for your support
Pragyan

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

I did a quick smoke test on an nvdimm-enabled machine and "ndctl list --dimms" returned a list of the modules (6 out of 8, not sure why 2 were missing), whereas on another machine without the nvdimm modules nothing was returned, not even with the memmap kernel command line option.

Revision history for this message
Dan Williams (djbw) wrote :

A couple notes. It is possible that one or more DIMMs are disabled. You can list disabled devices by specifying "-i" to ndctl list:

    ndctl list -Di

One reason for a DIMM to not be recognized is a failure reading the DIMM label area.

As for the the memmap= option, it is expected that there are no DIMM devices associated with that. Only the ACPI NFIT provides the information to associate DIMMs to an address range, the memmap= option only specifies an address range with no associated DIMM information.

Revision history for this message
Andreas Hasenack (ahasenack) wrote :

Thanks for the clarification. The -Di parameter indeed listed all 8 modules. I had forgotten that dmesg had some errors about two modules:

# dmesg|grep nvdimm
[ 7.746213] nvdimm: probe of nmem3 failed with error -5
[ 8.217295] nvdimm: probe of nmem5 failed with error -5

About memmap, yes, I forgot to mention that I expected no output from ndctl list when using memmap on a machine that has no nvdimm modules. I used that as a control machine, for comparison.

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

This bug was fixed in the package ndctl - 61.2-0ubuntu1~18.04.1

---------------
ndctl (61.2-0ubuntu1~18.04.1) bionic; urgency=medium

  * Backport to bionic (LP: #1781268)

 -- Andreas Hasenack <email address hidden> Wed, 01 Aug 2018 17:37:39 -0300

Changed in ndctl (Ubuntu Bionic):
status: Fix Committed → Fix Released
Revision history for this message
Steve Langasek (vorlon) wrote : Update Released

The verification of the Stable Release Update for ndctl 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.

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.