mirror of
https://github.com/freemkv/libfreemkv.git
synced 2026-10-07 18:13:10 +00:00
drive/scsi: DVD+ DL media is DVD, sg open errno keeps its cause, status-only failures carry no sense; may-block labels, stale comments; tests for the stop, CDB and allow-list guards
This commit is contained in:
@@ -117,6 +117,9 @@ mod tests {
|
|||||||
assert!(allowed_after_cancel(&EJECT, none, FinishEject));
|
assert!(allowed_after_cancel(&EJECT, none, FinishEject));
|
||||||
assert!(!allowed_after_cancel(&EJECT, locked, Plain));
|
assert!(!allowed_after_cancel(&EJECT, locked, Plain));
|
||||||
assert!(!allowed_after_cancel(&START, none, FinishEject), "LoEj=0");
|
assert!(!allowed_after_cancel(&START, none, FinishEject), "LoEj=0");
|
||||||
|
// LoEj=1 with Start=1 loads the tray: not an eject.
|
||||||
|
let load = [0x1B, 0, 0, 0, 0x03, 0];
|
||||||
|
assert!(!allowed_after_cancel(&load, none, FinishEject), "Start=1");
|
||||||
// Row 3: 0x3F iff that AGID's bit is set.
|
// Row 3: 0x3F iff that AGID's bit is set.
|
||||||
assert!(allowed_after_cancel(&release(2), agid2, Plain));
|
assert!(allowed_after_cancel(&release(2), agid2, Plain));
|
||||||
assert!(!allowed_after_cancel(&release(1), agid2, Plain));
|
assert!(!allowed_after_cancel(&release(1), agid2, Plain));
|
||||||
|
|||||||
+42
-6
@@ -492,7 +492,7 @@ impl Drive {
|
|||||||
buf: &mut [u8],
|
buf: &mut [u8],
|
||||||
timeout_ms: u32,
|
timeout_ms: u32,
|
||||||
) -> Result<crate::scsi::ScsiResult> {
|
) -> Result<crate::scsi::ScsiResult> {
|
||||||
crate::halt::diag::assert_may_block("Drive::exec", Duration::MAX);
|
crate::halt::diag::assert_may_block("Drive::exec_uncancellable", Duration::MAX);
|
||||||
self.dispatch(cdb, dir, buf, timeout_ms)
|
self.dispatch(cdb, dir, buf, timeout_ms)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -507,7 +507,7 @@ impl Drive {
|
|||||||
timeout_ms: u32,
|
timeout_ms: u32,
|
||||||
ctx: CleanupCtx,
|
ctx: CleanupCtx,
|
||||||
) -> Result<crate::scsi::ScsiResult> {
|
) -> Result<crate::scsi::ScsiResult> {
|
||||||
crate::halt::diag::assert_may_block("Drive::exec", Duration::MAX);
|
crate::halt::diag::assert_may_block("Drive::exec_cleanup", Duration::MAX);
|
||||||
if self.is_halted() && !allow::allowed_after_cancel(cdb, self.ledger(), ctx) {
|
if self.is_halted() && !allow::allowed_after_cancel(cdb, self.ledger(), ctx) {
|
||||||
return Err(Error::Halted);
|
return Err(Error::Halted);
|
||||||
}
|
}
|
||||||
@@ -849,9 +849,10 @@ impl Drive {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// True when the mounted disc is a DVD (profile family `0x0010..=0x001F`).
|
/// True when the mounted disc is a DVD (profile family `0x0010..=0x001F`,
|
||||||
|
/// plus DVD+RW DL `0x002A` and DVD+R DL `0x002B`).
|
||||||
pub(crate) fn disc_is_dvd(&mut self) -> bool {
|
pub(crate) fn disc_is_dvd(&mut self) -> bool {
|
||||||
matches!(self.current_profile(), Some(p) if (0x0010..=0x001F).contains(&p))
|
matches!(self.current_profile(), Some(p) if (0x0010..=0x001F).contains(&p) || p == 0x002A || p == 0x002B)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Initialize drive — drive-prep unlock + init.
|
/// Initialize drive — drive-prep unlock + init.
|
||||||
@@ -2282,7 +2283,8 @@ mod command_tests {
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
|
|
||||||
// disc_is_dvd() must match ONLY the DVD profile family (0x0010..=0x001F).
|
// disc_is_dvd() must match ONLY the DVD profile family (0x0010..=0x001F plus
|
||||||
|
// the DVD+ DL profiles 0x002A / 0x002B).
|
||||||
#[test]
|
#[test]
|
||||||
fn disc_is_dvd_matches_only_dvd_profile_family() {
|
fn disc_is_dvd_matches_only_dvd_profile_family() {
|
||||||
let probe = |profile: u16| {
|
let probe = |profile: u16| {
|
||||||
@@ -2294,7 +2296,9 @@ mod command_tests {
|
|||||||
// DVD family → DVD (skip drive unlock, run stock for CSS).
|
// DVD family → DVD (skip drive unlock, run stock for CSS).
|
||||||
assert!(probe(0x0010), "DVD-ROM");
|
assert!(probe(0x0010), "DVD-ROM");
|
||||||
assert!(probe(0x0011), "DVD-R");
|
assert!(probe(0x0011), "DVD-R");
|
||||||
assert!(probe(0x001B), "DVD+R DL");
|
assert!(probe(0x001B), "DVD+R");
|
||||||
|
assert!(probe(0x002A), "DVD+RW DL");
|
||||||
|
assert!(probe(0x002B), "DVD+R DL");
|
||||||
// BD/UHD family → NOT DVD (must keep today's unlock path).
|
// BD/UHD family → NOT DVD (must keep today's unlock path).
|
||||||
assert!(!probe(0x0040), "BD-ROM (UHD) must NOT be classed as DVD");
|
assert!(!probe(0x0040), "BD-ROM (UHD) must NOT be classed as DVD");
|
||||||
assert!(!probe(0x0041), "BD-R");
|
assert!(!probe(0x0041), "BD-R");
|
||||||
@@ -3876,6 +3880,38 @@ mod command_tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A Stop that lands DURING the spin-up settle (the second pause) must wake it too.
|
||||||
|
#[test]
|
||||||
|
fn spin_cycle_wakes_from_its_spin_up_settle_when_stopped() {
|
||||||
|
let RecordingHarness {
|
||||||
|
drive: mut d,
|
||||||
|
cdb,
|
||||||
|
timeouts: _to,
|
||||||
|
} = recording(TransportOutcome::Ok(0));
|
||||||
|
let flag = d.halt_flag();
|
||||||
|
let stopper = std::thread::spawn(move || {
|
||||||
|
// Wait for the START (spin-up) CDB, then stop inside the settle pause.
|
||||||
|
while cdb.lock().unwrap().get(4) != Some(&0x01) {
|
||||||
|
std::thread::sleep(std::time::Duration::from_millis(20));
|
||||||
|
}
|
||||||
|
std::thread::sleep(std::time::Duration::from_millis(200));
|
||||||
|
flag.store(true, Ordering::Relaxed);
|
||||||
|
});
|
||||||
|
let t0 = std::time::Instant::now();
|
||||||
|
let r = d.spin_cycle();
|
||||||
|
let took = t0.elapsed();
|
||||||
|
stopper.join().expect("stopper thread");
|
||||||
|
assert!(
|
||||||
|
matches!(r, Err(Error::Halted)),
|
||||||
|
"a Stop during the spin-up settle must surface as Halted: {r:?}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
took < std::time::Duration::from_secs(SPIN_DOWN_IDLE_SECS + 3),
|
||||||
|
"the settle must be halt-aware, not a blind {SPIN_UP_SETTLE_SECS}s \
|
||||||
|
thread::sleep; took {took:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// A READ(10) with GOOD status but a residual underrun must be refused AND logged.
|
// A READ(10) with GOOD status but a residual underrun must be refused AND logged.
|
||||||
#[test]
|
#[test]
|
||||||
fn read_logs_a_good_status_short_transfer() {
|
fn read_logs_a_good_status_short_transfer() {
|
||||||
|
|||||||
@@ -122,6 +122,7 @@ fn every_drive_cdb_goes_through_exec() {
|
|||||||
assert!(d.mode_sense_page(0x2A).is_none());
|
assert!(d.mode_sense_page(0x2A).is_none());
|
||||||
assert!(d.read_buffer(2, 0xF1, 16).is_none());
|
assert!(d.read_buffer(2, 0xF1, 16).is_none());
|
||||||
assert!(d.report_key_rpc_state().is_none());
|
assert!(d.report_key_rpc_state().is_none());
|
||||||
|
assert!(!d.disc_is_dvd());
|
||||||
assert!(!d.enable_recovered_error_reporting());
|
assert!(!d.enable_recovered_error_reporting());
|
||||||
assert!(matches!(d.eject(), Err(Error::Halted)));
|
assert!(matches!(d.eject(), Err(Error::Halted)));
|
||||||
assert!(matches!(
|
assert!(matches!(
|
||||||
@@ -198,6 +199,18 @@ fn read_postcheck_discards_after_cancel() {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// LD3: every READ-class opcode is checked again on completion, not only READ(10).
|
||||||
|
#[test]
|
||||||
|
fn read_class_covers_every_read_opcode() {
|
||||||
|
for op in [crate::scsi::SCSI_READ_10, 0xA8, 0x88, 0xBE, 0xB9] {
|
||||||
|
assert!(is_read_class(&[op, 0, 0]), "{op:#04x}");
|
||||||
|
}
|
||||||
|
for op in [SCSI_TEST_UNIT_READY, 0x1E, 0x46, 0x25] {
|
||||||
|
assert!(!is_read_class(&[op, 0, 0]), "{op:#04x}");
|
||||||
|
}
|
||||||
|
assert!(!is_read_class(&[]));
|
||||||
|
}
|
||||||
|
|
||||||
/// LD4 (Drive half): `exec_cleanup` admits the ALLOW for a tray this Drive locked
|
/// LD4 (Drive half): `exec_cleanup` admits the ALLOW for a tray this Drive locked
|
||||||
/// and refuses anything else after a cancel.
|
/// and refuses anything else after a cancel.
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
+33
-8
@@ -145,9 +145,9 @@ impl SgIoTransport {
|
|||||||
fd
|
fd
|
||||||
}
|
}
|
||||||
|
|
||||||
// Map errno from a failed open(): permission-denied -> DevicePermission,
|
// Map errno from a failed open(): EACCES -> DevicePermission, ENOENT/ENXIO/
|
||||||
// else DeviceNotFound. Path carried in the error; no English text (app
|
// ENODEV -> DeviceNotFound, else (EMFILE, EIO, EBUSY) IoError, so the real
|
||||||
// layer localizes).
|
// cause is not reported as an unplug. No English text (app layer localizes).
|
||||||
fn open_error<T>(device: &Path) -> Result<T> {
|
fn open_error<T>(device: &Path) -> Result<T> {
|
||||||
Err(Self::map_open_error(
|
Err(Self::map_open_error(
|
||||||
&std::io::Error::last_os_error(),
|
&std::io::Error::last_os_error(),
|
||||||
@@ -160,10 +160,19 @@ impl SgIoTransport {
|
|||||||
Error::DevicePermission {
|
Error::DevicePermission {
|
||||||
path: device.display().to_string(),
|
path: device.display().to_string(),
|
||||||
}
|
}
|
||||||
} else {
|
} else if err.kind() == std::io::ErrorKind::NotFound
|
||||||
|
|| matches!(err.raw_os_error(), Some(libc::ENXIO | libc::ENODEV))
|
||||||
|
{
|
||||||
Error::DeviceNotFound {
|
Error::DeviceNotFound {
|
||||||
path: device.display().to_string(),
|
path: device.display().to_string(),
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
Error::IoError {
|
||||||
|
source: match err.raw_os_error() {
|
||||||
|
Some(code) => std::io::Error::from_raw_os_error(code),
|
||||||
|
None => std::io::Error::new(err.kind(), err.to_string()),
|
||||||
|
},
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -208,7 +217,7 @@ impl SgIoTransport {
|
|||||||
return Err(Error::ScsiError {
|
return Err(Error::ScsiError {
|
||||||
opcode: cdb[0],
|
opcode: cdb[0],
|
||||||
status: hdr.status,
|
status: hdr.status,
|
||||||
sense: Some(super::parse_sense(&sense, hdr.sb_len_wr)),
|
sense: super::sense_for_status(hdr.status, &sense, hdr.sb_len_wr),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
Ok(())
|
Ok(())
|
||||||
@@ -301,11 +310,11 @@ impl Drop for SgIoTransport {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl ScsiTransport for SgIoTransport {
|
impl ScsiTransport for SgIoTransport {
|
||||||
// Execute via one synchronous SG_IO ioctl; errors map to IoError/ScsiError.
|
|
||||||
fn last_sense_progress(&self) -> Option<u16> {
|
fn last_sense_progress(&self) -> Option<u16> {
|
||||||
self.last_progress
|
self.last_progress
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Execute via one synchronous SG_IO ioctl; errors map to IoError/ScsiError.
|
||||||
fn execute(
|
fn execute(
|
||||||
&mut self,
|
&mut self,
|
||||||
cdb: &[u8],
|
cdb: &[u8],
|
||||||
@@ -666,8 +675,6 @@ pub(super) fn disc_presence(path: &Path) -> Result<super::DiscPresence> {
|
|||||||
r
|
r
|
||||||
}
|
}
|
||||||
|
|
||||||
// After a transport failure the fd is reopened in the background and the next
|
|
||||||
// execute() adopts it. Needs a real device: FREEMKV_TEST_SG_DEVICE (default sg2).
|
|
||||||
/// Open a block device read-only (no O_DIRECT). Negative on failure.
|
/// Open a block device read-only (no O_DIRECT). Negative on failure.
|
||||||
pub(crate) fn open_block_ro(path: &str) -> i32 {
|
pub(crate) fn open_block_ro(path: &str) -> i32 {
|
||||||
let mut bytes = path.as_bytes().to_vec();
|
let mut bytes = path.as_bytes().to_vec();
|
||||||
@@ -701,6 +708,8 @@ mod recovery_device_tests {
|
|||||||
use super::*;
|
use super::*;
|
||||||
use std::time::Duration;
|
use std::time::Duration;
|
||||||
|
|
||||||
|
// After a transport failure the fd is reopened in the background and the next
|
||||||
|
// execute() adopts it. Needs a real device: FREEMKV_TEST_SG_DEVICE (default sg2).
|
||||||
#[test]
|
#[test]
|
||||||
#[ignore]
|
#[ignore]
|
||||||
fn timeout_does_not_kill_transport() {
|
fn timeout_does_not_kill_transport() {
|
||||||
@@ -741,6 +750,22 @@ mod recovery_device_tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ── CDB guard wiring ───────────────────────────────────────────────────────
|
// ── CDB guard wiring ───────────────────────────────────────────────────────
|
||||||
|
#[cfg(test)]
|
||||||
|
mod open_error_tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn open_errno_maps_to_its_real_cause() {
|
||||||
|
let p = Path::new("/dev/sg9");
|
||||||
|
let map = |code| SgIoTransport::map_open_error(&std::io::Error::from_raw_os_error(code), p);
|
||||||
|
assert!(matches!(map(libc::EACCES), Error::DevicePermission { .. }));
|
||||||
|
assert!(matches!(map(libc::ENOENT), Error::DeviceNotFound { .. }));
|
||||||
|
assert!(matches!(map(libc::ENODEV), Error::DeviceNotFound { .. }));
|
||||||
|
assert!(matches!(map(libc::EMFILE), Error::IoError { .. }));
|
||||||
|
assert!(matches!(map(libc::EBUSY), Error::IoError { .. }));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod raw_command_cdb_guard_tests {
|
mod raw_command_cdb_guard_tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|||||||
+25
-3
@@ -15,9 +15,9 @@ use std::sync::atomic::{AtomicBool, Ordering};
|
|||||||
|
|
||||||
const K_SENSE_DATA_SIZE: usize = 32;
|
const K_SENSE_DATA_SIZE: usize = 32;
|
||||||
|
|
||||||
/// Max CDB length the SCSI commands this library issues ever use; also
|
/// Max CDB length the SCSI commands this library issues ever use. An
|
||||||
/// the clamp Linux applies. Used to bound the `cdb_len` passed to the
|
/// over-length CDB is rejected (never clamped) before the `cdb_len` reaches
|
||||||
/// shim so a pathological >255-byte slice can't wrap a `u8`.
|
/// the shim, so a pathological >255-byte slice can't wrap a `u8`.
|
||||||
const K_MAX_CDB_SIZE: usize = 16;
|
const K_MAX_CDB_SIZE: usize = 16;
|
||||||
|
|
||||||
// The C shim uses a single global IOKit handle, so only one MacScsiTransport may exist at a
|
// The C shim uses a single global IOKit handle, so only one MacScsiTransport may exist at a
|
||||||
@@ -493,6 +493,28 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `execute` itself rejects an over-length CDB; the fake device would accept
|
||||||
|
/// it if the guard were bypassed and the length truncated.
|
||||||
|
#[test]
|
||||||
|
fn execute_rejects_an_oversized_cdb_before_the_shim() {
|
||||||
|
let _globals = FakeDeviceGuard(shim_globals());
|
||||||
|
assert!(
|
||||||
|
!OPEN.swap(true, Ordering::Acquire),
|
||||||
|
"OPEN held outside SHIM_GLOBALS"
|
||||||
|
);
|
||||||
|
assert_eq!(unsafe { shim_selftest_install_fake_device() }, 0);
|
||||||
|
let mut transport = MacScsiTransport {
|
||||||
|
last_progress: None,
|
||||||
|
};
|
||||||
|
for len in [K_MAX_CDB_SIZE + 1, 256 + 6] {
|
||||||
|
let r = transport.execute(&vec![0u8; len], DataDirection::None, &mut [], 1_000);
|
||||||
|
assert!(
|
||||||
|
matches!(r, Err(Error::InvalidCdbLength { .. })),
|
||||||
|
"{len}-byte CDB: {r:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// A CDB exactly at the limit must not trigger the guard, and its length
|
/// A CDB exactly at the limit must not trigger the guard, and its length
|
||||||
/// reaches the shim verbatim.
|
/// reaches the shim verbatim.
|
||||||
#[test]
|
#[test]
|
||||||
|
|||||||
@@ -1192,6 +1192,11 @@ mod transport_helper_tests {
|
|||||||
let v = get_config_010c(&mut t).unwrap();
|
let v = get_config_010c(&mut t).unwrap();
|
||||||
assert_eq!(v.len(), 28, "header + full 010Ch descriptor");
|
assert_eq!(v.len(), 28, "header + full 010Ch descriptor");
|
||||||
assert_eq!(&v[12..24], b"202101311259", "date through the minute");
|
assert_eq!(&v[12..24], b"202101311259", "date through the minute");
|
||||||
|
// MMC-6 §6.6: RT=10b (one feature), Starting Feature 010Ch, Allocation Length 28.
|
||||||
|
assert_eq!(
|
||||||
|
t.0,
|
||||||
|
[SCSI_GET_CONFIGURATION, 0x02, 0x01, 0x0C, 0, 0, 0, 0, 28, 0]
|
||||||
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+2
-2
@@ -254,8 +254,8 @@ pub(super) fn list_drives() -> Vec<super::DriveInfo> {
|
|||||||
drives
|
drives
|
||||||
}
|
}
|
||||||
|
|
||||||
/// TEST UNIT READY probe on Windows. No in-library recovery — see the
|
/// TEST UNIT READY probe on Windows. No in-library recovery: the answer is
|
||||||
/// Linux `disc_presence` doc block for the rationale.
|
/// classified by [`super::tur_disc_presence`].
|
||||||
pub(super) fn disc_presence(path: &Path) -> Result<super::DiscPresence> {
|
pub(super) fn disc_presence(path: &Path) -> Result<super::DiscPresence> {
|
||||||
let mut transport = SptiTransport::open(path)?;
|
let mut transport = SptiTransport::open(path)?;
|
||||||
let cdb = [crate::scsi::SCSI_TEST_UNIT_READY, 0, 0, 0, 0, 0];
|
let cdb = [crate::scsi::SCSI_TEST_UNIT_READY, 0, 0, 0, 0, 0];
|
||||||
|
|||||||
Reference in New Issue
Block a user