Review for Source Package: nvme-stas [Summary] MIR team ACK under the constraint to resolve the below listed required TODOs and as much as possible having a look at the recommended TODOs. This does need a security review, so I'll assign ubuntu-security List of specific binary packages to be promoted to main: nvme-stas Specific binary packages built, but NOT to be promoted to main: none Recommended TODOs: #1 - The package should get a team bug subscriber before being promoted # 2 - Since this is rather new (not brekaing many existing cases) but doing a very special limited set of things (might be easy to describe) it might be very helpful to consider writing apparmor profiles and/or using more of the system service isolation features. #3 - We are currently in discussions to make it easier for software that needs special hardware to get into main. And this is IMHO such a case. Gladly it only provides usability sugar over what components that are already in main (kernel, libnvme, nvme-cli) already provide. But I've not heard about testing on actual nvme-of hardware. I'd like to ask you to read through [1][2] where we list some ideas how a case like that could be overcome. As often there are so many alone the lower transport nvmet-rdma or a real adapter loading qla2xxx adapter or nvme_tcp - but one would be better than none; Maybe you can even test this fully virtual - think of two qemu guests, one with emulated NVME which it provides via nvme_tcp and the other the client that does access it, test write/loads some and disconnects. Add the auto discovery of stafd/stacd on top and this might be great. To be clear - all that this package adds - is tested well, so this is only a recommended task but if possible would help to QA the whole stack regularly. #4 - Build time tests complain about missing components skipping tests ../test/meson.build:79: WARNING: Dbus daemon is not running ../test/meson.build:85: WARNING: Avahi daemon is not running ../test/meson.build:111: WARNING: Skip Avahi Test due to missing dependencies ../test/meson.build:151: WARNING: Skiping some of the tests because "vermin" is missing. Would it pollute/break the build if you'd build-depend (with !nocheck)? Because if that could work it would be an easy win for QA. If you are concerned of the build env, then consider re-run this test in an autopkgtest stage? [1]: https://github.com/canonical/ubuntu-mir/pull/31 [2]: https://github.com/canonical/ubuntu-mir/issues/30 [Duplication] It is related to some other similar sounding packages like nvme-cli, libnvme and belongs to the same area. But there is no other package in main providing the same functionality as nvme-stas. [Dependencies] OK: - no other Dependencies to MIR due to this - dasbus in requested bug 2025912 - python3-nvme is from libnvme in bug 1998114 and considered fine to come back - no others - no -dev/-debug/-doc packages that need exclusion - No dependencies in main that are only superficially tested requiring more tests now. Problems: None [Embedded sources and static linking] OK: - no embedded source present In fact I appreciate that it is reusing code that was split from nvme-cli (python3-libnvme/libnvme) and already in main for other use cases. This was added/split in bug 1998114 - no static linking - does not have unexpected Built-Using entries - not a go package, no extra constraints to consider in that regard - not a rust package, no extra constraints to consider in that regard Problems: None [Security] OK: - history of CVEs does not look concerning - does not use webkit1,2 - does not use lib*v8 directly - does not process arbitrary web content - does not use centralized online accounts - does not integrate arbitrary javascript into the desktop - does not deal with system authentication (eg, pam), etc) - does not deal with security attestation (secure boot, tpm, signatures) - does not deal with cryptography (en-/decryption, certificates, signing, ...) Problems: - does run a daemon as root, and that reaches out via fabrics network to do things which is an uncommon but possible way of exposure - does parse data formats (files [images, video, audio, xml, json, asn.1], network packets, structures, ...) from an untrusted source. - does not open a port/socket (actually dbus endpoint) [Common blockers] OK: - does not FTBFS currently - does have a test suite that runs at build time - test suite fails will fail the build upon error. - does have a non-trivial test suite that runs as autopkgtest - no new python2 dependency - Python package, but using dh_python Problems: - This does seem to need special HW for in depth test so it can't be automatic. Do me get right, I highly appreciate that there are build tests and a full unit test at autopkgtest time. That is already great, but for the real thing that this is meant to provide you need special HW. Is there a way to provide that either simulated, or at least get started on getting a hold on such hardware, or .... Mentioned in recommended todo. I mean, eventually AssertPathExists=/dev/nvme-fabrics means it mostly does nothing, so I wonder if/where/when it was and will be tested for real. [Packaging red flags] OK: - Ubuntu does not carry a delta - symbols tracking not applicable for this kind of code. - debian/watch is present and looks ok - Upstream update history is good, but short - Debian/Ubuntu update history is good, but short - the current release is packaged (2.3 is only RC2 atm) - promoting this does not seem to cause issues for MOTUs that so far maintained the package - no massive Lintian warnings - debian/rules is rather clean - It is not on the lto-disabled list Problems: None [Upstream red flags] OK: - no Errors/warnings during the build - no incautious use of malloc/sprintf (python) - no use of user nobody - no use of setuid - no important open bugs (crashers, etc) in Debian or Ubuntu - no dependency on webkit, qtwebkit, seed or libgoa-* - not part of the UI for extra checks - no translation present, but none needed for this case Problems: None