From 227545fabc70dd28a6e39593ad5a9c7bc6af62b6 Mon Sep 17 00:00:00 2001 From: MattJackson Date: Tue, 28 Jul 2026 11:32:13 -0700 Subject: [PATCH] scsi: promote SenseFamily to a lib-level SCSI-fact primitive Moved SenseFamily::from_sense_key + is_wedge_family from disc/read_error.rs into scsi/mod.rs (with its own tests) and re-exported at the crate root. This is pure SCSI sense-code classification -- objective hardware fact, zero recovery-policy opinion -- so it belongs in the library primitives, unlike the retry-DECISION state machine (ReadCtx/PassSummary/ReadAction/ handle_read_error) built on top of it, which is freemkv's specific recovery strategy and is moving to freemkv-engine next. disc/read_error.rs and disc/section_recover.rs now import SenseFamily from crate::scsi instead of defining/re-exporting their own copy. No behavior change. Precommit green on Rust 1.86 (fmt+clippy+test). --- src/disc/read_error.rs | 31 +--------------- src/disc/section_recover.rs | 2 +- src/lib.rs | 2 +- src/scsi/mod.rs | 70 +++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/disc/read_error.rs b/src/disc/read_error.rs index 2f93afc..cf229fa 100644 --- a/src/disc/read_error.rs +++ b/src/disc/read_error.rs @@ -11,6 +11,7 @@ use crate::error::Error; use crate::scsi; +use crate::scsi::SenseFamily; /// In-flight bookkeeping a read loop must keep across iterations. The /// handler reads and mutates this. Caller owns the storage. @@ -122,36 +123,6 @@ pub struct ReadCtx { pub marginal_recovered: u64, } -/// Coarse classification of a SCSI sense key for diagnostic logging. -/// Wedge-family events (Hardware + IllegalRequest) get their own -/// transition log when the sense family changes. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum SenseFamily { - NotReady, - Medium, - Hardware, - IllegalRequest, - Other, -} - -impl SenseFamily { - pub fn from_sense_key(sense_key: u8) -> Self { - match sense_key { - scsi::SENSE_KEY_NOT_READY => SenseFamily::NotReady, - scsi::SENSE_KEY_MEDIUM_ERROR => SenseFamily::Medium, - scsi::SENSE_KEY_HARDWARE_ERROR => SenseFamily::Hardware, - scsi::SENSE_KEY_ILLEGAL_REQUEST => SenseFamily::IllegalRequest, - _ => SenseFamily::Other, - } - } - - /// True for the "wedge family" — Hardware + IllegalRequest are - /// the senses the BU40N firmware returns in its fast-fail state. - pub fn is_wedge_family(self) -> bool { - matches!(self, SenseFamily::Hardware | SenseFamily::IllegalRequest) - } -} - impl ReadCtx { /// Initial context for a Pass 1 sweep. The job is "fast and /// accurate, get the most data in the shortest time" — Pass N diff --git a/src/disc/section_recover.rs b/src/disc/section_recover.rs index b8ffd17..1577da2 100644 --- a/src/disc/section_recover.rs +++ b/src/disc/section_recover.rs @@ -33,7 +33,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Instant; use super::patch::{SubRanges, recovery_read}; -use super::read_error::SenseFamily; +use crate::scsi::SenseFamily; use crate::sector::SectorSource; /// One 2048-byte sector. diff --git a/src/lib.rs b/src/lib.rs index b4d4544..93e0ef5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -265,7 +265,7 @@ pub use mux::{Mp4FitReport, Mp4SkipReason, mp4_fit_report}; pub use mux::build_iso_pipeline; pub use mux::resolve_mux_key_map; pub use mux::{MuxEvents, MuxInput, MuxOptions, MuxOutcome, NoopEvents, mux_stream}; -pub use scsi::{DriveInfo, ScsiSense, ScsiTransport, drive_has_disc, list_drives}; +pub use scsi::{DriveInfo, ScsiSense, ScsiTransport, SenseFamily, drive_has_disc, list_drives}; pub use sector::{ DecryptingSectorSource, FileSectorSink, FileSectorSource, KeyFetch, PrefetchedSectorSource, SectorSink, SectorSource, diff --git a/src/scsi/mod.rs b/src/scsi/mod.rs index 7a23b5c..042f099 100644 --- a/src/scsi/mod.rs +++ b/src/scsi/mod.rs @@ -119,6 +119,76 @@ pub const SENSE_KEY_DATA_PROTECT: u8 = 0x07; pub const SENSE_KEY_BLANK_CHECK: u8 = 0x08; pub const SENSE_KEY_ABORTED_COMMAND: u8 = 0x0B; +/// Coarse classification of a SCSI sense key, for callers that need to +/// branch on "what kind of failure was this" without a `match` over every +/// `SENSE_KEY_*` constant. Pure hardware-fact translation — no retry policy +/// here; see the recovery/engine layer for what to DO about a given family. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SenseFamily { + NotReady, + Medium, + Hardware, + IllegalRequest, + Other, +} + +impl SenseFamily { + pub fn from_sense_key(sense_key: u8) -> Self { + match sense_key { + SENSE_KEY_NOT_READY => SenseFamily::NotReady, + SENSE_KEY_MEDIUM_ERROR => SenseFamily::Medium, + SENSE_KEY_HARDWARE_ERROR => SenseFamily::Hardware, + SENSE_KEY_ILLEGAL_REQUEST => SenseFamily::IllegalRequest, + _ => SenseFamily::Other, + } + } + + /// True for the "wedge family" — some drives (e.g. the BU40N over a + /// USB-SATA bridge) return Hardware or IllegalRequest sense once their + /// firmware enters a fast-fail state after sustained bad-media reads. + pub fn is_wedge_family(self) -> bool { + matches!(self, SenseFamily::Hardware | SenseFamily::IllegalRequest) + } +} + +#[cfg(test)] +mod sense_family_tests { + use super::*; + + #[test] + fn classifies_each_named_sense_key() { + assert_eq!( + SenseFamily::from_sense_key(SENSE_KEY_NOT_READY), + SenseFamily::NotReady + ); + assert_eq!( + SenseFamily::from_sense_key(SENSE_KEY_MEDIUM_ERROR), + SenseFamily::Medium + ); + assert_eq!( + SenseFamily::from_sense_key(SENSE_KEY_HARDWARE_ERROR), + SenseFamily::Hardware + ); + assert_eq!( + SenseFamily::from_sense_key(SENSE_KEY_ILLEGAL_REQUEST), + SenseFamily::IllegalRequest + ); + assert_eq!( + SenseFamily::from_sense_key(SENSE_KEY_ABORTED_COMMAND), + SenseFamily::Other + ); + } + + #[test] + fn wedge_family_is_hardware_and_illegal_request_only() { + assert!(SenseFamily::Hardware.is_wedge_family()); + assert!(SenseFamily::IllegalRequest.is_wedge_family()); + assert!(!SenseFamily::Medium.is_wedge_family()); + assert!(!SenseFamily::NotReady.is_wedge_family()); + assert!(!SenseFamily::Other.is_wedge_family()); + } +} + // ── Sense parsing ─────────────────────────────────────────────────────────── /// Decoded SPC-4 sense triple — the precise reason a SCSI command failed.