From 23893eb3781c1a4ea6b74d36ef156ac334f841b0 Mon Sep 17 00:00:00 2001 From: Joel Town Road <13539685+hippi345@users.noreply.github.com> Date: Tue, 6 Oct 2026 02:24:42 -0400 Subject: [PATCH] volume: return error instead of panicking when .dat open fails (#11619) * volume: return error instead of panicking when .dat open fails When backend.OpenVolumeFile returns an error (e.g. disk below -minFreeSpace), dataFile is nil. Calling backend.NewDiskFile(nil) immediately after caused a nil-pointer panic in f.Stat()/f.Name(). Move the existing error check to run right after OpenVolumeFile, before NewDiskFile is called, so the error is returned cleanly. Fixes seaweedfs/seaweedfs#11615 * volume: share .dat load error handling Extract datFileLoadError helper so open and create paths share one check. * volume: trim dat-open-fail test comments * volume(rust): cover unopenable .dat load path --------- Co-authored-by: Chris Lu --- seaweed-volume/src/storage/volume.rs | 25 +++++++++++++ weed/storage/volume_loading.go | 16 ++++++--- weed/storage/volume_loading_open_fail_test.go | 35 +++++++++++++++++++ 3 files changed, 71 insertions(+), 5 deletions(-) create mode 100644 weed/storage/volume_loading_open_fail_test.go diff --git a/seaweed-volume/src/storage/volume.rs b/seaweed-volume/src/storage/volume.rs index ca7b76c71..d97e52300 100644 --- a/seaweed-volume/src/storage/volume.rs +++ b/seaweed-volume/src/storage/volume.rs @@ -9687,6 +9687,31 @@ mod tests { } } + // A directory occupying the .dat path fails open even as root; load must + // return the error. Mirrors Go's TestLoad_DatOpenFail_NoNilPanic. + #[test] + fn test_load_dat_open_fail_returns_error() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().to_str().unwrap(); + + { + let _v = make_test_volume(dir); + } + + let dat_path = format!("{dir}/1.dat"); + std::fs::remove_file(&dat_path).unwrap(); + std::fs::create_dir(&dat_path).unwrap(); + + let result = Volume::new( + dir, + dir, + VolumeId(1), + NeedleMapKind::InMemory, + &VolumeSpec::default(), + ); + assert!(result.is_err(), "unopenable .dat must fail load"); + } + #[test] fn test_remote_only_volume_load_reads_from_tier_backend() { let tmp = TempDir::new().unwrap(); diff --git a/weed/storage/volume_loading.go b/weed/storage/volume_loading.go index 797e5c3ef..9a0518b85 100644 --- a/weed/storage/volume_loading.go +++ b/weed/storage/volume_loading.go @@ -198,6 +198,9 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind dataFile, err = backend.OpenVolumeFile(v.FileName(".dat"), os.O_RDONLY) v.noWriteOrDelete = true } + if err != nil { + return datFileLoadError(v.FileName(".dat"), err) + } v.lastModifiedTsSeconds = uint64(modifiedTime.Unix()) if fileSize >= super_block.SuperBlockSize { alreadyHasSuperBlock = true @@ -212,11 +215,7 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind } if err != nil { - if !os.IsPermission(err) { - return fmt.Errorf("cannot load volume data %s: %v", v.FileName(".dat"), err) - } else { - return fmt.Errorf("load data file %s: %v", v.FileName(".dat"), err) - } + return datFileLoadError(v.FileName(".dat"), err) } if alreadyHasSuperBlock { @@ -417,3 +416,10 @@ func (v *Volume) load(alsoLoadIndex bool, createDatIfMissing bool, needleMapKind return err } + +func datFileLoadError(fileName string, err error) error { + if os.IsPermission(err) { + return fmt.Errorf("load data file %s: %v", fileName, err) + } + return fmt.Errorf("cannot load volume data %s: %v", fileName, err) +} diff --git a/weed/storage/volume_loading_open_fail_test.go b/weed/storage/volume_loading_open_fail_test.go new file mode 100644 index 000000000..cc8ec0d13 --- /dev/null +++ b/weed/storage/volume_loading_open_fail_test.go @@ -0,0 +1,35 @@ +package storage + +import ( + "os" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/storage/needle" + "github.com/seaweedfs/seaweedfs/weed/storage/super_block" +) + +// A directory occupying the .dat path fails open even as root; load must +// return the error rather than crash inside backend.NewDiskFile. +func TestLoad_DatOpenFail_NoNilPanic(t *testing.T) { + dir := t.TempDir() + + v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) + if err != nil { + t.Fatalf("create volume: %v", err) + } + v.Close() + + datPath := VolumeFileName(dir, "", 1) + ".dat" + if err := os.Remove(datPath); err != nil { + t.Fatalf("remove .dat: %v", err) + } + if err := os.Mkdir(datPath, 0755); err != nil { + t.Fatalf("mkdir .dat: %v", err) + } + + v2, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) + if err == nil { + v2.Close() + t.Fatal("expected error when .dat cannot be opened, got nil") + } +}