mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-24 13:16:28 +00:00
fix(ecstore): map disk-representable StorageError variants in reverse conversion (#5980)
The StorageError -> DiskError conversion dropped seven variants with exact DiskError counterparts (FaultyRemoteDisk, DiskAccessDenied, DriveIsRoot, IsNotRegular, VolumeNotEmpty, VolumeAccessDenied, FileAccessDenied) into the DiskError::other fallback, degrading them to an opaque Io error. Two of them sit on the quorum ignore-lists in disk/error_reduce.rs, so a degraded instance would stop matching the ignore list and count toward the dominant error in reduce_errs. Also mirror the StorageError-side io::Error downdrill in From<io::Error> for DiskError: recover a StorageError boxed through From<StorageError> for io::Error instead of wrapping it as Io. Add a round-trip identity test over every DiskError variant (DiskError -> StorageError -> DiskError) and a boxed-StorageError recovery test.
This commit is contained in:
@@ -331,7 +331,14 @@ impl From<std::io::Error> for DiskError {
|
|||||||
}
|
}
|
||||||
match e.downcast::<DiskError>() {
|
match e.downcast::<DiskError>() {
|
||||||
Ok(disk_error) => disk_error,
|
Ok(disk_error) => disk_error,
|
||||||
Err(io_error) => DiskError::Io(io_error),
|
// Mirror `From<io::Error> for StorageError`: a StorageError boxed
|
||||||
|
// through `From<StorageError> for io::Error` must recover its typed
|
||||||
|
// classification instead of degrading to `DiskError::Io`, which
|
||||||
|
// quorum aggregation (`reduce_errs`) would count as a distinct error.
|
||||||
|
Err(io_error) => match io_error.downcast::<crate::error::StorageError>() {
|
||||||
|
Ok(storage_error) => storage_error.into(),
|
||||||
|
Err(io_error) => DiskError::Io(io_error),
|
||||||
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -953,6 +960,27 @@ mod tests {
|
|||||||
assert_eq!(original_disk_error, recovered_disk_error);
|
assert_eq!(original_disk_error, recovered_disk_error);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn test_io_error_with_storage_error_inside() {
|
||||||
|
use crate::error::StorageError;
|
||||||
|
|
||||||
|
// An io::Error boxing a disk-representable StorageError (as produced by
|
||||||
|
// `From<StorageError> for io::Error`) must recover the typed DiskError
|
||||||
|
// variant instead of degrading to an opaque DiskError::Io.
|
||||||
|
let io_with_storage_error: std::io::Error = StorageError::FaultyRemoteDisk.into();
|
||||||
|
let recovered: DiskError = io_with_storage_error.into();
|
||||||
|
assert_eq!(recovered, DiskError::FaultyRemoteDisk);
|
||||||
|
|
||||||
|
let io_with_storage_error: std::io::Error = StorageError::FileAccessDenied.into();
|
||||||
|
let recovered: DiskError = io_with_storage_error.into();
|
||||||
|
assert_eq!(recovered, DiskError::FileAccessDenied);
|
||||||
|
|
||||||
|
// A StorageError with no DiskError analog stays an opaque Io error.
|
||||||
|
let io_with_bucket_error: std::io::Error = StorageError::BucketNotFound("bucket".to_string()).into();
|
||||||
|
let recovered: DiskError = io_with_bucket_error.into();
|
||||||
|
assert!(matches!(recovered, DiskError::Io(_)));
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_io_error_different_kinds() {
|
fn test_io_error_different_kinds() {
|
||||||
use std::io::ErrorKind;
|
use std::io::ErrorKind;
|
||||||
|
|||||||
@@ -358,6 +358,13 @@ impl From<StorageError> for DiskError {
|
|||||||
StorageError::VolumeNotFound => DiskError::VolumeNotFound,
|
StorageError::VolumeNotFound => DiskError::VolumeNotFound,
|
||||||
StorageError::VolumeExists => DiskError::VolumeExists,
|
StorageError::VolumeExists => DiskError::VolumeExists,
|
||||||
StorageError::FileNameTooLong => DiskError::FileNameTooLong,
|
StorageError::FileNameTooLong => DiskError::FileNameTooLong,
|
||||||
|
StorageError::FaultyRemoteDisk => DiskError::FaultyRemoteDisk,
|
||||||
|
StorageError::DiskAccessDenied => DiskError::DiskAccessDenied,
|
||||||
|
StorageError::DriveIsRoot => DiskError::DriveIsRoot,
|
||||||
|
StorageError::IsNotRegular => DiskError::IsNotRegular,
|
||||||
|
StorageError::VolumeNotEmpty => DiskError::VolumeNotEmpty,
|
||||||
|
StorageError::VolumeAccessDenied => DiskError::VolumeAccessDenied,
|
||||||
|
StorageError::FileAccessDenied => DiskError::FileAccessDenied,
|
||||||
_ => DiskError::other(val),
|
_ => DiskError::other(val),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1492,6 +1499,49 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Every DiskError variant must survive DiskError -> StorageError -> DiskError
|
||||||
|
// unchanged. A variant that degrades to `DiskError::Io` on the way back loses
|
||||||
|
// its identity for quorum aggregation (`reduce_errs` classifies by variant
|
||||||
|
// equality), so ignore-list entries such as FaultyRemoteDisk and
|
||||||
|
// DiskAccessDenied would silently stop matching.
|
||||||
|
#[test]
|
||||||
|
fn test_disk_error_storage_error_round_trip_identity_all_variants() {
|
||||||
|
// DiskError codes are contiguous from 0x01, so enumerating via from_u32
|
||||||
|
// covers every variant and picks up newly appended ones automatically.
|
||||||
|
let all_variants: Vec<DiskError> = (1u32..).map_while(DiskError::from_u32).collect();
|
||||||
|
assert!(
|
||||||
|
all_variants.len() >= 42,
|
||||||
|
"DiskError variant enumeration shrank: got {}, expected at least 42",
|
||||||
|
all_variants.len()
|
||||||
|
);
|
||||||
|
|
||||||
|
for original in all_variants {
|
||||||
|
let storage_error: StorageError = original.clone().into();
|
||||||
|
let round_tripped: DiskError = storage_error.into();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
std::mem::discriminant(&original),
|
||||||
|
std::mem::discriminant(&round_tripped),
|
||||||
|
"round trip changed variant: {original:?} -> {round_tripped:?}"
|
||||||
|
);
|
||||||
|
assert_eq!(original, round_tripped, "round trip not identical for {original:?}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Io is the only payload-carrying variant: a representative kind and
|
||||||
|
// message must both survive the round trip.
|
||||||
|
let io_original = DiskError::Io(IoError::new(ErrorKind::PermissionDenied, "denied"));
|
||||||
|
let storage_error: StorageError = io_original.clone().into();
|
||||||
|
let io_round_tripped: DiskError = storage_error.into();
|
||||||
|
assert_eq!(io_original, io_round_tripped);
|
||||||
|
match io_round_tripped {
|
||||||
|
DiskError::Io(inner) => {
|
||||||
|
assert_eq!(inner.kind(), ErrorKind::PermissionDenied);
|
||||||
|
assert_eq!(inner.to_string(), "denied");
|
||||||
|
}
|
||||||
|
other => panic!("expected DiskError::Io, got {other:?}"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_storage_error_from_io_error() {
|
fn test_storage_error_from_io_error() {
|
||||||
// Test direct IO error conversion
|
// Test direct IO error conversion
|
||||||
|
|||||||
Reference in New Issue
Block a user