diff --git a/pkg/unikontainers/block.go b/pkg/unikontainers/block.go index bedda38f4..55928f4b0 100644 --- a/pkg/unikontainers/block.go +++ b/pkg/unikontainers/block.go @@ -209,11 +209,27 @@ func handleExplicitBlockImage(blockImg string, mountPoint string) (types.BlockDe // Search the bind mounts passed to the guest, as selected by onlyBindMounts, // and find the ones that come from a block. func getBlockVolumes(mounts []specs.Mount, ukernel types.Unikernel) ([]types.BlockDevParams, error) { + return blockVolumesFrom(mounts, ukernel, func() ([]*mountinfo.Info, error) { + return mountinfo.GetMounts(nil) + }) +} + +// blockVolumesFrom resolves every bind mount against one snapshot of the mount +// table instead of reading /proc/thread-self/mountinfo once per bind. The +// snapshot is refreshed after an unmount, because unmounting a source changes +// the table that the following lookups must see. +func blockVolumesFrom(mounts []specs.Mount, ukernel types.Unikernel, readMountInfo func() ([]*mountinfo.Info, error)) ([]types.BlockDevParams, error) { blkImgs := []types.BlockDevParams{} + + mountInfo, err := readMountInfo() + if err != nil { + return nil, fmt.Errorf("failed to read mountinfo: %w", err) + } + for i, m := range mounts { // Get the information of the source path - // from /proc/thread-self/mountinfo - mInfo, err := getMountInfo(m.Source) + // from the mount table snapshot + mInfo, err := findMountInfo(mountInfo, m.Source) if errors.Is(err, ErrMountpoint) { // ErrMountpoint means we did not find any // such mount and hence we can skip it. @@ -246,6 +262,12 @@ func getBlockVolumes(mounts []specs.Mount, ukernel types.Unikernel) ([]types.Blo if err != nil { return nil, err } + // The unmount removed the source from the mount table, so refresh + // the snapshot before resolving the next bind. + mountInfo, err = readMountInfo() + if err != nil { + return nil, fmt.Errorf("failed to read mountinfo: %w", err) + } mInfo.ID = fmt.Sprintf("vol%d", i) mInfo.HostMountPoint = mInfo.MountPoint mInfo.MountPoint = m.Destination diff --git a/pkg/unikontainers/block_test.go b/pkg/unikontainers/block_test.go index 0f563e6a9..f9ba55546 100644 --- a/pkg/unikontainers/block_test.go +++ b/pkg/unikontainers/block_test.go @@ -15,10 +15,12 @@ package unikontainers import ( + "fmt" "strings" "testing" "github.com/moby/sys/mountinfo" + "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/urunc-dev/urunc/pkg/unikontainers/types" ) @@ -70,3 +72,58 @@ func TestFindMountInfoEscapedPath(t *testing.T) { }) } } + +// stubUnikernel only implements the parts of types.Unikernel that +// blockVolumesFrom can reach in the tests below. +type stubUnikernel struct { + types.Unikernel + fsType string +} + +func (s stubUnikernel) SupportsFS(fsType string) bool { + return fsType == s.fsType +} + +func testBinds(n int) []specs.Mount { + mounts := make([]specs.Mount, n) + for i := range mounts { + mounts[i] = specs.Mount{ + Source: fmt.Sprintf("/mnt/vol%d", i), + Destination: fmt.Sprintf("/data/vol%d", i), + Type: "bind", + } + } + return mounts +} + +// TestBlockVolumesShareMountInfoSnapshot makes sure the mount table is read +// once for all the bind mounts instead of once per bind. None of the binds +// below is mounted, so every lookup ends in ErrMountpoint and no unmount +// (which would need a real mount) is triggered. +func TestBlockVolumesShareMountInfoSnapshot(t *testing.T) { + reads := 0 + readMountInfo := func() ([]*mountinfo.Info, error) { + reads++ + return nil, nil + } + + vols, err := blockVolumesFrom(testBinds(8), stubUnikernel{}, readMountInfo) + assert.NoError(t, err) + assert.Empty(t, vols) + assert.Equal(t, 1, reads, "mountinfo must be read once for all binds") +} + +func BenchmarkBlockVolumesShareMountInfoSnapshot(b *testing.B) { + line := "36 35 8:1 / /mnt/data rw,relatime shared:1 - ext4 /dev/sdb1 rw\n" + readMountInfo := func() ([]*mountinfo.Info, error) { + return mountinfo.GetMountsFromReader(strings.NewReader(strings.Repeat(line, 500)), nil) + } + mounts := testBinds(32) + + b.ResetTimer() + for b.Loop() { + if _, err := blockVolumesFrom(mounts, stubUnikernel{}, readMountInfo); err != nil { + b.Fatal(err) + } + } +}