Skip to content

[AI Generated] BugFix: Exclude namespace-free remote NVMe controllers - #4842

Open
LiliDeng wants to merge 2 commits into
mainfrom
bugfix/nvme-remote-controllers_301026_185407
Open

LiliDeng wants to merge 2 commits into
mainfrom
bugfix/nvme-remote-controllers_301026_185407

Conversation

@LiliDeng

Copy link
Copy Markdown
Collaborator

Summary

Exclude remote NVMe controllers that have no attached namespaces from local NVMe discovery.

nvme list only reports controllers with namespaces. Consequently, namespace-based model detection can miss an additional MSFT NVMe Accelerator controller and count it as a local device. This makes verify_nvme_basic fail its controller-to-namespace count check.

  • On Linux, read the controller model from sysfs when namespace-based identification has not classified the controller as remote.
  • Preserve local controller discovery, namespace filtering, non-Linux behavior, and the existing count assertions.
  • Update the basic test description to reflect remote-device filtering and capability-based namespace counts.

Validation

  • Reproduced the controller/namespace count assertion failure before the fix.
  • Ran the complete verify_nvme_basic twice after the fix on the same VM; both runs passed.
  • JUnit confirms one test, zero failures, and zero errors.
  • Black, flake8, and pylint passed.
  • Changed-file mypy passed with --ignore-missing-imports --follow-imports=silent. The default recursive check encountered missing dependency stubs and existing errors in imported modules.

Key Test Cases:
verify_nvme_basic

Impacted LISA Features:
Nvme

Tested Azure Marketplace Images:
canonical 0001-com-ubuntu-server-jammy 22_04-lts-arm64 22.04.202610220

Identify remote NVMe controllers by their sysfs model when namespace-based discovery cannot classify them. Update the basic test description to reflect local disk filtering and capability-based counts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 11:31
@LiliDeng

Copy link
Copy Markdown
Collaborator Author

Validation Results

Image Result
canonical 0001-com-ubuntu-server-jammy 22_04-lts-arm64 22.04.202610220 PASSED (two runs)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the Linux NVMe discovery update that filters remote controllers without attached namespaces.

No open issues 🎉

What changed in this PR

Fixes local NVMe discovery on Linux by excluding remote controllers that have no attached namespaces and therefore do not appear in nvme list.

Changes:

  • Adds a sysfs model lookup to identify remote controllers missed by namespace-based detection.
  • Updates the basic test description to reflect remote-device filtering and capability-based namespace counts; assertions remain unchanged.

Suggested validation:

Key Test Cases:
verify_nvme_basic

Impacted LISA Features:
Nvme

Tested Azure Marketplace Images:

  • canonical 0001-com-ubuntu-server-jammy 22_04-lts-arm64 latest
File Description
lisa/​microsoft/​testsuites/​nvme/​nvme.py Updates test metadata wording to describe existing validation accurately.
lisa/​features/​nvme.py Adds Linux sysfs model detection for namespace-free remote controllers.

Review effort: Balanced

@LiliDeng

Copy link
Copy Markdown
Collaborator Author

AI Test Case Selection

Selected 9 test case(s): verify_nvme_basic,verify_nvme_max_disk,verify_nvme_function,verify_nvme_function_unpartitioned,verify_nvme_fstrim,verify_nvme_blkdiscard,verify_nvme_manage_ns,verify_nvme_rescind,verify_nvme_sriov_rescind

Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest

Result: Succeeded

View full logs in Azure DevOps

@johnsongeorge-w

Copy link
Copy Markdown
Collaborator

Run tests with Dpv7 and Dv6 sizes with AzLinux and Canonical images.

@LiliDeng

Copy link
Copy Markdown
Collaborator Author

Run tests with Dpv7 and Dv6 sizes with AzLinux and Canonical images.

verify_nvme_basic
verify_nvme_manage_ns
microsoftazurelinux azurelinux-4 4-arm64 4.0.2026100801 | Standard_D2pds_v7 | PASSED
canonical ubuntu-24_04-lts server-arm64 latest | Standard_D2pds_v7 | PASSED
canonical ubuntu-24_04-lts server latest | Standard_D2ds_v6 | PASSED

Read all controller models in one remote command only when Linux discovery contains controller paths. Avoid per-controller SSH commands and skip sysfs reads for namespace-only lists. Preserve existing remote disk filtering and legacy fallback behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 2, 2026 02:38
@LiliDeng

LiliDeng commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed the bulk sysfs model-read feedback in 621e598. Linux controller discovery now reads all controller models in one remote command per invocation; namespace-only lists do not trigger a sysfs read. Nine isolated simulated checks verify command counts, filtering, platform gating, legacy fallback and error propagation. Black, flake8, pylint and focused mypy passed. No new live Azure validation was run for this follow-up.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The NVMe discovery assertion omits the failed controller path, command output, and model-file permission guidance, leaving missing or unreadable model files with an unhelpful generic failure.

1 open issue

Review effort: Balanced

Comment thread lisa/features/nvme.py Resolved
@LiliDeng

LiliDeng commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

AI Test Case Selection

Selected 9 test case(s): verify_nvme_basic,verify_nvme_max_disk,verify_nvme_function,verify_nvme_function_unpartitioned,verify_nvme_fstrim,verify_nvme_blkdiscard,verify_nvme_manage_ns,verify_nvme_rescind,verify_nvme_sriov_rescind

Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest

Result: Succeeded

View full logs in Azure DevOps

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants