pq/libfreemkv
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

v0.13.23 — stop discarding the drive's SCSI sense data

Browse Source 
Through the entire 0.13.x line, every CHECK CONDITION reply from the
drive (the standard way SCSI tells you why a sector failed) was being
collapsed into a synthetic status=0xFF, sense_key=0 transport-wedge
sentinel and the actual sense data was thrown away. Confirmed live on
the BU40N reading Dune 2 on 2026-04-27: drive returned host_status=0,
driver_status=8, status=2, exec_elapsed_ms=1416 on every bad sector
— a clean CHECK CONDITION carrying full sense data — and Disc::copy
was bailing on it as if the bridge had wedged.

Root cause: scsi/linux.rs's wedge check was
  `host_status != 0 || driver_status != 0`
SG's DRIVER_SENSE bit (0x08) is set on every CHECK CONDITION reply
just to flag "sense buffer is populated" — it's not a transport
failure on its own. Pre-fix we conflated the two and silently lost
every drive-reported error reason. macOS and Windows backends had
the same shape: they extracted sense_key only, dropping ASC/ASCQ.

API restructure (clean separation):

  Error::ScsiError {
      opcode: u8,
      status: u8,                  // 0xFF = synthetic transport-failure
      sense: Option<ScsiSense>,    // None ⇔ no sense delivered
  }

  pub struct ScsiSense { sense_key: u8, asc: u8, ascq: u8 }
  impl ScsiSense {
      pub fn is_marginal(&self) -> bool       // keys 0/1/3/B
      pub fn is_medium_error(&self) -> bool
      pub fn is_hardware_error(&self) -> bool
      pub fn is_unit_attention(&self) -> bool
      pub fn is_data_protect(&self) -> bool
      pub fn is_not_ready(&self) -> bool
      pub fn is_illegal_request(&self) -> bool
      pub fn is_aborted_command(&self) -> bool
  }

  impl Error {
      pub fn scsi_sense(&self) -> Option<&ScsiSense>
      pub fn is_scsi_transport_failure(&self) -> bool
      pub fn is_marginal_read(&self) -> bool
  }

SCSI protocol constants (SCSI_STATUS_*, SENSE_KEY_*) moved from
error.rs to scsi/mod.rs where they belong alongside SCSI_INQUIRY,
SCSI_READ_10, etc. parse_sense replaces parse_sense_key (returns the
full triple, not just the key); inline tests now exercise ASC/ASCQ
extraction at the right offsets for both descriptor (0x72/0x73) and
fixed (0x70/0x71) sense formats.

Disc::copy + Disc::patch sense-aware dispatch:
  - marginal sense (MEDIUM ERROR / ABORTED COMMAND / RECOVERED ERROR
    / NO SENSE) → engage hysteresis (Block→Single, bpt=1)
  - non-marginal sense (HARDWARE / DATA PROTECT / UNIT ATTENTION /
    NOT READY / ILLEGAL REQUEST / transport failure / kernel
    IoError) → bail with full sense info preserved; caller (autorip)
    surfaces "physical replug" / "drive failing" / "media changed"

  Pre-fix: every CHECK CONDITION → 0xFF synthetic → Disc::copy bailed
  → bytes_good froze at the bad zone. The hysteresis from v0.13.22
  was correct but never got to run. This release unblocks it.

  Disc::patch's wedged_threshold (50 consecutive failures) stays as
  defense-in-depth for chains of marginal failures; a single
  non-marginal sense now short-circuits it.

New phase=bail trace event records the bail reason with the sense
triple. phase=transport_err remains for genuine bridge wedges /
kernel timeouts; phase=scsi_err carries the parsed sense_key, asc,
ascq for drive-reported errors.

All 350 tests pass. Clippy clean across all targets.
This commit is contained in:
matthew
2026-04-26 19:06:19 -07:00
parent 0ecf7c7c46
commit 0cb497b431
13 changed files with 761 additions and 219 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/disc/mod.rs
+80 -15
View File
@@ -1382,16 +1382,14 @@ impl Disc {
let read_t0 = block_t0;
let block_bytes_usz = block_bytes as usize;
iter_count += 1;
let block_ok = reader
.read_sectors(
block_lba,
block_count,
&mut buf[..block_bytes_usz],
recovery,
)
.is_ok();
let block_result = reader.read_sectors(
block_lba,
block_count,
&mut buf[..block_bytes_usz],
recovery,
);
if block_ok {
if block_result.is_ok() {
// Fast path — full block read cleanly.
read_ok_count += 1;
if opts.decrypt {
@@ -1413,6 +1411,31 @@ impl Disc {
return Err(Error::DiscRead {
sector: block_lba as u64,
});
} else if !block_result
.as_ref()
.err()
.map(Error::is_marginal_read)
.unwrap_or(false)
{
// 0.13.23: SCSI sense-aware dispatch. Block read failed
// with a sense class outside the marginal-read set
// (real transport failure, HARDWARE ERROR, DATA
// PROTECT, UNIT ATTENTION, NOT READY, ILLEGAL
// REQUEST, kernel IoError). Bail with the full sense
// triple preserved — caller (autorip) surfaces
// "physical replug needed" / "drive failing" /
// "media changed" / etc to the user. Hysteresis
// would just hammer the same failure for thousands
// of sectors at 1.4 sec each.
let err = block_result.err().unwrap();
tracing::trace!(
target: "freemkv::disc",
phase = "bail",
lba = block_lba,
error = %err,
"block read failed with non-recoverable sense; bailing"
);
return Err(err);
} else {
// Block failed → drop to Single and read this range
// sector-by-sector. Stay in Single across subsequent
@@ -1445,9 +1468,30 @@ impl Disc {
let s_lba = block_lba + s as u32;
let s_pos = pos + (s as u64) * 2048;
let one_bytes = 2048usize;
let one_ok = reader
.read_sectors(s_lba, 1, &mut buf[..one_bytes], recovery)
.is_ok();
let one_result =
reader.read_sectors(s_lba, 1, &mut buf[..one_bytes], recovery);
// 0.13.23: same sense-aware dispatch inside Single
// mode. If a single-sector read fails with a
// non-marginal sense (transport / hardware /
// DATA PROTECT / UNIT ATTENTION / NOT READY /
// ILLEGAL REQUEST / kernel IoError), the drive
// isn't going to start succeeding for the next
// 60 sectors either — bail with full sense info
// rather than chewing through bpt=1 timeouts.
if let Err(ref e) = one_result {
if !e.is_marginal_read() {
let err = one_result.err().unwrap();
tracing::trace!(
target: "freemkv::disc",
phase = "bail",
lba = s_lba,
error = %err,
"bpt=1 read failed with non-marginal sense; bailing"
);
return Err(err);
}
}
let one_ok = one_result.is_ok();
if one_ok {
read_ok_count += 1;
consecutive_good = consecutive_good.saturating_add(1);
@@ -1769,9 +1813,30 @@ impl Disc {
let count = (block_bytes / 2048) as u16;
let bytes = count as usize * 2048;
blocks_attempted += 1;
let read_ok = reader
.read_sectors(lba, count, &mut buf[..bytes], recovery)
.is_ok();
let read_result = reader.read_sectors(lba, count, &mut buf[..bytes], recovery);
// 0.13.23: parity with Disc::copy — bail immediately on a
// non-marginal SCSI sense (transport failure, HARDWARE
// ERROR, DATA PROTECT, UNIT ATTENTION, NOT READY,
// ILLEGAL REQUEST, kernel IoError). The wedged_threshold
// counter is a defense-in-depth backstop for ~50
// consecutive marginal failures, but a single
// non-marginal error already proves the drive can't
// produce data this pass — don't waste 50× the timeout
// budget proving it again.
if let Err(ref e) = read_result {
if !e.is_marginal_read() {
let err = read_result.err().unwrap();
tracing::trace!(
target: "freemkv::disc",
phase = "patch_bail",
lba,
error = %err,
"patch read failed with non-marginal sense; bailing"
);
return Err(err);
}
}
let read_ok = read_result.is_ok();
if read_ok {
blocks_read_ok += 1;
consecutive_failures = 0;