libfreemkv: extend DiscRead with SCSI status/sense for 30% wedge diagnostics

2026-04-27 16:47:27 -07:00
parent c16fb8ac9a
commit 8fa748cdb4
7 changed files with 143 additions and 23 deletions
@@ -1,5 +1,42 @@
 # Changelog

+## 0.13.26 (2026-04-27)
+
+### Extend DiscRead with SCSI status/sense for 30% wedge diagnostics
+
+`Error::DiscRead` now carries `status` (SCSI status byte) and `sense`
+(ScsiSense with key/asc/ascq). Previously this info was discarded,
+showing only `E6000: {sector}`. Now shows:
+- `E6000: {sector} 0x{status}/0x{sense_key}/0x{asc}`
+- Enables recovery loop to distinguish recoverable errors from drive wedge
+- Enables programmatic handling: `if status == 0xFF { reset } else { retry }`
+
+### Error display shows up to 5 fields
+
+Display format changed from `E{sector}` to `E{code}: sector status/key/asc`.
+
+## 0.13.25 (2026-04-27)
+
+### Drop dead `Drive::device_path_owned()`
+
+The method was marked `// NOTE: Debug aid — remove after fd issue is
+resolved` and the fd issue closed in 0.13.6. Use `device_path()` (which
+returns `&str`) instead. Removing it clears a `cargo clippy -- -D
+warnings` red on Linux CI that the Mac toolchain doesn't catch.
+
+### Pre-commit gate uses CI's exact toolchain
+
+`freemkv-private/scripts/precommit.sh` (new) runs `cargo +1.86 fmt
+--check`, `cargo +1.86 clippy -- -D warnings`, and `cargo +1.86 test
+--tests` across all 5 freemkv crates. Mirrors each repo's
+`.github/workflows/ci.yml` step-for-step. Use it as a pre-commit hook
+or run by hand before pushing — green here means green CI.
+
+The Mac default toolchain is newer (1.94) and its clippy rejects
+slightly different sets of lints than 1.86 — running locally without
+pinning misses lints CI catches. The script forces 1.86 so drift
+between local and CI ends.
+
 ## 0.13.24 (2026-04-27)

 ### MapStats: split `bytes_pending` into `bytes_nontried` + `bytes_retryable`
@@ -1,6 +1,6 @@
 [package]
 name = "libfreemkv"
-version = "0.13.24"
+version = "0.13.26"
 edition = "2024"
 rust-version = "1.86"
 license = "AGPL-3.0-only"
@@ -13,7 +13,7 @@ mod dvd;
 mod encrypt;
 pub mod mapfile;

-use crate::drive::Drive;
+use crate::drive::{Drive, extract_scsi_context};
 use crate::error::{Error, Result};
 use crate::sector::SectorReader;
 use crate::udf;
@@ -1404,8 +1404,15 @@ impl Disc {
                    bytes_done = bytes_done.saturating_add(block_bytes);
                } else if !opts.skip_on_error {
                    // Strict mode (skip_on_error=false): abort on first bad.
+                    let (status, sense) = block_result
+                        .err()
+                        .as_ref()
+                        .map(extract_scsi_context)
+                        .unwrap_or((0, None));
                    return Err(Error::DiscRead {
                        sector: block_lba as u64,
+                        status: Some(status),
+                        sense,
                    });
                } else if !block_result
                    .as_ref()
@@ -1,11 +1,14 @@
 //! Drive session — open, identify, and read from optical drives.
 //!
-//! Three-step open:
-//!   1. `open()` — open device, identify drive. Always OEM.
-//!   2. `wait_ready()` — wait for disc to spin up. Call before reading.
-//!   3. `init()` — activate custom firmware. Removes riplock.
 //!   4. `probe_disc()` — probe disc surface. Drive learns optimal speeds.

+pub(crate) fn extract_scsi_context(e: &Error) -> (u8, Option<crate::scsi::ScsiSense>) {
+    match e {
+        Error::ScsiError { status, sense, .. } => (*status, *sense),
+        _ => (0, None),
+    }
+}
+
 pub mod capture;

 // Per-platform discovery helpers (the `pub(crate)` `find_drives` /
@@ -160,11 +163,6 @@ impl Drive {
        self.unlock_tray();
    }

-    // NOTE: Debug aid — remove after fd issue is resolved
-    pub fn device_path_owned(&self) -> String {
-        self.device_path.clone()
-    }
-
    /// Whether this drive has a known profile (unlock parameters available).
    pub fn has_profile(&self) -> bool {
        self.profile.is_some()
@@ -466,7 +464,14 @@ impl Drive {
        ) {
            Ok(result) => Ok(result.bytes_transferred),
            Err(Error::Halted) => Err(Error::Halted),
-            Err(_) => Err(Error::DiscRead { sector: lba as u64 }),
+            Err(e) => {
+                let (status, sense) = extract_scsi_context(&e);
+                Err(Error::DiscRead {
+                    sector: lba as u64,
+                    status: Some(status),
+                    sense,
+                })
+            }
        }
    }

@@ -183,6 +183,8 @@ pub enum Error {
    // Disc format (6xxx)
    DiscRead {
        sector: u64,
+        status: Option<u8>,
+        sense: Option<crate::scsi::ScsiSense>,
    },
    /// Drive was halted by caller.
    Halted,
@@ -392,7 +394,31 @@ impl std::fmt::Display for Error {
                None => write!(f, "E{}: 0x{:02x}/0x{:02x}", self.code(), opcode, status,),
            },
            Error::IoError { source } => write!(f, "E{}: {}", self.code(), source),
-            Error::DiscRead { sector } => write!(f, "E{}: {}", self.code(), sector),
+            Error::DiscRead {
+                sector,
+                status,
+                sense,
+            } => match (status, sense) {
+                (Some(st), Some(s)) => write!(
+                    f,
+                    "E{}: {} 0x{:02x}/0x{:02x}/0x{:02x}",
+                    self.code(),
+                    sector,
+                    st,
+                    s.sense_key,
+                    s.asc,
+                ),
+                (Some(st), None) => write!(f, "E{}: {} 0x{:02x}", self.code(), sector, st,),
+                (None, Some(s)) => write!(
+                    f,
+                    "E{}: {} 0x{:02x}/0x{:02x}",
+                    self.code(),
+                    sector,
+                    s.sense_key,
+                    s.asc,
+                ),
+                (None, None) => write!(f, "E{}: {}", self.code(), sector),
+            },
            Error::Halted => write!(f, "E{}", self.code()),
            Error::UdfNotFound { path } => write!(f, "E{}: {}", self.code(), path),
            Error::DiscTitleRange { index, count } => {
@@ -6,6 +6,7 @@
 //! Read-only. For disc→ISO (raw sector copy), use `Disc::copy()`.

 use crate::disc::{Disc, DiscTitle, Extent};
+use crate::drive::extract_scsi_context;
 use crate::event::{BatchSizeReason, Event, EventKind};
 use crate::sector::SectorReader;
 use std::io;
@@ -317,7 +318,18 @@ impl DiscStream {
                    self.current_offset += 1;
                    break;
                } else {
-                    return Err(crate::error::Error::DiscRead { sector: lba as u64 }.into());
+                    let err = self
+                        .reader
+                        .read_sectors(lba, sectors, &mut self.read_buf[..2048], false)
+                        .err();
+                    let (status, sense) =
+                        err.as_ref().map(extract_scsi_context).unwrap_or((0, None));
+                    return Err(crate::error::Error::DiscRead {
+                        sector: lba as u64,
+                        status: Some(status),
+                        sense,
+                    }
+                    .into());
                }
            }

@@ -278,10 +278,11 @@ impl UdfFs {
    /// The data_lba is partition-relative.
    fn read_icb_extent(&self, reader: &mut dyn SectorReader, meta_lba: u32) -> Result<(u32, u32)> {
        let extents = self.read_icb_extents(reader, meta_lba)?;
-        extents
-            .first()
-            .copied()
-            .ok_or(Error::DiscRead { sector: 0 })
+        extents.first().copied().ok_or(Error::DiscRead {
+            sector: 0,
+            status: None,
+            sense: None,
+        })
    }

    /// Read ALL allocation extents for a file from its ICB.
@@ -307,6 +308,8 @@ impl UdfFs {
                if ad_offset + l_ad > icb.len() {
                    return Err(Error::DiscRead {
                        sector: self.meta_to_abs(meta_lba) as u64,
+                        status: None,
+                        sense: None,
                    });
                }
                (ad_offset, l_ad)
@@ -319,11 +322,19 @@ impl UdfFs {
                if ad_offset + l_ad > icb.len() {
                    return Err(Error::DiscRead {
                        sector: self.meta_to_abs(meta_lba) as u64,
+                        status: None,
+                        sense: None,
                    });
                }
                (ad_offset, l_ad)
            }
-            _ => return Err(Error::DiscRead { sector: 0 }),
+            _ => {
+                return Err(Error::DiscRead {
+                    sector: 0,
+                    status: None,
+                    sense: None,
+                });
+            }
        };

        let mut extents = Vec::new();
@@ -414,7 +425,11 @@ pub fn read_filesystem(reader: &mut dyn SectorReader) -> Result<UdfFs> {

    let tag_id = u16::from_le_bytes([avdp[0], avdp[1]]);
    if tag_id != 2 {
-        return Err(Error::DiscRead { sector: 0 });
+        return Err(Error::DiscRead {
+            sector: 0,
+            status: None,
+            sense: None,
+        });
    }

    // Main VDS extent location: bytes [16:20] = LBA, [20:24] = length
@@ -455,14 +470,22 @@ pub fn read_filesystem(reader: &mut dyn SectorReader) -> Result<UdfFs> {
    }

    if partition_start == 0 {
-        return Err(Error::DiscRead { sector: 0 });
+        return Err(Error::DiscRead {
+            sector: 0,
+            status: None,
+            sense: None,
+        });
    }

    // Step 3: Parse partition maps from LVD to find metadata partition
    // BD-ROM discs (UDF 2.50) use a metadata partition (Type 2 map with "*UDF Metadata Partition")
    // The metadata file is stored at lba=0 of the physical partition
    let metadata_start = if num_partition_maps >= 2 {
-        let lvd_sec = lvd_sector.ok_or(Error::DiscRead { sector: 0 })?;
+        let lvd_sec = lvd_sector.ok_or(Error::DiscRead {
+            sector: 0,
+            status: None,
+            sense: None,
+        })?;

        // Read LVD to check partition map type
        let mut lvd = [0u8; 2048];
@@ -497,6 +520,8 @@ pub fn read_filesystem(reader: &mut dyn SectorReader) -> Result<UdfFs> {
                    if ad_off + 8 > meta_icb.len() {
                        return Err(Error::DiscRead {
                            sector: meta_file_lba as u64,
+                            status: None,
+                            sense: None,
                        });
                    }
                    let ad_len = u32::from_le_bytes([
@@ -536,7 +561,11 @@ pub fn read_filesystem(reader: &mut dyn SectorReader) -> Result<UdfFs> {

    let fsd_tag = u16::from_le_bytes([fsd[0], fsd[1]]);
    if fsd_tag != 256 {
-        return Err(Error::DiscRead { sector: 0 });
+        return Err(Error::DiscRead {
+            sector: 0,
+            status: None,
+            sense: None,
+        });
    }

    // Root Directory ICB: long_ad at FSD offset 400
@@ -585,6 +614,8 @@ fn read_directory(
            if ad_off + 8 > icb.len() {
                return Err(Error::DiscRead {
                    sector: (meta_start + meta_lba) as u64,
+                    status: None,
+                    sense: None,
                });
            }
            let len = u32::from_le_bytes([
@@ -607,6 +638,8 @@ fn read_directory(
            if ad_off + 8 > icb.len() {
                return Err(Error::DiscRead {
                    sector: (meta_start + meta_lba) as u64,
+                    status: None,
+                    sense: None,
                });
            }
            let len = u32::from_le_bytes([