Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions tests/nvme_attach_detach_ns_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ class TestNVMeAttachDetachNSCmd(TestNVMe):
def setUp(self):
""" Pre Section for TestNVMeAttachDetachNSCmd """
super().setUp()

if not self.ns_mgmt_supported:
self.skipTest("Namespace Management / Attach not supported by controller")

self.dps = 0
self.flbas = 0
(ds, ms) = self.get_lba_format_size()
Expand Down
4 changes: 4 additions & 0 deletions tests/nvme_create_max_ns_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ class TestNVMeCreateMaxNS(TestNVMe):
def setUp(self):
""" Pre Section for TestNVMeAttachDetachNSCmd """
super().setUp()

if not self.ns_mgmt_supported:
self.skipTest("Namespace Management / Attach not supported by controller")

self.dps = 0
self.flbas = 0
(ds, ms) = self.get_lba_format_size()
Expand Down
4 changes: 4 additions & 0 deletions tests/nvme_format_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ class TestNVMeFormatCmd(TestNVMe):
def setUp(self):
""" Pre Section for TestNVMeFormatCmd """
super().setUp()

if not self.ns_mgmt_supported:
self.skipTest("Namespace Management / Attach not supported by controller")

self.dps = 0
self.flbas = 0
# Assuming run_ns_io with 4KiB * 10 writes.
Expand Down
38 changes: 35 additions & 3 deletions tests/nvme_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ def to_decimal(value):
- Returns:
- Decimal integer
"""
return int(str(value), 0)
val = 0
try:
val = int(str(value), 0)
except (TypeError, ValueError):
raise ValueError(f"Invalid value: {value!r}")
return val


class TestNVMe(unittest.TestCase):
Expand Down Expand Up @@ -77,14 +82,17 @@ def setUp(self):
self.load_config()
if self.do_validate_pci_device:
self.validate_pci_device()
self.create_and_attach_default_ns()
self.ns_mgmt_supported = self.get_ns_mgmt_support()
if self.ns_mgmt_supported:
self.create_and_attach_default_ns()
print(f"\nsetup: ctrl: {self.ctrl}, ns1: {self.ns1}, default_nsid: {self.default_nsid}, flbas: {self.flbas}\n")

def tearDown(self):
""" Post Section for TestNVMe. """
if self.clear_log_dir is True:
shutil.rmtree(self.log_dir, ignore_errors=True)
self.create_and_attach_default_ns()
if self.ns_mgmt_supported:
self.create_and_attach_default_ns()
print(f"\nteardown: ctrl: {self.ctrl}, ns1: {self.ns1}, default_nsid: {self.default_nsid}, flbas: {self.flbas}\n")

@classmethod
Expand Down Expand Up @@ -209,6 +217,30 @@ def get_ctrl_id(self):
"ERROR : nvme list-ctrl could not find ctrl")
return str(json_output['ctrl_list'][0]['ctrl_id'])

def get_ns_mgmt_support(self):
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The new get_ns_mgmt_support() helper is missing a docstring, while surrounding helpers in this file consistently document purpose/args/returns. Please add a brief docstring (and ideally note which OACS bit(s) are being checked) to keep the file’s documentation style consistent.

Suggested change
def get_ns_mgmt_support(self):
def get_ns_mgmt_support(self):
""" Parse controller OACS field to determine namespace management support.
- Args:
- None
- Returns:
- Dictionary with:
- "raw": integer OACS value.
- "ns_mgmt": True if OACS bit 3 (Namespace Management) is set.
- "ns_attach": True if OACS bit 4 (Namespace Attachment) is set.
"""

Copilot uses AI. Check for mistakes.
"""
Determine whether Namespace Management and Namespace Attachment
operations are supported by the controller.

This method reads the Optional Admin Command Support (OACS) field
from the Identify Controller data structure and evaluates specific
bits that indicate support for:
- Namespace Management (bit 3)
- Namespace Attachment (bit 4)

Both features must be supported for this function to return True.

Returns:
bool: True if both Namespace Management and Namespace Attachment
are supported, False otherwise.
"""
oacs = to_decimal(self.get_id_ctrl_field_value("oacs"))

ns_mgmt_supported = bool(oacs & (1 << 3))
ns_attach_supported = bool(oacs & (1 << 4))
Comment on lines +239 to +240
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

get_ns_mgmt_support() is interpreting OACS bit 4 as “namespace attach”, but in this repo OACS[4] is used for Device Self-test and OACS[3] is “NS Management and Attachment” (combined). This will incorrectly report ns_mgmt_supported as false on controllers that support NS mgmt/attach but don’t support self-test, causing namespace setup/teardown to be skipped. Please base the NS mgmt+attach detection on OACS bit 3 only (and adjust the returned fields accordingly).

Suggested change
ns_mgmt_supported = bool(oacs & (1 << 3))
ns_attach_supported = bool(oacs & (1 << 4))
# OACS bit 3 indicates combined Namespace Management and Attachment support.
ns_mgmt_supported = bool(oacs & (1 << 3))
ns_attach_supported = ns_mgmt_supported

Copilot uses AI. Check for mistakes.

return ns_mgmt_supported and ns_attach_supported

def get_nsid_list(self):
""" Wrapper for extracting the namespace list.
- Args:
Expand Down
Loading