v0.13.20 — sync blocking SG_IO + cross-platform parity strip

- scsi/linux.rs: full rewrite from async write/poll/read+1.5s timeout+
  close-on-timeout to one synchronous ioctl(fd, SG_IO, &hdr). Kernel
  honors hdr.timeout and runs its own ABORT/RESET escalation. Errors
  check host_status and driver_status (both 0xFF-synthesised) plus
  status. Sense-key parser handles descriptor (0x72/0x73) + fixed
  (0x70/0x71) formats. Deleted fd_recovery, bg close+open thread, fd
  swap dance. -331/+155 lines.

- scsi/macos.rs: try_recover() removed (userspace handle-recovery on
  task failure was the same anti-pattern stripped from Linux). bsd_name
  field deleted. Errors bubble up directly.

- scsi/windows.rs: try_recover() removed, wide_path field deleted,
  INVALID_HANDLE guard removed.

- scsi/mod.rs: parse_sense_key() helper extracted (used by all three
  platforms now — single canonical sense-key parse rather than three
  inlined copies). +10 unit tests covering descriptor format, fixed
  format, truncated buffers, unknown response codes.

- drive/mod.rs: Drive::reset() deleted (escalating eject + STOP/START +
  reinit recovery — per audit, kernel handles its own escalation;
  userspace shouldn't).
  pub fn find_drives() -> Vec<Drive> deleted (opened N drives just to
  throw most away). find_drive() now uses discover_drives() directly.
  wait_ready() simplified — drops the reset path on sense_key=5,
  just keeps polling TUR for 60 iterations.

- lib.rs: find_drives re-export removed.

- benches/sgio_read.rs: switched to find_drive() (no longer iterates a
  drive list).

Net: 9 files changed, 226 insertions(+), 473 deletions(-). 329 tests
pass, clippy -D warnings clean. No consumer breakage (CLI, autorip,
bdemu compile + test green).

Architecture decision documented in
freemkv-private/docs/audits/2026-04-26-scsi-architecture-research.md
(primary-source survey of MakeMKV, sg_dd, ddrescue, and the kernel
mid-layer's own scsi_eh.rst escalation ladder).
This commit is contained in:
2026-04-26 09:51:46 -07:00
parent 603d569188
commit c7f5d64d1b
9 changed files with 332 additions and 473 deletions
+11 -55
View File
@@ -175,9 +175,6 @@ const VTIDX_EXECUTE_SYNC: usize = 15;
pub struct MacScsiTransport {
device_iface: ComRef,
exclusive: bool,
/// BSD name (e.g. "disk2") retained for `try_recover()` after a
/// task-level failure. Without it we can't re-call `find_scsi_service`.
bsd_name: String,
}
// IOKit COM interface pointers are Mach port references — safe to send between threads.
@@ -203,13 +200,11 @@ impl MacScsiTransport {
Ok(MacScsiTransport {
device_iface,
exclusive: true,
bsd_name: bsd_name.to_string(),
})
}
/// Resolve the BSD name → IOKit SCSITaskDeviceInterface with exclusive
/// access. Shared between `open()` and `try_recover()`. Returns the
/// COM ref the caller must release.
/// access. Returns the COM ref the caller must release.
fn acquire_device_iface(bsd_name: &str) -> Result<ComRef> {
let service = find_scsi_service(bsd_name)?;
@@ -270,42 +265,11 @@ impl MacScsiTransport {
Ok(device_iface)
}
/// Recover the IOKit interface after a task-level failure. Releases
/// the current device_iface and re-acquires fresh state via
/// `acquire_device_iface`. Same observable contract as the Linux fd
/// recovery: after `try_recover()`, the next `execute()` either uses a
/// fresh interface or returns `DeviceNotFound` if recovery failed.
///
/// Synchronous because IOKit `RELEASE_EXCLUSIVE` + `com_release` don't
/// block on in-flight CDBs the way Linux SG_IO `close` does.
fn try_recover(&mut self) {
if !self.device_iface.is_null() {
if self.exclusive {
unsafe {
type Fn = unsafe extern "C" fn(ComRef) -> IOReturn;
let f: Fn = vtable_fn(self.device_iface, VTIDX_RELEASE_EXCLUSIVE);
f(self.device_iface);
}
self.exclusive = false;
}
com_release(self.device_iface);
self.device_iface = std::ptr::null_mut();
}
match Self::acquire_device_iface(&self.bsd_name) {
Ok(new_iface) => {
self.device_iface = new_iface;
self.exclusive = true;
}
Err(_) => {
// Leave device_iface null; next execute() returns
// DeviceNotFound. Caller's retry path will reopen Drive.
self.device_iface = std::ptr::null_mut();
self.exclusive = false;
}
}
}
// `reset()` removed in 0.13.6 — see scsi/mod.rs for rationale.
// `try_recover()` removed in 0.13.20 — userspace handle-recovery on
// task failure was the same anti-pattern stripped from Linux SG_IO
// (see freemkv-private/docs/audits/2026-04-26-scsi-architecture-research.md).
// Errors bubble up; caller decides whether to reopen the Drive.
}
/// Enumerate optical drives on macOS. Mirrors `drive::macos::find_drives`
@@ -404,15 +368,6 @@ impl ScsiTransport for MacScsiTransport {
data: &mut [u8],
timeout_ms: u32,
) -> Result<ScsiResult> {
// Per RIP_DESIGN.md §15.1: parity with Linux/Windows recovery
// contract. If a prior execute() invalidated the interface and
// try_recover() also failed, fail fast.
if self.device_iface.is_null() {
return Err(Error::DeviceNotFound {
path: self.bsd_name.clone(),
});
}
// Create a SCSI task
let task: ComRef = unsafe {
type Fn = unsafe extern "C" fn(ComRef) -> ComRef;
@@ -420,7 +375,6 @@ impl ScsiTransport for MacScsiTransport {
f(self.device_iface)
};
if task.is_null() {
self.try_recover();
return Err(Error::ScsiError {
opcode: cdb[0],
status: 0xFF,
@@ -491,9 +445,8 @@ impl ScsiTransport for MacScsiTransport {
com_release(task);
if kr != K_IO_RETURN_SUCCESS {
// Task-level failure (timeout / IOKit error). Recover the
// interface so the caller's retry path can resume.
self.try_recover();
// Task-level failure (timeout / IOKit error). Bubble it up;
// the kernel mid-layer has already done what it can.
return Err(Error::ScsiError {
opcode: cdb[0],
status: 0xFF,
@@ -502,7 +455,10 @@ impl ScsiTransport for MacScsiTransport {
}
if task_status != K_SCSI_TASK_STATUS_GOOD as u32 {
let sense_key = if sense[2] != 0 { sense[2] & 0x0F } else { 0 };
// IOKit doesn't surface a "bytes written into sense buffer"
// count the way SG_IO does — pass the buffer's full length
// and let parse_sense_key inspect byte 0's response code.
let sense_key = super::parse_sense_key(&sense, sense.len() as u8);
return Err(Error::ScsiError {
opcode: cdb[0],
status: task_status as u8,