)]}'
{"specs/2026.2/approved/generic-nvme-driver-with-secure-cleanup.rst":[{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"ba35d5ae_e40f0c60","line":1,"updated":"2026-04-20 10:29:10.000000000","message":"The commit message subject line \u0027Add spec for generic NVMe driver with cleanup\u0027 undersells the \u0027secure cleanup\u0027 aspect which is the key differentiator of this spec.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: A more descriptive subject line helps reviewers and git-log readers quickly understand the change scope.\n\n**Recommendation**:\nConsider updating the commit subject to include \u0027secure cleanup\u0027 explicitly, such as \u0027Add NVMe driver spec with secure cleanup\u0027 (41 chars).","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"9da00969_ab449a34","line":100,"updated":"2026-04-20 10:29:10.000000000","message":"Inconsistent capitalization of NVMe/NVME throughout the document. The spec uses \u0027NVME\u0027 (all caps) in many places where \u0027NVMe\u0027 (mixed case) is the correct industry-standard capitalization.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Inconsistent terminology makes the spec harder to read and may cause confusion about whether NVME and NVMe refer to the same thing. It also looks unprofessional in published documentation.\n\n**Suggestion**:\nStandardize on \u0027NVMe\u0027 throughout the document. Specific instances to fix include line 100 \u0027NVME device\u0027, line 115 \u0027NVME device\u0027, line 119-120 \u0027NVME device\u0027, line 139 heading \u0027NVME Controller\u0027, line 162 \u0027NVME cleanup\u0027, line 190 heading \u0027NVME Cleanup\u0027, line 193 \u0027NVME devices\u0027. The correct form \u0027NVMe\u0027 is already used in other places, so this is an inconsistency rather than a deliberate choice.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"3ecc2c1a_d8c4bbd1","line":128,"updated":"2026-04-20 10:29:10.000000000","message":"The proposed Placement trait naming CUSTOM_NVME_VENDOR_NAME_VENDOR_ID_PRODUCT_ID could become very long. Consider whether a shorter convention would be more practical.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Shorter trait names are easier to use in flavors, easier to read in placement output, and less prone to typos. The vendor name component is also ambiguous.\n\n**Recommendation**:\nDefine the exact trait format with an example, e.g., CUSTOM_NVME_1BD4_1001 using numeric vendor_id and product_id. Clarify what \u0027VENDOR_NAME\u0027 means and whether it is a human-readable string or an ID.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"afaec45a_3211c1a3","line":145,"updated":"2026-04-20 10:29:10.000000000","message":"The spec does not address the impact on Cyborg\u0027s scheduling when a device is in cleanup-in-progress state. Placement inventory should be reduced or removed during cleanup to prevent double allocation.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Explicitly stating the Placement inventory behavior during cleanup prevents accidental double-allocation of a device that is being sanitized.\n\n**Recommendation**:\nAdd a sentence to the \u0027Cleanup on unbind\u0027 section stating that the device\u0027s Placement inventory must be removed or set to zero when cleanup begins, and restored only after successful cleanup completion and verification.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"01680a33_2c896839","line":145,"updated":"2026-04-20 10:29:10.000000000","message":"The spec does not describe what happens to in-flight I/O when cleanup starts. NVMe sanitize operations will fail if the namespace has active controllers or open references.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Documenting the prerequisite state for cleanup (device fully detached, no active I/O) prevents implementation issues where sanitize fails due to open references.\n\n**Recommendation**:\nAdd a note that cleanup must only begin after the device is fully detached from the instance (unbind complete, no active PCI assignment) and all NVMe controller references are released.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"bce4dd0c_e5681e47","line":151,"updated":"2026-04-20 10:29:10.000000000","message":"The spec references \u0027cyborg/accelerator/drivers/nvme/utils.py\u0027 as an existing file to follow for the cleanup pattern, but Cyborg\u0027s current NVMe-related code is vendor-specific (Inspur). The relationship between the existing Inspur NVMe code and this new generic driver deserves clarification.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Reviewers and implementers need to understand whether the existing Inspur NVMe utility code will be refactored into shared infrastructure or whether the generic driver will have entirely independent utility functions.\n\n**Suggestion**:\nClarify the relationship with the existing Inspur driver code. Will the generic driver share utility code with the Inspur driver? Will the Inspur driver be refactored to inherit from NVMeDriver? The \u0027Upgrade impact\u0027 section mentions coexistence but does not address code sharing.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"492bd201_9cc70188","line":154,"updated":"2026-04-20 10:29:10.000000000","message":"The code example in the \u0027Cleanup on unbind\u0027 section has incorrect Python indentation: the function body is indented under the decorator without a proper function definition line, and the docstring is misaligned.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Incorrect code examples in specs can mislead implementers and cause bugs in the initial implementation.\n\n**Priority**: Before merge\n**Why This Matters**: The code block shows the decorator and def on the same line, with the body incorrectly indented relative to the function definition. This would not parse as valid Python and sets a confusing precedent for the implementation.\n\n**Recommendation**:\nFix the code example to use proper Python indentation: the @decorator on its own line, def on the next line, docstring indented 4 spaces under def, and function body indented 8 spaces.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"12053214_f64a0ade","line":169,"updated":"2026-04-20 10:29:10.000000000","message":"The spec uses \u0027sudo nvme ...\u0027 commands in the prose but the code example correctly uses privsep. The prose descriptions should avoid mentioning \u0027sudo\u0027 and instead refer to privsep or elevated privileges consistently.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Consistent terminology avoids confusion about whether operators need to configure sudoers rules or if privsep handles privilege escalation automatically.\n\n**Recommendation**:\nReplace \u0027sudo nvme ...\u0027 references in the prose with \u0027nvme ...\u0027 and note that all commands run under the privsep context described in the code example.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"c3156aa8_01d64a5c","line":176,"updated":"2026-04-20 10:29:10.000000000","message":"The sanitize command example uses \u0027-a 0x04\u0027 (Block Erase) but the spec does not discuss how to choose between sanitize modes (Crypto Erase 0x02, Block Erase 0x04, Overwrite 0x01) or what happens when the device does not support the chosen mode.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Different NVMe devices support different sanitize capabilities. Hardcoding a single mode without fallback logic may cause cleanup failures on devices that only support Crypto Erase or Overwrite.\n\n**Suggestion**:\nDescribe the capability detection and mode selection logic: read id-ctrl output for supported sanitize capabilities, select the most appropriate mode with a fallback chain (e.g., Crypto Erase first if supported, then Block Erase, then format), and handle the case where no sanitize is supported.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"4d940182_402b3b3b","line":188,"updated":"2026-04-20 10:29:10.000000000","message":"The spec mentions Nova blocking instance deletion until NVMe cleanup finishes, but does not describe the coordination mechanism between Cyborg and Nova for this blocking behavior.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Without specifying the Nova-Cyborg coordination mechanism, implementers may design incompatible solutions. The current Nova-Cyborg interaction model uses asynchronous device profile handling, and blocking deletion is a significant design change.\n\n**Suggestion**:\nAdd a subsection or paragraph explaining how Nova will wait for cleanup: whether through a synchronous RPC call, a callback mechanism, a status polling loop, or modification to the existing Cyborg-Nova interaction in the accelerators API. Address whether this requires changes to Nova itself.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"bdcf749f_4c438c4d","line":203,"updated":"2026-04-20 10:29:10.000000000","message":"The spec mentions concurrent cleanup prevention but does not specify the locking mechanism (process-level lock, file lock, database row lock, etc.).\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Specifying the locking approach helps reviewers assess correctness and helps implementers avoid race conditions.\n\n**Recommendation**:\nState whether locking will use an in-process lock (e.g., threading.Lock), a file-based lock (e.g., fasteners), or a database-level mechanism. Recommend a concrete approach.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"021517ba_42473a3f","line":205,"updated":"2026-04-20 10:29:10.000000000","message":"The \u0027NVME Cleanup failed with timeout\u0027 section describes moving device state and blocking instance deletion, but the signaling mechanism back from cleanup completion to Cyborg and then to Nova is vague (\u0027we need to find a way to signal Cyborg\u0027).\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: The cleanup completion signaling is a core part of the architecture. Leaving it unspecified risks inconsistent or incomplete implementation.\n\n**Suggestion**:\nReplace \u0027we need to find a way to signal\u0027 with a concrete proposal: for example, a periodic status check polling nvme sanitize-log, a privileged helper that writes completion to a status file, or an event-driven callback. Similarly, specify how Cyborg notifies Nova to proceed with deletion.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"0671f40c_c9636875","line":230,"updated":"2026-04-20 10:29:10.000000000","message":"The spec states \u0027Data model impact: None\u0027 but the cleanup timeout configuration, device state management for failed cleanups, and the new cyborg-nvme-cleanup CLI all imply new data model or state tracking requirements that should be explicitly described.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: If data model changes are needed but not planned upfront, the implementation phase may require a spec amendment or could introduce schema migration issues during deployment.\n\n**Priority**: Before merge\n**Why This Matters**: Device state tracking for cleanup status (clean, dirty, cleanup-in-progress, cleanup-failed) requires persistent state. The spec mentions moving devices to different states but does not describe how this state is stored or what schema changes are required.\n\n**Recommendation**:\nReplace \u0027None\u0027 with a description of the required data model changes: at minimum, a device state field for tracking cleanup status, and any new database tables or columns needed for the cleanup timeout configuration and failed cleanup recovery.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"9815dedc559d417faa81e0c0661076d5","url":"https://zuul.teim.app/t/main/buildset/9815dedc559d417faa81e0c0661076d5","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"148af4c00e06f667014f2b9128abb21db1368c79","patch_set":2,"id":"12852a75_9437dac0","line":235,"updated":"2026-04-20 10:29:10.000000000","message":"The spec says \u0027REST API impact: None\u0027 but introduces a new cyborg-nvme-cleanup CLI command and implies new device state transitions that may require API exposure for operators to query cleanup status.\n\n**Severity**: HIGH | **Confidence**: 0.8\n\n**Risk**: If the CLI tool or any new API endpoints are needed but not specified, the implementation may introduce undocumented or inconsistent API surfaces.\n\n**Priority**: Before merge\n**Why This Matters**: The cyborg-nvme-cleanup CLI tool and any API needed to query device cleanup status or trigger manual cleanup should be documented in the REST API impact section, even if only to say the tool is a standalone CLI that does not change the REST API.\n\n**Recommendation**:\nClarify whether the cyborg-nvme-cleanup CLI interacts with the Cyborg REST API or directly with the database. If it uses existing APIs, state that. If it requires new endpoints, describe them. At minimum, mention the CLI tool in this section.","commit_id":"7be76b0e65d25f340630e631037ff2b91e420bd0"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"53a66549_64ab2465","line":1,"updated":"2026-04-29 15:10:29.000000000","message":"The spec does not include the APIImpact git trailer in the commit message, which is required per the cyborg-specs template when the spec proposes REST API changes.\n\n**Severity**: WARNING | **Confidence**: 0.9\n\n**Impact**: Without the APIImpact tag, the change may not surface in API-focused review queries on Gerrit, reducing visibility for API reviewers\n\n**Suggestion**:\nAdd the APIImpact footer to the commit message. The template explicitly states: \u0027If your specification proposes any changes to the Cyborg REST API... then you should add the APIImpact flag to the commit message.\u0027","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"9caec180_93985717","line":8,"updated":"2026-04-29 15:10:29.000000000","message":"The commit subject line \u0027Add generic NVMe driver spec with secure cleanup\u0027 is 49 characters -- just under the 50-char limit. Consider shortening for clarity.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Provides more breathing room under the 50-char commit subject limit and improves readability in git log views\n\n**Recommendation**:\nConsider \u0027Add generic NVMe driver spec\u0027 (30 chars) as the commit subject, with the cleanup detail in the body. The full context is already in the spec title and body.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"deb7112b_dbe1eda8","line":11,"updated":"2026-04-29 15:10:29.000000000","message":"The spec does not include an \u0027Implements:\u0027 blueprint reference in the commit message footer, even though a Launchpad blueprint URL is provided in the document body.\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: Adding the Implements trailer links the spec to its Launchpad blueprint for tracking purposes\n\n**Recommendation**:\nAdd \u0027Implements: blueprint generic-nvme-driver-with-secure-cleanup\u0027 to the commit message footer, matching the standard OpenStack convention for spec commits.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"e2c39084_4367702e","line":47,"updated":"2026-04-29 15:10:29.000000000","message":"The spec states resize, cold migration, and live migration are not supported but does not explain what happens if a user attempts these operations on an instance with an NVMe device attached.\n\n**Severity**: SUGGESTION | **Confidence**: 0.8\n\n**Benefit**: Operators and users need to know whether unsupported operations are blocked, warned, or silently ignored\n\n**Recommendation**:\nAdd a statement clarifying the expected behavior: e.g., Nova\u0027s scheduler should reject resize/migrate requests for instances with NVMe ARQs, or Cyborg should block the operation at the ARQ level.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"a80a309b_13eec0b7","line":198,"updated":"2026-04-29 15:10:29.000000000","message":"The spec does not address how the driver handles NVMe devices with multiple namespaces. A single NVMe controller may expose multiple namespaces, each of which could be independently allocated.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Clarifying namespace handling prevents ambiguity during implementation and ensures cleanup covers all allocated namespaces\n\n**Recommendation**:\nState explicitly whether the driver manages devices at the controller level (one RP per PCI device, all namespaces bound together) or namespace level (separate RPs per namespace). The current design implies controller-level, but this should be explicit.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"22e1d655_1632f9de","line":230,"updated":"2026-04-29 15:10:29.000000000","message":"The Device Profile Example uses CUSTOM_NVME_INTEL_8086_0001 as a trait name, which hard-codes the vendor name (INTEL). The trait naming convention uses CUSTOM_NVME_\u003cVENDOR\u003e_\u003cVENDOR_ID\u003e_\u003cPRODUCT_ID\u003e, which assumes the vendor string is known and stable.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Clarifying how the VENDOR component is derived prevents inconsistent trait naming across deployments\n\n**Recommendation**:\nSpecify the source of the \u003cVENDOR\u003e string: is it derived from the PCI vendor database, sysfs, or the device_spec configuration? Consider simplifying the trait to CUSTOM_NVME_\u003cVENDOR_ID\u003e_\u003cPRODUCT_ID\u003e to avoid vendor name lookup ambiguity.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"778e0859_f80f7216","line":250,"updated":"2026-04-29 15:10:29.000000000","message":"The cleanup flow holds the ARQ and its Placement allocation until cleanup completes, but the spec does not address what happens if the conductor or agent crashes after step 1 (reserved\u003dtotal) but before dispatching cleanup. The ARQ would remain allocated with no cleanup triggered.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: A conductor crash between reservation and dispatch could leave devices permanently reserved in Placement with no cleanup in progress, requiring manual operator intervention with no documented recovery path\n\n**Suggestion**:\nAdd a conductor startup recovery step similar to the agent crash recovery section: on conductor restart, scan for devices in cleaning state where no cleanup RPC is in-flight, and either re-dispatch cleanup or transition to maintaining with cleanup_failed\u003dTrue.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"6f031dcc_259b0458","line":269,"updated":"2026-04-29 15:10:29.000000000","message":"The spec references nvme format on /dev/nvme0n1 (a namespace) but cleanup receives a PCI address resolving to /dev/nvme0 (a controller). The relationship between controller and namespace for format operations is not specified.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: nvme format operates on namespaces (nvme0n1), not controllers (nvme0). If the device has multiple namespaces, the spec does not clarify whether all namespaces must be formatted or just the first.\n\n**Suggestion**:\nClarify whether the driver targets a single namespace (nvme0n1), all namespaces, or operates at the controller level. For sanitize, this is a controller-level operation so no ambiguity exists, but format is namespace-level.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"522d67fc_ec73ae8e","line":277,"updated":"2026-04-29 15:10:29.000000000","message":"The spec says cleanup is triggered by the unbind RPC, but the existing Cyborg flow triggers unbind when Nova calls DELETE on the ARQ. The spec should clarify whether cleanup replaces unbind or runs after unbind, and whether the existing unbind logic still detaches the PCI device.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If cleanup runs before unbind completes the PCI detach, the nvme-cli commands may fail because the device is still attached to the instance. If cleanup runs after unbind, the PCI address may no longer be valid.\n\n**Suggestion**:\nAdd an explicit sequencing statement: e.g., cleanup runs after PCI detach from the instance but before the device is returned to the pool. Document the expected PCI device state at cleanup time.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"b6be6ca9_eca0904e","line":291,"updated":"2026-04-29 15:10:29.000000000","message":"The spec mentions nvme sanitize-log polling but does not specify the polling interval or strategy (linear backoff, exponential backoff, fixed interval).\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: Specifying the polling strategy prevents implementers from choosing an approach that either hammers the device or introduces unnecessary delay\n\n**Recommendation**:\nAdd a recommended polling interval (e.g., poll every 10 seconds, or use an interval configurable via [nvme] cleanup_poll_interval) and note that sanitize-log reads are lightweight NVMe admin commands.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"d4eb5ff7_7a8f74a1","line":363,"updated":"2026-04-29 15:10:29.000000000","message":"The Alternatives section has only two rejected alternatives. Consider adding a third: using Nova\u0027s Cinder volume attach with NVMe-oF to leverage existing volume cleanup mechanisms.\n\n**Severity**: SUGGESTION | **Confidence**: 0.7\n\n**Benefit**: A more thorough alternatives analysis strengthens the spec and preempts reviewer questions about why existing Cinder-based cleanup paths were not chosen\n\n**Recommendation**:\nAdd a brief third alternative noting that routing local NVMe through Cinder (as NVMe-oF volumes) would provide cleanup via Cinder\u0027s volume management, but this requires network infrastructure and does not address local-device use cases.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"e85774dd_4ba7f2d0","line":376,"updated":"2026-04-29 15:10:29.000000000","message":"The data model impact section mentions an Alembic migration for the cleaning status and cleanup_failed column but does not specify the default value for cleanup_failed on existing rows, the column nullability, or whether the migration is expand/contract or single-step.\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: Ambiguity in migration strategy could cause issues during rolling upgrades; existing device rows need a deterministic default for the new boolean column\n\n**Suggestion**:\nExplicitly state: (1) cleanup_failed is NOT NULL with server_default\u003dFalse, (2) whether the status constraint update uses a check constraint or enum expansion, and (3) the migration is a single expand step (no contract needed for additive columns).","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"},{"robot_id":"zuul","robot_run_id":"cf5d3cb27d624267b5258d0b8e10277c","url":"https://zuul.teim.app/t/main/buildset/cf5d3cb27d624267b5258d0b8e10277c","author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"93fd8a6f44e337fe3247ef931985dfe553d057df","patch_set":3,"id":"e1212a7e_fd1001c9","line":388,"updated":"2026-04-29 15:10:29.000000000","message":"REST API impact section lacks the detail required by the template and OpenStack spec standards. Missing: HTTP method types, normal/error response codes, URL parameters, JSON schema definitions for request/response bodies, and example API samples with actual request/response payloads.\n\n**Severity**: HIGH | **Confidence**: 0.9\n\n**Risk**: Without explicit API contracts, implementers may produce incompatible implementations and reviewers cannot validate the microversion design\n\n**Priority**: Before merge\n**Why This Matters**: The template explicitly states API changes are held to higher scrutiny and require method specifications, response codes, URL parameters, and JSON schema. The current section describes what changes at a high level but does not provide any of the required API specification details.\n\n**Recommendation**:\nAdd concrete API examples showing: (1) GET /v2/devices response with the new cleaning status and cleanup_failed field, (2) the microversion header value, (3) JSON schema for the changed response fields, and (4) expected error codes. Follow the template\u0027s REST API impact section format with Method type, Normal response code, Error response codes, URL, and JSON schema.","commit_id":"5186392408c64620be2d1a188ce633cba77ce24c"}]}