Skip to content

SCSI: Fix Persistent Reservation (PR) NVMe->SCSI conversion of HostID to Transport ID - #4383

Merged
Alex Landau (alandau) merged 2 commits into
microsoft:mainfrom
alandau:pr-nvme-to-scsi
Sep 9, 2026
Merged

Alex Landau (alandau) merged 2 commits into
microsoft:mainfrom
alandau:pr-nvme-to-scsi

Conversation

@alandau

@alandau Alex Landau (alandau) commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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:

# 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                             ........

… 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.
Copilot AI lite review requested due to automatic review settings September 3, 2026 20:29
@alandau Alex Landau (alandau) changed the title SCSI: Fix Persistent Reservation (PR) NVMe->SCSI conversion of HostID… SCSI: Fix Persistent Reservation (PR) NVMe->SCSI conversion of HostID to Transport ID Sep 3, 2026

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 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_length to 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.

Comment thread vm/devices/storage/scsidisk/src/reservation.rs
Comment thread vm/devices/storage/scsidisk/src/reservation.rs
Copilot AI review requested due to automatic review settings September 3, 2026 22:25
@alandau
Alex Landau (alandau) marked this pull request as ready for review September 3, 2026 22:26

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

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

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

@mattkur Matt LaFayette (Kurjanowicz) (mattkur) added the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Sep 8, 2026
@alandau
Alex Landau (alandau) merged commit ab10638 into microsoft:main Sep 9, 2026
237 of 240 checks passed
Alex Landau (alandau) added a commit that referenced this pull request Sep 9, 2026
… 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                             ........
```
@benhillis

Copy link
Copy Markdown
Member

Backported to release/1.8.2607 in #4407

@benhillis Ben Hillis (benhillis) added backported_1.8.2607 PR that has been backported to release/1.8.2607 and removed backport_1.8.2607 Change should be backported to the release/1.8.2607 branch labels Sep 10, 2026
Alex Landau (alandau) added a commit that referenced this pull request Sep 16, 2026
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.
Pedro Henrique Penna (ppenna) pushed a commit to nanvix/openvmm that referenced this pull request Sep 17, 2026
… 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                             ........
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported_1.8.2607 PR that has been backported to release/1.8.2607

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants