SCSI: Fix Persistent Reservation (PR) NVMe->SCSI conversion of HostID to Transport ID - #4383
Conversation
… to Transport ID Just copying it, as was done before, is wrong since HostID is 8 or 16 bytes (random-ish) and TransportID is 24 bytes (and has structure). This change follows the [T10 SPC-3 spec](https://www.t10.org/ftp/t10/document.02/02-246r1.pdf) for the format of the TransportID retaining backward compatibility for NVMe HostIDs that start with `06 00 00 00`. This last part can happen when the NVMe HostID itself is incorrectly derived from an underlying SCSI TransportID by just copying bytes.
There was a problem hiding this comment.
🟡 Changes recommended
The new SAS TransportID construction currently risks populating reserved bytes for 16-byte HostIDs, which can produce a non-conformant TransportID and should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates SCSI Persistent Reservation (PR) “REPORT FULL STATUS” handling to correctly convert NVMe HostID values into a structured SCSI TransportID (SAS protocol identifier), rather than copying HostID bytes directly.
Changes:
- Constructs a fixed-size SAS TransportID buffer and emits it in the FULL STATUS descriptor.
- Adds a compatibility heuristic for NVMe HostIDs that appear to already contain the SAS TransportID prefix.
- Updates the descriptor’s
additional_descriptor_lengthto match the emitted TransportID size.
File summaries
| File | Description |
|---|---|
| vm/devices/storage/scsidisk/src/reservation.rs | Builds a SAS TransportID for REPORT FULL STATUS and adjusts descriptor length/output accordingly. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, internally consistent (length fields match emitted bytes), and the updated tests align with the corrected on-wire format.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
ab10638
into
microsoft:main
… to Transport ID (#4383) (#4407) Clean cherry pick of PR #4383 Just copying it, as was done before, is wrong since HostID is 8 or 16 bytes (random-ish) and TransportID is 24 bytes (and has structure). This change follows the [T10 SPC-3 spec](https://www.t10.org/ftp/t10/document.02/02-246r1.pdf) for the format of the TransportID retaining backward compatibility for NVMe HostIDs that start with `06 00 00 00`. This last part can happen when the NVMe HostID itself is incorrectly derived from an underlying SCSI TransportID by just copying bytes. Before: ``` # sg_persist --in --read-full-status /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk PR generation=0x1 Key=0x1234 All target ports bit set not reservation holder Transport Id short or not multiple of 4 [length=1024]: SAS address: 0xbd591cd38dc846b6 # sg_raw -r 8192 /dev/sda 5e 03 00 00 00 00 00 20 00 00 | hexdump -C SCSI Status: Good Received 48 bytes of data: 00 00 00 00 18 00 00 00 28 ff 00 7c 80 09 2f 6d 08 .......(..|../m. 10 00 00 00 00 02 00 00 00 00 00 00 00 00 00 00 10 ................ 20 06 00 00 00 bd 59 1c d3 8d c8 46 b6 99 68 1d 91 .....Y....F..h.. ``` After: ``` alpine:~# sg_persist --out --register --param-rk=0 --param-sark=0x1234 /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk alpine:~# sg_persist --in --read-keys /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk PR generation=0x1, 1 registered reservation key follows: 0x1234 alpine:~# sg_persist --in --read-full-status /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk PR generation=0x1 Key=0x1234 All target ports bit set not reservation holder Transport Id of initiator: SAS address: 0x1020304050607 alpine:~# sg_raw -r 8192 /dev/sda 5e 03 00 00 00 00 00 20 00 00 SCSI Status: Good Received 56 bytes of data: 00 00 00 00 01 00 00 00 30 00 00 00 00 00 00 12 34 .......0.......4 10 00 00 00 00 02 00 00 00 00 00 00 00 00 00 00 18 ................ 20 06 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b ................ 30 0c 0d 0e 0f 00 00 00 00 ........ ```
|
Backported to release/1.8.2607 in #4407 |
See #4383 for full context. This PR moves the SAS workaround (Host ID starting with 6 0 0 0) from the SCSI code to NVMe.
… to Transport ID (microsoft#4383) Just copying it, as was done before, is wrong since HostID is 8 or 16 bytes (random-ish) and TransportID is 24 bytes (and has structure). This change follows the [T10 SPC-3 spec](https://www.t10.org/ftp/t10/document.02/02-246r1.pdf) for the format of the TransportID retaining backward compatibility for NVMe HostIDs that start with `06 00 00 00`. This last part can happen when the NVMe HostID itself is incorrectly derived from an underlying SCSI TransportID by just copying bytes. Before: ``` # sg_persist --in --read-full-status /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk PR generation=0x1 Key=0x1234 All target ports bit set not reservation holder Transport Id short or not multiple of 4 [length=1024]: SAS address: 0xbd591cd38dc846b6 # sg_raw -r 8192 /dev/sda 5e 03 00 00 00 00 00 20 00 00 | hexdump -C SCSI Status: Good Received 48 bytes of data: 00 00 00 00 18 00 00 00 28 ff 00 7c 80 09 2f 6d 08 .......(..|../m. 10 00 00 00 00 02 00 00 00 00 00 00 00 00 00 00 10 ................ 20 06 00 00 00 bd 59 1c d3 8d c8 46 b6 99 68 1d 91 .....Y....F..h.. ``` After: ``` alpine:~# sg_persist --out --register --param-rk=0 --param-sark=0x1234 /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk alpine:~# sg_persist --in --read-keys /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk PR generation=0x1, 1 registered reservation key follows: 0x1234 alpine:~# sg_persist --in --read-full-status /dev/sda OpenVMM Disk 1.0 Peripheral device type: disk PR generation=0x1 Key=0x1234 All target ports bit set not reservation holder Transport Id of initiator: SAS address: 0x1020304050607 alpine:~# sg_raw -r 8192 /dev/sda 5e 03 00 00 00 00 00 20 00 00 SCSI Status: Good Received 56 bytes of data: 00 00 00 00 01 00 00 00 30 00 00 00 00 00 00 12 34 .......0.......4 10 00 00 00 00 02 00 00 00 00 00 00 00 00 00 00 18 ................ 20 06 00 00 00 00 01 02 03 04 05 06 07 08 09 0a 0b ................ 30 0c 0d 0e 0f 00 00 00 00 ........ ```
Just copying it, as was done before, is wrong since HostID is 8 or 16 bytes (random-ish) and TransportID is 24 bytes (and has structure).
This change follows the T10 SPC-3 spec for the format of the TransportID retaining backward compatibility for NVMe HostIDs that start with
06 00 00 00. This last part can happen when the NVMe HostID itself is incorrectly derived from an underlying SCSI TransportID by just copying bytes.Before:
After: