Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 8 additions & 27 deletions internal/erofs/vmdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"fmt"
"io"
"os"
"path/filepath"
)

const (
Expand Down Expand Up @@ -95,34 +94,16 @@ ddb.adapterType = "%s"
return err
}

// DumpVMDKDescriptorToFile writes a VMDK descriptor to path atomically.
// It creates a uniquely-named temporary file in the same directory as path,
// syncs it to disk, and renames it into place. This ensures that a concurrent
// reader never observes a partially-written descriptor and that two concurrent
// writers do not clobber each other's temporary file.
func DumpVMDKDescriptorToFile(path string, cid uint32, devices []string) error {
dir := filepath.Dir(path)
f, err := os.CreateTemp(dir, "merged_fs.vmdk.*.tmp")
func DumpVMDKDescriptorToFile(vmdkdesc string, cid uint32, devices []string) error {
f, err := os.Create(vmdkdesc)
if err != nil {
return err
}
tmpName := f.Name()

werr := DumpVMDKDescriptor(f, cid, devices)
serr := f.Sync()
cerr := f.Close()

if werr != nil {
os.Remove(tmpName)
return werr
}
if serr != nil {
os.Remove(tmpName)
return serr
}
if cerr != nil {
os.Remove(tmpName)
return cerr
err = DumpVMDKDescriptor(f, cid, devices)
f.Close()
if err != nil {
defer os.Remove(vmdkdesc)
return err
}
return os.Rename(tmpName, path)
return nil
}
31 changes: 5 additions & 26 deletions internal/erofs/vmdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package erofs
import (
"os"
"path/filepath"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -38,23 +37,9 @@ func makeExtentFile(t *testing.T, dir, name string, sectors int) string {
return path
}

// tmpFilesIn returns the names of all files in dir whose name contains ".tmp".
func tmpFilesIn(t *testing.T, dir string) []string {
t.Helper()
entries, err := os.ReadDir(dir)
require.NoError(t, err)
var out []string
for _, e := range entries {
if strings.Contains(e.Name(), ".tmp") {
out = append(out, e.Name())
}
}
return out
}

// TestDumpVMDKDescriptorToFile_Atomic verifies that DumpVMDKDescriptorToFile
// TestDumpVMDKDescriptorToFile verifies that DumpVMDKDescriptorToFile
// does not leave any .tmp intermediate file behind after a successful write.
func TestDumpVMDKDescriptorToFile_Atomic(t *testing.T) {
func TestDumpVMDKDescriptorToFile(t *testing.T) {
dir := t.TempDir()
ext := makeExtentFile(t, dir, "layer.erofs", 8)

Expand All @@ -63,28 +48,24 @@ func TestDumpVMDKDescriptorToFile_Atomic(t *testing.T) {

_, err := os.Stat(descPath)
require.NoError(t, err, "descriptor file should exist")

assert.Empty(t, tmpFilesIn(t, dir), ".tmp files must not persist after a successful write")
}

// TestDumpVMDKDescriptorToFile_WriteFail verifies that DumpVMDKDescriptorToFile
// cleans up the temporary file when the write fails (e.g. a missing device).
// cleans up the vmdk file when the write fails (e.g. a missing device).
func TestDumpVMDKDescriptorToFile_WriteFail(t *testing.T) {
dir := t.TempDir()
descPath := filepath.Join(dir, "merged_fs.vmdk")

err := DumpVMDKDescriptorToFile(descPath, 0xfffffffe, []string{"/nonexistent/layer.erofs"})
require.Error(t, err)

assert.Empty(t, tmpFilesIn(t, dir), ".tmp files must be cleaned up on write failure")
_, statErr := os.Stat(descPath)
assert.True(t, os.IsNotExist(statErr), "descriptor must not exist after a failed write")
}

// TestDumpVMDKDescriptorToFile_Regenerate verifies that calling
// DumpVMDKDescriptorToFile twice on the same path atomically replaces the
// descriptor each time and leaves no .tmp residue. This mirrors the
// unconditional-regeneration behaviour used by transformMounts.
// DumpVMDKDescriptorToFile twice on the same path replaces the descriptor
// each time. This mirrors the unconditional-regeneration behaviour used by transformMounts.
func TestDumpVMDKDescriptorToFile_Regenerate(t *testing.T) {
dir := t.TempDir()
ext := makeExtentFile(t, dir, "layer.erofs", 8)
Expand All @@ -108,7 +89,6 @@ func TestDumpVMDKDescriptorToFile_Regenerate(t *testing.T) {
// Size should be identical (same inputs), confirming the file was rewritten
// rather than skipped.
assert.Equal(t, fi1.Size(), fi2.Size(), "descriptor size must match between regenerations with the same inputs")
assert.Empty(t, tmpFilesIn(t, dir), ".tmp files must not persist after second write")
}

// TestDumpVMDKDescriptorToFile_MultipleDevices verifies that a descriptor
Expand All @@ -123,5 +103,4 @@ func TestDumpVMDKDescriptorToFile_MultipleDevices(t *testing.T) {

_, err := os.Stat(descPath)
require.NoError(t, err, "descriptor file should exist after multi-device write")
assert.Empty(t, tmpFilesIn(t, dir), ".tmp files must not persist")
}
Loading