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
(internal)/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:
MattJackson
2026-04-26 09:51:46 -07:00
parent b4de5d343d
commit d2905ba7bb
9 changed files with 332 additions and 473 deletions
+63
View File
@@ -1,5 +1,68 @@
# Changelog
## 0.13.20 (2026-04-26)
### Architecture: SCSI transport — sync blocking SG_IO
`scsi/linux.rs` rewritten from async `write/poll/read + 1.5 s timeout
+ close-on-timeout in bg thread` to a single synchronous blocking
`ioctl(fd, SG_IO, &hdr)`. The old pattern abandoned slow-but-alive
commands faster than the drive could drain its internal queue,
deepening the BU40N wedge. Per the audit at
`(internal)/docs/audits/2026-04-26-scsi-architecture-research.md`,
no reference project (MakeMKV / sg_dd / ddrescue) does what we did —
all use sync blocking SG_IO with 8-60 s timeouts and let the kernel's
mid-layer (`scsi_eh.rst`) run ABORT TASK / LUN RESET / BUS RESET /
HOST RESET escalation internally.
What changed:
- `SgIoTransport::execute()` is one syscall now. Caller-supplied
`timeout_ms` is honored by the kernel, which does its own
ABORT/RESET escalation if the device times out.
- Errors check `host_status` and `driver_status` (both 0xFF-synthesised
for the caller) in addition to `status` — transport-level failures
no longer slip through as Ok.
- Sense-key parser handles both descriptor format (0x72/0x73, key at
byte 1) and fixed format (0x70/0x71, key at byte 2).
- Deleted the `fd_recovery: Arc<AtomicI32>` field, the bg close+open
thread, and the stale-fd swap dance. `scsi/linux.rs` shrank from
~720 to ~520 lines.
- Module doc rewritten to reflect the new architecture.
### Architecture: parity strip on macOS + Windows
`scsi/macos.rs` and `scsi/windows.rs` had `try_recover()`
userspace handle-recovery on task failure. Same anti-pattern as the
Linux fd-recovery dance, removed for the same reason: the kernel
mid-layer already runs its own escalation. Errors bubble up directly.
Cleanups:
- `MacScsiTransport`: `try_recover()` deleted, `bsd_name` field
deleted (was only used by try_recover), fail-fast device_iface guard
deleted (no longer null'd mid-session).
- `SptiTransport`: `try_recover()` deleted, `wide_path` field deleted,
INVALID_HANDLE guard deleted.
### API cleanup: drop `Drive::reset` and `find_drives`
Two duplicates removed from the public surface:
- `Drive::reset()` — escalating recovery (STOP/START unit + eject +
reinit). Per the audit, userspace shouldn't escalate; the kernel
already does. Only one internal caller (`wait_ready` line 195),
which now just keeps polling TUR for 60 iterations. No external
consumer used it.
- `pub fn find_drives() -> Vec<Drive>` — opened N drives just to throw
most away. Only caller was `find_drive()` itself, which now uses
`discover_drives()` directly. No external consumer used it. For
lightweight enumeration (UI sidebar etc.) use `scsi::list_drives()`.
`lib.rs` re-export of `find_drives` removed.
## 0.13.19 (2026-04-26 — held, never released)
Held in development; folded into 0.13.20.
## 0.13.18 (2026-04-26)
### Sync release — no functional changes
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "libfreemkv"
version = "0.13.18"
version = "0.13.20"
edition = "2024"
rust-version = "1.86"
license = "AGPL-3.0-only"
+3 -4
View File
@@ -8,13 +8,12 @@ fn main() {
let device = std::env::args()
.skip(1)
.find(|a| !a.starts_with('-'))
.unwrap_or_else(|| {
let drives = libfreemkv::find_drives();
if drives.is_empty() {
.unwrap_or_else(|| match libfreemkv::find_drive() {
Some(d) => d.device_path().to_string(),
None => {
eprintln!("No drives found");
std::process::exit(1);
}
drives[0].device_path().to_string()
});
let mut drive = Drive::open(Path::new(&device)).unwrap_or_else(|e| {
+13 -112
View File
@@ -177,32 +177,17 @@ impl Drive {
pub fn wait_ready(&mut self) -> Result<()> {
let tur = [SCSI_TEST_UNIT_READY, 0x00, 0x00, 0x00, 0x00, 0x00];
let mut tried_reset = false;
for _ in 0..60 {
let mut buf = [0u8; 0];
match self.scsi.as_mut().execute(
&tur,
crate::scsi::DataDirection::None,
&mut buf,
5_000,
) {
Ok(_) => return Ok(()),
Err(Error::ScsiError { sense_key: 5, .. }) if !tried_reset => {
// Illegal Request on TUR — drive may be stuck from a previous session.
// Try reset() which attempts multiple recovery approaches.
tried_reset = true;
if self.reset().is_ok() {
if self
.scsi
.as_mut()
.execute(&tur, crate::scsi::DataDirection::None, &mut buf, 5_000)
.is_ok()
{
return Ok(());
}
// If reset failed but disc is present, proceed anyway —
// the scan path will handle errors individually.
if self.drive_status() == DriveStatus::DiscPresent {
return Ok(());
}
}
Err(_) => {}
}
std::thread::sleep(std::time::Duration::from_millis(500));
}
Err(Error::DeviceNotReady {
@@ -264,86 +249,6 @@ impl Drive {
}
}
/// Attempt to reset the drive to a clean state.
///
/// Escalates through increasingly aggressive recovery:
/// 1. Unlock tray + stop/start — handles normal stuck states
/// 2. Eject cycle — clears LibreDrive firmware stuck state (proven on BU40N)
/// 3. Re-init — firmware re-upload if profile available
///
/// Note: step 2 physically ejects the tray. On slimline drives the user
/// must push it back in manually. Returns Ok(()) if TUR succeeds after
/// any step, even if the drive reports "tray open" (that's a valid state).
pub fn reset(&mut self) -> Result<()> {
let mut buf = [0u8; 0];
let tur = [SCSI_TEST_UNIT_READY, 0x00, 0x00, 0x00, 0x00, 0x00];
// 1. Unlock + stop/start
self.unlock_tray();
let stop = [SCSI_START_STOP_UNIT, 0x00, 0x00, 0x00, 0x00, 0x00];
let _ =
self.scsi
.as_mut()
.execute(&stop, crate::scsi::DataDirection::None, &mut buf, 5_000);
std::thread::sleep(std::time::Duration::from_millis(500));
let start = [SCSI_START_STOP_UNIT, 0x00, 0x00, 0x00, 0x01, 0x00];
let _ =
self.scsi
.as_mut()
.execute(&start, crate::scsi::DataDirection::None, &mut buf, 5_000);
std::thread::sleep(std::time::Duration::from_millis(2000));
if self
.scsi
.as_mut()
.execute(&tur, crate::scsi::DataDirection::None, &mut buf, 5_000)
.is_ok()
{
return Ok(());
}
// 2. Eject cycle — clears MT1959 LibreDrive stuck state.
// After eject, TUR returning "Not Ready — tray open" (sense key 2)
// counts as success: the drive is functional, just needs disc reinserted.
self.unlock_tray();
let eject = [SCSI_START_STOP_UNIT, 0x00, 0x00, 0x00, 0x02, 0x00];
let _ =
self.scsi
.as_mut()
.execute(&eject, crate::scsi::DataDirection::None, &mut buf, 30_000);
std::thread::sleep(std::time::Duration::from_millis(2000));
match self
.scsi
.as_mut()
.execute(&tur, crate::scsi::DataDirection::None, &mut buf, 5_000)
{
Ok(_) => return Ok(()),
Err(Error::ScsiError { sense_key: 2, .. }) => return Ok(()), // tray open = valid
_ => {}
}
// 3. If still stuck and we have a profile, try re-init
if self.driver.is_some() {
self.init()?;
std::thread::sleep(std::time::Duration::from_millis(1000));
match self.scsi.as_mut().execute(
&tur,
crate::scsi::DataDirection::None,
&mut buf,
5_000,
) {
Ok(_) => return Ok(()),
Err(Error::ScsiError { sense_key: 2, .. }) => return Ok(()),
_ => {}
}
}
Err(Error::DeviceResetFailed {
path: self.device_path.clone(),
})
}
pub fn platform_name(&self) -> &str {
match self.platform {
Some(ref p) => p.name(),
@@ -662,19 +567,15 @@ impl SectorReader for Drive {
}
}
/// Find all optical drives connected to this system.
/// Returns opened Drive objects ready for use.
pub fn find_drives() -> Vec<Drive> {
/// Find the first optical drive on this system and open it.
///
/// For just listing drives without opening (e.g. UI sidebar), use
/// `scsi::list_drives()` — that returns `DriveInfo` (path + identity)
/// without the cost of running every drive's profile + identity probe.
pub fn find_drive() -> Option<Drive> {
discover_drives()
.into_iter()
.filter_map(|(path, _)| Drive::open(std::path::Path::new(&path)).ok())
.collect()
}
/// Find the first optical drive.
/// Returns an opened Drive ready for use.
pub fn find_drive() -> Option<Drive> {
find_drives().into_iter().next()
.find_map(|(path, _)| Drive::open(std::path::Path::new(&path)).ok())
}
/// Halt-aware sleep primitive — wakes within ~100 ms of `halt` flipping
+1 -1
View File
@@ -103,7 +103,7 @@ pub mod verify;
pub use drive::capture::{
CapturedFeature, DriveCapture, capture_drive_data, mask_bytes, mask_string,
};
pub use drive::{Drive, DriveStatus, find_drive, find_drives};
pub use drive::{Drive, DriveStatus, find_drive};
// ─── Errors ─────────────────────────────────────────────────────────────────
//
+87 -244
View File
@@ -1,11 +1,23 @@
//! Linux SCSI transport via async sg write/poll/read.
//! Linux SCSI transport via synchronous blocking SG_IO ioctl.
//!
//! Uses the sg driver's asynchronous interface instead of the blocking
//! SG_IO ioctl. Commands are submitted via write(), waited on via
//! poll() with a hard timeout, and completed via read(). If poll()
//! times out, the fd is abandoned (closed in a background thread) and
//! a fresh fd is opened. This gives us true user-controlled timeouts
//! that the kernel's USB error recovery cannot override.
//! `execute()` is one syscall: `ioctl(fd, SG_IO, &hdr)` blocks until the
//! kernel completes the command (success, error, or its own timeout).
//! No userspace abort, no fd close+reopen, no SG_SCSI_RESET escalation —
//! the kernel SCSI mid-layer's `scsi_eh.rst` ladder
//! (ABORT TASK → LUN RESET → BUS RESET → HOST RESET) runs internally
//! when `hdr.timeout` expires, and by the time the ioctl returns the
//! kernel has already done what it can.
//!
//! This matches what every reference project does: MakeMKV (8 s sync
//! ioctl), sg_dd (60 s sync ioctl), the kernel default for SCSI block
//! devices (30 s `/sys/.../timeout`). See
//! `(internal)/docs/audits/2026-04-26-scsi-architecture-research.md`
//! for the full primary-source audit.
//!
//! Pre-0.13.20 we ran an async `write() + poll(1.5s) + close-on-timeout +
//! bg reopen` pattern. That abandoned slow-but-alive commands faster than
//! the drive could drain its internal queue, deepening the wedge
//! pattern on the LG BU40N. Reverted in 0.13.20.
use super::{DataDirection, ScsiResult, ScsiTransport};
use crate::error::{Error, Result};
@@ -45,7 +57,7 @@ struct sg_io_hdr {
}
// Compile-time validation: sg_io_hdr must match the kernel's layout.
// 64 bytes on 64-bit, 44 bytes on 32-bit (pointer-size dependent).
// 88 bytes on 64-bit, 64 bytes on 32-bit (pointer-size dependent).
#[cfg(target_pointer_width = "64")]
const _: () = assert!(std::mem::size_of::<sg_io_hdr>() == 88);
#[cfg(target_pointer_width = "32")]
@@ -54,21 +66,12 @@ const _: () = assert!(std::mem::size_of::<sg_io_hdr>() == 64);
pub struct SgIoTransport {
fd: i32,
device_path: std::path::PathBuf,
/// Background-recovered fd. After a poll timeout `execute()` spawns a
/// thread that closes `self.fd` and opens a fresh fd; the new fd is
/// stored here. The next call to `execute()` swaps it into `self.fd`.
/// `-1` means no recovery is ready (or the recovery open failed). See
/// RIP_DESIGN.md §7 for the design rationale.
fd_recovery: std::sync::Arc<std::sync::atomic::AtomicI32>,
}
// SgIoTransport's contained types (i32, PathBuf, Arc<AtomicI32>) are all
// Send; the auto-derived Send is intentional. Sync is NOT — callers must
// hold &mut for execute(), which the trait object dispatch enforces.
impl SgIoTransport {
/// Open a SCSI device for use. Resets the drive first to ensure
/// a known good state, then opens a fresh fd for commands.
/// Open a SCSI device for use. Software-clean the kernel SG queue
/// (`reset()`) before opening so a previous killed process's queued
/// commands don't bleed into ours.
pub fn open(device: &Path) -> Result<Self> {
let device = Self::resolve_to_sg(device);
Self::reset(&device)?;
@@ -85,7 +88,6 @@ impl SgIoTransport {
Ok(SgIoTransport {
fd,
device_path: device,
fd_recovery: std::sync::Arc::new(std::sync::atomic::AtomicI32::new(-1)),
})
}
@@ -105,7 +107,7 @@ impl SgIoTransport {
/// STOP+START UNIT. Both escalations were tried in 0.13.00.13.5
/// against the LG BU40N (Initio USB-SATA bridge); both failed to
/// recover wedged drives and made the wedge worse — see
/// (internal)/postmortems/2026-04-25-bu40n-wedge-recovery.md.
/// `(internal)/postmortems/2026-04-25-bu40n-wedge-recovery.md`.
/// If the drive is genuinely unresponsive, the next workload command
/// fails naturally and the caller surfaces a "physical reconnect
/// required" prompt. Software has no path back from a wedged Initio
@@ -161,8 +163,7 @@ impl SgIoTransport {
}
/// Send a raw SCSI command on an fd. Used by reset() before the
/// transport is constructed. Uses synchronous SG_IO — fine for
/// short commands (TUR, PREVENT MEDIUM REMOVAL, START/STOP).
/// transport is constructed.
fn raw_command(fd: i32, cdb: &[u8], timeout_ms: u32) -> std::result::Result<(), ()> {
let mut sense = [0u8; 32];
let mut hdr: sg_io_hdr = unsafe { std::mem::zeroed() };
@@ -178,7 +179,7 @@ impl SgIoTransport {
hdr.flags = SG_FLAG_Q_AT_HEAD;
let ret = unsafe { libc::ioctl(fd, SG_IO as _, &mut hdr as *mut sg_io_hdr) };
if ret < 0 || hdr.status != 0 {
if ret < 0 || hdr.status != 0 || hdr.host_status != 0 || hdr.driver_status != 0 {
Err(())
} else {
Ok(())
@@ -223,36 +224,32 @@ impl SgIoTransport {
impl Drop for SgIoTransport {
fn drop(&mut self) {
if self.fd >= 0 {
// Unlock tray before closing — don't leave it locked
// Unlock tray before closing — don't leave it locked.
let _ = Self::raw_command(self.fd, &[0x1E, 0, 0, 0, 0, 0], 3_000);
unsafe { libc::close(self.fd) };
}
// Drain any background-recovered fd so it doesn't leak.
let recovered = self
.fd_recovery
.swap(-1, std::sync::atomic::Ordering::Acquire);
if recovered >= 0 {
unsafe { libc::close(recovered) };
}
}
}
impl ScsiTransport for SgIoTransport {
/// Execute a SCSI command with an enforceable timeout.
/// Execute a SCSI command via synchronous blocking SG_IO.
///
/// Uses the sg driver's async write/poll/read interface:
/// 1. write() submits the command — returns immediately
/// 2. poll() waits for completion — respects our timeout exactly
/// 3. read() retrieves the result — copies data to caller's buffer
/// One syscall: `ioctl(fd, SG_IO, &hdr)`. The kernel honors
/// `hdr.timeout` and runs its own ABORT TASK → LUN RESET → BUS
/// RESET → HOST RESET escalation if the device times out (per
/// `Documentation/scsi/scsi_eh.rst`). By the time this returns,
/// the kernel has done its recovery work.
///
/// If poll() times out, the pending command is abandoned: the old fd
/// is closed in a background thread (may block while kernel finishes
/// the USB transfer) and a fresh fd is opened. The caller sees a
/// normal SCSI error and can retry.
/// Errors we surface to caller (any of these = command failed):
///
/// Without SG_FLAG_DIRECT_IO, the kernel uses internal buffers for
/// DMA and copies to userspace during read(). On timeout (no read),
/// the caller's buffer is untouched — safe to return immediately.
/// - ioctl returned -1 → `Error::IoError` (kernel-level failure)
/// - `hdr.host_status` != 0 → `Error::ScsiError` with status=0xFF
/// (transport-level: timeout, bridge wedge, etc.)
/// - `hdr.driver_status` != 0 → `Error::ScsiError` with status=0xFF
/// - `hdr.status` != 0 → `Error::ScsiError` with parsed sense key
///
/// Caller's `data` buffer is mutated only on success; partial
/// transfers are reported via `bytes_transferred = data.len() - resid`.
fn execute(
&mut self,
cdb: &[u8],
@@ -272,43 +269,6 @@ impl ScsiTransport for SgIoTransport {
"SgIoTransport::execute"
);
// Recover from a prior timeout: if a background reopen produced a
// fresh fd, swap it in. If recovery is still pending (-1), the
// background thread hasn't finished — return DeviceNotFound and let
// the caller's retry loop come back later.
if self.fd < 0 {
let recovered = self
.fd_recovery
.swap(-1, std::sync::atomic::Ordering::Acquire);
if recovered >= 0 {
tracing::trace!(
target: "freemkv::scsi",
phase = "recovery_swap_ok",
new_fd = recovered,
"fd_recovery delivered fresh fd"
);
self.fd = recovered;
} else {
tracing::trace!(
target: "freemkv::scsi",
phase = "recovery_pending",
elapsed_us = exec_t0.elapsed().as_micros() as u64,
"fd_recovery still pending → DeviceNotFound"
);
return Err(Error::DeviceNotFound {
path: self.device_path.display().to_string(),
});
}
}
let mut sense = [0u8; 32];
let dxfer_direction = match direction {
DataDirection::None => SG_DXFER_NONE,
DataDirection::FromDevice => SG_DXFER_FROM_DEV,
DataDirection::ToDevice => SG_DXFER_TO_DEV,
};
if data.len() > u32::MAX as usize {
return Err(Error::ScsiError {
opcode: cdb[0],
@@ -317,8 +277,14 @@ impl ScsiTransport for SgIoTransport {
});
}
let dxfer_direction = match direction {
DataDirection::None => SG_DXFER_NONE,
DataDirection::FromDevice => SG_DXFER_FROM_DEV,
DataDirection::ToDevice => SG_DXFER_TO_DEV,
};
let cmd_len = cdb.len().min(16) as u8;
let mut sense = [0u8; 32];
let mut hdr: sg_io_hdr = unsafe { std::mem::zeroed() };
hdr.interface_id = b'S' as i32;
hdr.dxfer_direction = dxfer_direction;
@@ -331,125 +297,43 @@ impl ScsiTransport for SgIoTransport {
hdr.timeout = timeout_ms;
hdr.flags = SG_FLAG_Q_AT_HEAD;
// Submit command asynchronously via write()
let hdr_size = std::mem::size_of::<sg_io_hdr>();
let write_t0 = std::time::Instant::now();
let wr = unsafe {
libc::write(
self.fd,
&hdr as *const sg_io_hdr as *const libc::c_void,
hdr_size,
)
};
let write_elapsed_us = write_t0.elapsed().as_micros() as u64;
if wr < 0 {
// The single blocking syscall. Returns when the device responds,
// when the kernel's timeout fires, or when the kernel's error
// recovery completes its escalation. On a healthy read this is
// <100 ms; on a slow-recovery bad sector it can be tens of
// seconds; on a hung drive it returns at `timeout_ms` with
// `host_status` flagged.
let ret = unsafe { libc::ioctl(self.fd, SG_IO as _, &mut hdr as *mut sg_io_hdr) };
let exec_elapsed_ms = exec_t0.elapsed().as_millis() as u64;
if ret < 0 {
let errno = std::io::Error::last_os_error();
tracing::trace!(
target: "freemkv::scsi",
phase = "write_err",
phase = "ioctl_err",
opcode = opcode,
errno = errno.raw_os_error().unwrap_or(0),
write_elapsed_us,
"sg write() returned <0"
exec_elapsed_ms,
"ioctl(SG_IO) returned <0"
);
return Err(Error::IoError { source: errno });
}
tracing::trace!(
target: "freemkv::scsi",
phase = "write_ok",
opcode = opcode,
wr,
write_elapsed_us,
"sg write() submitted"
);
// Wait for completion with enforceable timeout.
// Retry on EINTR (signal interrupted poll) with remaining time.
let poll_t0 = std::time::Instant::now();
let deadline = poll_t0 + std::time::Duration::from_millis(timeout_ms as u64);
let pr = loop {
let remaining = deadline
.saturating_duration_since(std::time::Instant::now())
.as_millis() as i32;
if remaining <= 0 {
break 0; // expired
}
let mut pfd = libc::pollfd {
fd: self.fd,
events: libc::POLLIN,
revents: 0,
};
let ret = unsafe { libc::poll(&mut pfd, 1, remaining) };
if ret >= 0 || std::io::Error::last_os_error().kind() != std::io::ErrorKind::Interrupted
{
break ret;
}
};
let poll_elapsed_ms = poll_t0.elapsed().as_millis() as u64;
// Transport-level failure (kernel timeout, USB bridge wedge,
// bus error). `hdr.status` may still be zero — the SCSI device
// never got to send a status byte. Surface as 0xFF so callers
// (e.g. `drive_has_disc`) can detect the wedge signature.
if hdr.host_status != 0 || hdr.driver_status != 0 {
tracing::trace!(
target: "freemkv::scsi",
phase = "poll_done",
phase = "transport_err",
opcode = opcode,
pr,
poll_elapsed_ms,
timeout_ms,
"poll() returned"
host_status = hdr.host_status,
driver_status = hdr.driver_status,
status = hdr.status,
exec_elapsed_ms,
"transport-level failure (timeout / bridge wedge)"
);
if pr <= 0 {
// Timeout (0) or fatal poll error (-1). Command is still pending
// in the kernel. Per RIP_DESIGN.md §4(b)/§7: close + reopen run
// in a background thread so the main thread is never blocked
// beyond the poll() budget. The recovered fd is published to
// `fd_recovery`; the next call to execute() picks it up.
let old_fd = self.fd;
self.fd = -1;
let c_path = Self::to_c_path(&self.device_path);
let recovery = self.fd_recovery.clone();
tracing::trace!(
target: "freemkv::scsi",
phase = "timeout_spawn_recovery",
opcode = opcode,
old_fd,
exec_elapsed_ms = exec_t0.elapsed().as_millis() as u64,
"poll timeout — spawning bg close+open"
);
std::thread::spawn(move || {
// Close blocks until the kernel finishes/aborts the
// abandoned command. Then we open a fresh fd. Both happen
// off the main thread.
let close_t0 = std::time::Instant::now();
unsafe { libc::close(old_fd) };
let close_ms = close_t0.elapsed().as_millis() as u64;
let open_t0 = std::time::Instant::now();
let new_fd = unsafe {
libc::open(
c_path.as_ptr() as *const libc::c_char,
libc::O_RDWR | libc::O_NONBLOCK | libc::O_CLOEXEC,
)
};
let open_ms = open_t0.elapsed().as_millis() as u64;
tracing::trace!(
target: "freemkv::scsi",
phase = "bg_recovery_done",
old_fd,
new_fd,
close_ms,
open_ms,
"bg recovery thread completed close+open"
);
if new_fd >= 0 {
let prev = recovery.swap(new_fd, std::sync::atomic::Ordering::Release);
if prev >= 0 {
// Stale recovery fd from a prior unclaimed attempt;
// close it so it doesn't leak.
unsafe { libc::close(prev) };
}
} else {
recovery.store(-1, std::sync::atomic::Ordering::Release);
}
});
return Err(Error::ScsiError {
opcode: cdb[0],
status: 0xFF,
@@ -457,52 +341,17 @@ impl ScsiTransport for SgIoTransport {
});
}
// Read response — copies data from kernel buffer to caller's buffer
let read_t0 = std::time::Instant::now();
let rd = unsafe {
libc::read(
self.fd,
&mut hdr as *mut sg_io_hdr as *mut libc::c_void,
hdr_size,
)
};
let read_elapsed_us = read_t0.elapsed().as_micros() as u64;
if rd < 0 {
tracing::trace!(
target: "freemkv::scsi",
phase = "read_err",
opcode = opcode,
read_elapsed_us,
exec_elapsed_ms = exec_t0.elapsed().as_millis() as u64,
"sg read() returned <0"
);
return Err(Error::IoError {
source: std::io::Error::last_os_error(),
});
}
let bytes_transferred = (data.len() as i32).saturating_sub(hdr.resid).max(0) as usize;
// SCSI-level failure: device responded, returned non-zero status.
// Parse sense key for the caller.
if hdr.status != 0 {
let sense_key = if hdr.sb_len_wr >= 3 {
let response_code = sense[0] & 0x7F;
if response_code == 0x72 || response_code == 0x73 {
// Descriptor format sense: sense key at byte 1
sense[1] & 0x0F
} else {
// Fixed format sense (0x70/0x71): sense key at byte 2
sense[2] & 0x0F
}
} else {
0
};
let sense_key = super::parse_sense_key(&sense, hdr.sb_len_wr);
tracing::trace!(
target: "freemkv::scsi",
phase = "scsi_err",
opcode = opcode,
status = hdr.status,
sense_key,
exec_elapsed_ms = exec_t0.elapsed().as_millis() as u64,
exec_elapsed_ms,
"SCSI status non-zero"
);
return Err(Error::ScsiError {
@@ -512,12 +361,13 @@ impl ScsiTransport for SgIoTransport {
});
}
let bytes_transferred = (data.len() as i32).saturating_sub(hdr.resid).max(0) as usize;
tracing::trace!(
target: "freemkv::scsi",
phase = "ok",
opcode = opcode,
bytes_transferred,
exec_elapsed_ms = exec_t0.elapsed().as_millis() as u64,
exec_elapsed_ms,
"execute() success"
);
Ok(ScsiResult {
@@ -535,10 +385,10 @@ impl ScsiTransport for SgIoTransport {
// `/dev/sg0..15` probe when sysfs is unreadable (minimal containers).
//
// `drive_has_disc` issues a single TEST UNIT READY. On the wedge signature
// (kernel returns status `0xff` with no sense) it escalates: SCSI bus reset
// → if still wedged → USB device reset (`USBDEVFS_RESET`) → retry TUR.
// Callers never see the escalation; if it fails too, surface
// `DeviceResetFailed` so the caller can back off.
// (kernel returns status `0xff` with no sense — synthesised by `execute()`
// from a non-zero `host_status`) the error bubbles directly to the caller;
// no in-library reset escalation. See the rationale block on
// `drive_has_disc` below.
/// SCSI peripheral type 5 = "CD-ROM device" (covers DVD, BD-ROM, BD-RE, etc.).
/// Stored in `/sys/class/scsi_generic/sgN/device/type` as ASCII decimal.
@@ -554,12 +404,6 @@ const SENSE_KEY_NOT_READY: u8 = 2;
/// realistic homelab (typical PERC + USB optical = ≤8 nodes).
const SG_FALLBACK_MAX: u8 = 16;
// SCSI INQUIRY field-offset constants previously lived here. They were
// used by an in-process SCSI INQUIRY parse path that 0.13.6 retired in
// favour of reading the kernel-cached sysfs identity (vendor/model/rev
// under /sys/class/scsi_generic/sgN/device/). Removed to keep clippy
// -D warnings clean.
pub(super) fn list_drives() -> Vec<super::DriveInfo> {
let mut out = Vec::new();
let names = enumerate_sg_names();
@@ -579,8 +423,7 @@ pub(super) fn list_drives() -> Vec<super::DriveInfo> {
// INQUIRY-only probe — open transport, run INQUIRY, drop. No
// identify, no init, no firmware reset preamble's secondary
// commands beyond what `SgIoTransport::open` already does (one
// SCSI bus reset on the kernel SG fd, ~2 s).
// commands beyond what `SgIoTransport::open` already does.
let info = match SgIoTransport::open(std::path::Path::new(&path)) {
Ok(mut transport) => match super::inquiry(&mut transport) {
Ok(r) => super::DriveInfo {
@@ -664,13 +507,13 @@ fn enumerate_sg_names() -> Vec<String> {
names
}
/// `drive_has_disc` = single TEST UNIT READY. Any error (including our
/// synthesised wedge signature, `ScsiError { status: 0xFF }`, when
/// `execute()` times out) bubbles straight up to the caller.
/// `drive_has_disc` = single TEST UNIT READY. Any error (including the
/// wedge signature `ScsiError { status: 0xFF }` synthesised by `execute()`
/// when `host_status` is set) bubbles straight up to the caller.
///
/// ## No in-library wedge recovery — and why
///
/// Versions 0.13.1 0.13.3 layered `scsi::reset()` + `scsi::usb_reset()`
/// Versions 0.13.10.13.3 layered `scsi::reset()` + `scsi::usb_reset()`
/// (`USBDEVFS_RESET`) escalation inside `drive_has_disc`. Production
/// testing on the LG BU40N USB BD-RE showed all three userspace recovery
/// ladders succeed at the USB transport level (the kernel logs
+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 (internal)/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,
+136
View File
@@ -43,6 +43,36 @@ pub const AACS_KEY_CLASS: u8 = 0x02;
/// a poll-loop tick.
pub(crate) const TUR_TIMEOUT_MS: u32 = 5_000;
// ── Sense-key parsing ───────────────────────────────────────────────────────
/// Extract the SPC-4 sense key from a sense buffer.
///
/// Handles both response-code formats:
/// - Descriptor format (0x72 / 0x73): sense key in the low nibble of byte 1.
/// - Fixed format (0x70 / 0x71): sense key in the low nibble of byte 2.
///
/// `sb_len_wr` is the number of bytes the transport actually wrote into
/// `sense`. When < 3 (or `sense.len() < 3`) we can't safely read either
/// the format byte or the key byte — return 0 (NO SENSE) per SPC-4 §4.5.3.
///
/// Pure function. Same parse runs on every platform backend so
/// callers don't have to special-case Linux SG_IO vs macOS IOKit vs
/// Windows SPTI sense layouts.
pub(crate) fn parse_sense_key(sense: &[u8], sb_len_wr: u8) -> u8 {
if (sb_len_wr as usize) < 3 || sense.len() < 3 {
return 0;
}
let response_code = sense[0] & 0x7F;
if response_code == 0x72 || response_code == 0x73 {
sense[1] & 0x0F
} else {
// Fixed format (0x70/0x71) and any unknown code fall through here;
// SPC-4 says implementations MUST tolerate unknown response codes
// and treat them as fixed — matches what reference projects do.
sense[2] & 0x0F
}
}
// ── Types ───────────────────────────────────────────────────────────────────
#[derive(Debug, Clone, Copy, PartialEq)]
@@ -326,3 +356,109 @@ pub fn build_read10_raw(lba: u32, count: u16) -> [u8; 10] {
0x00,
]
}
#[cfg(test)]
mod parse_sense_tests {
//! Unit tests for `parse_sense_key`. Covers both SPC-4 sense data
//! formats (descriptor / fixed) and the short-buffer fallback. The
//! same helper runs on every platform backend so a regression here
//! would silently miscategorize SCSI errors on Linux, macOS, and
//! Windows simultaneously.
use super::parse_sense_key;
/// Helper: build a 32-byte sense buffer whose first three bytes are
/// the given prefix; the rest are zeroes (sense data area).
fn buf(b0: u8, b1: u8, b2: u8) -> [u8; 32] {
let mut s = [0u8; 32];
s[0] = b0;
s[1] = b1;
s[2] = b2;
s
}
#[test]
fn descriptor_format_72_picks_byte_1() {
// Response code 0x72 (current, descriptor): sense key is the
// low nibble of byte 1. Byte 2 here is 0x77 to prove it is NOT
// the byte the parser reads.
let s = buf(0x72, 0x05, 0x77); // ILLEGAL REQUEST
assert_eq!(parse_sense_key(&s, 8), 5);
}
#[test]
fn descriptor_format_73_picks_byte_1() {
// Response code 0x73 (deferred, descriptor): same parse rule
// as 0x72.
let s = buf(0x73, 0x06, 0xFF); // UNIT ATTENTION
assert_eq!(parse_sense_key(&s, 8), 6);
}
#[test]
fn fixed_format_70_picks_byte_2() {
// Response code 0x70 (current, fixed): sense key is the low
// nibble of byte 2. Byte 1 is 0x77 to prove it is NOT read.
let s = buf(0x70, 0x77, 0x05); // ILLEGAL REQUEST
assert_eq!(parse_sense_key(&s, 18), 5);
}
#[test]
fn fixed_format_71_picks_byte_2() {
// Response code 0x71 (deferred, fixed): same parse as 0x70.
let s = buf(0x71, 0x77, 0x02); // NOT READY
assert_eq!(parse_sense_key(&s, 18), 2);
}
#[test]
fn high_bit_in_byte_0_is_masked() {
// SPC-4 sets the top bit of byte 0 ("INFORMATION VALID" / "VALID")
// independently of the response code. parse_sense_key must mask
// it off before classifying the format.
let s = buf(0xF2, 0x05, 0x77);
assert_eq!(parse_sense_key(&s, 8), 5, "VALID-bit must not leak into format detection");
let s = buf(0xF0, 0x77, 0x02);
assert_eq!(parse_sense_key(&s, 18), 2);
}
#[test]
fn high_nibble_in_key_byte_is_masked() {
// Sense key is byte_n & 0x0F (low nibble). Top nibble holds
// FILEMARK / EOM / ILI / SDAT_OVFL flags, which must not bleed
// into the key value.
let s = buf(0x70, 0x00, 0xE5); // 0xE0 flags + key 5
assert_eq!(parse_sense_key(&s, 18), 5);
}
#[test]
fn sb_len_wr_zero_returns_no_sense() {
// Transport set status non-zero but wrote zero sense bytes —
// SPC-4 §4.5.3 says treat as NO SENSE (key 0).
let s = buf(0x72, 0x05, 0x05);
assert_eq!(parse_sense_key(&s, 0), 0);
}
#[test]
fn sb_len_wr_below_three_returns_no_sense() {
// Less than three bytes in the buffer means we can't safely
// read either format byte 0 or key byte 2 — return 0.
let s = buf(0x72, 0x05, 0x05);
assert_eq!(parse_sense_key(&s, 1), 0);
assert_eq!(parse_sense_key(&s, 2), 0);
}
#[test]
fn slice_below_three_returns_no_sense() {
// Defense-in-depth: even if a caller passes a too-short slice
// with a falsely-large sb_len_wr, we don't panic and we return 0.
let s = [0x72u8, 0x05];
assert_eq!(parse_sense_key(&s, 8), 0);
}
#[test]
fn unknown_response_code_falls_through_to_fixed() {
// SPC-4 mandates implementations tolerate unknown response
// codes and treat them as fixed format. Vendor-specific codes
// in the 0x74..0x7E range surface here.
let s = buf(0x7A, 0x77, 0x03); // MEDIUM ERROR via "fixed"
assert_eq!(parse_sense_key(&s, 18), 3);
}
}
+16 -55
View File
@@ -85,15 +85,11 @@ unsafe extern "system" {
pub struct SptiTransport {
handle: isize,
/// Wide-encoded device path used by `try_recover()` to reopen the
/// handle after a failed DeviceIoControl. Saved from `open()` so we
/// don't have to re-resolve the device path on recovery.
wide_path: Vec<u16>,
}
// SptiTransport contains an isize HANDLE and a Vec<u16>; both Send. The
// auto-derived Send is intentional. Sync is NOT handle mutation in
// execute() requires &mut, enforced by the trait object dispatch.
// SptiTransport's only field is the isize HANDLE — Send is auto-derived
// and intentional. Sync is NOT: handle mutation in execute() requires
// &mut, enforced by the trait object dispatch.
/// Normalize a device path to Windows \\.\X: format.
///
@@ -151,34 +147,7 @@ impl SptiTransport {
});
}
Ok(SptiTransport {
handle,
wide_path: wide,
})
}
/// Recover the handle after a failed DeviceIoControl. Closes the bad
/// handle and opens a fresh one synchronously (CloseHandle/CreateFileW
/// are fast on Windows — no in-flight CDB to drain like Linux SG_IO).
/// On success, `self.handle` is replaced and the next `execute()` call
/// uses the new handle. On failure, `self.handle` is set to
/// INVALID_HANDLE_VALUE and subsequent calls return `DeviceNotFound`.
fn try_recover(&mut self) {
if self.handle != INVALID_HANDLE_VALUE {
unsafe { CloseHandle(self.handle) };
}
let new_handle = unsafe {
CreateFileW(
self.wide_path.as_ptr(),
GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE,
std::ptr::null(),
OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL,
std::ptr::null(),
)
};
self.handle = new_handle;
Ok(SptiTransport { handle })
}
/// Reset the drive to a known good state.
@@ -282,15 +251,6 @@ impl ScsiTransport for SptiTransport {
data: &mut [u8],
timeout_ms: u32,
) -> Result<ScsiResult> {
// Per RIP_DESIGN.md §15.1: parity with Linux's recovery contract.
// If a prior call invalidated the handle and try_recover() also
// failed, fail fast.
if self.handle == INVALID_HANDLE_VALUE {
return Err(Error::DeviceNotFound {
path: String::new(),
});
}
// Zero the data buffer for reads to prevent returning uninitialized data
// if the driver doesn't fully update DataTransferLength.
if direction == DataDirection::FromDevice {
@@ -339,12 +299,13 @@ impl ScsiTransport for SptiTransport {
};
if ok == 0 {
// Driver-level failure (timeout, handle gone, etc.). Recover
// the handle so the caller's retry loop can resume — same
// observable contract as Linux's async fd recovery, but
// synchronous because Windows's CloseHandle/CreateFileW don't
// block on in-flight CDBs.
self.try_recover();
// Driver-level failure (timeout, handle gone, etc.). Bubble
// up; in-library handle recovery was removed in 0.13.20 along
// with Linux's async fd-recovery and macOS's `try_recover` —
// the kernel mid-layer already did its escalation by the time
// DeviceIoControl returned, and re-issuing reset/reopen here
// is at best redundant and at worst deepens the wedge. Caller
// surfaces the failure to UX.
return Err(Error::ScsiError {
opcode: cdb[0],
status: 0xFF,
@@ -353,11 +314,11 @@ impl ScsiTransport for SptiTransport {
}
if sptwb.spt.ScsiStatus != 0 {
let sense_key = if sptwb.sense[2] != 0 {
sptwb.sense[2] & 0x0F
} else {
0
};
// SPTI doesn't surface a "bytes written into sense buffer"
// count separate from SenseInfoLength (input). Pass the full
// K_SENSE_SIZE; parse_sense_key keys off byte 0's response
// code to handle descriptor (0x72/0x73) vs fixed (0x70/0x71).
let sense_key = super::parse_sense_key(&sptwb.sense, K_SENSE_SIZE as u8);
return Err(Error::ScsiError {
opcode: cdb[0],
status: sptwb.spt.ScsiStatus,