From 806c46479cc857e16c95cdf424f047fb22fedd6a Mon Sep 17 00:00:00 2001 From: viju Date: Thu, 6 Aug 2026 00:20:30 +0530 Subject: [PATCH] feat: support multiple MirageOS block devices Ref: #315 Signed-off-by: viju --- pkg/unikontainers/block.go | 63 ++++++++++++++++----- pkg/unikontainers/block_test.go | 56 ++++++++++++++++++ pkg/unikontainers/rootfs.go | 22 ++++--- pkg/unikontainers/types/types.go | 2 +- pkg/unikontainers/unikernels/mirage.go | 52 +++++++++-------- pkg/unikontainers/unikernels/mirage_test.go | 57 +++++++++++++++++++ 6 files changed, 207 insertions(+), 45 deletions(-) diff --git a/pkg/unikontainers/block.go b/pkg/unikontainers/block.go index 69db4629a..68fc9724c 100644 --- a/pkg/unikontainers/block.go +++ b/pkg/unikontainers/block.go @@ -170,25 +170,62 @@ func copyMountfiles(targetPath string, mounts []specs.Mount) error { return nil } -func handleExplicitBlockImage(blockImg string, mountPoint string) (types.BlockDevParams, error) { - if blockImg == "" { - return types.BlockDevParams{}, nil +// splitAnnotList splits an ordered comma separated annotation value into +// trimmed elements. An empty string yields a nil slice. +func splitAnnotList(s string) []string { + if s == "" { + return nil + } + parts := strings.Split(s, ",") + for i := range parts { + parts[i] = strings.TrimSpace(parts[i]) + } + return parts +} + +// handleExplicitBlockImages parses the block image and mountpoint annotations, +// each of which may hold an ordered comma separated list, into one +// BlockDevParams per declared block device. Both lists must have the same +// number of entries. A block mounted at "/" is the guest's rootfs and gets +// the "rootfs" ID. +func handleExplicitBlockImages(blockImgs string, mountPoints string) ([]types.BlockDevParams, error) { + if blockImgs == "" { + return nil, nil } - if mountPoint == "" { - return types.BlockDevParams{}, fmt.Errorf("annotation for block device was set without a mountpoint") + paths := splitAnnotList(blockImgs) + mounts := splitAnnotList(mountPoints) + if len(paths) != len(mounts) { + return nil, fmt.Errorf("block images (%d) and mount points (%d) must have the same count", + len(paths), len(mounts)) } - id := "" - if mountPoint == "/" { - id = "rootfs" + // A block device mounted at "/" is the guest rootfs, resolved elsewhere + // through a path that does not parse lists, so mixing it with others + // would silently drop it. Empty entries shift the rest onto wrong IDs. + for i, path := range paths { + if path == "" || mounts[i] == "" { + return nil, fmt.Errorf("block image and mount point entries can not be empty") + } + if mounts[i] == "/" && len(paths) > 1 { + return nil, fmt.Errorf("a block device mounted at / can not be combined with other block devices") + } + } + + blocks := make([]types.BlockDevParams, 0, len(paths)) + for i, path := range paths { + id := "" + if mounts[i] == "/" { + id = "rootfs" + } + blocks = append(blocks, types.BlockDevParams{ + Source: path, + MountPoint: mounts[i], + ID: id, + }) } - return types.BlockDevParams{ - Source: blockImg, - MountPoint: mountPoint, - ID: id, - }, nil + return blocks, nil } // Search all the mount entries in the container's config and diff --git a/pkg/unikontainers/block_test.go b/pkg/unikontainers/block_test.go index a33cd7eff..378e30e15 100644 --- a/pkg/unikontainers/block_test.go +++ b/pkg/unikontainers/block_test.go @@ -37,3 +37,59 @@ func TestGetBlockDevice(t *testing.T) { assert.Equal(t, tmpMnt.FsType, rootFs.FsType, "Expected filesystem type to be proc") assert.Equal(t, tmpMnt.ID, rootFs.ID, "Expected ID to be empty") } + +func TestHandleExplicitBlockImages(t *testing.T) { + t.Run("parses an ordered list of block images and mount points", func(t *testing.T) { + blocks, err := handleExplicitBlockImages("/.boot/disk1, /.boot/disk2", "/data1, /data2") + assert.NoError(t, err) + assert.Len(t, blocks, 2) + assert.Equal(t, "/.boot/disk1", blocks[0].Source) + assert.Equal(t, "/data1", blocks[0].MountPoint) + assert.Equal(t, "/.boot/disk2", blocks[1].Source) + assert.Equal(t, "/data2", blocks[1].MountPoint) + }) + + t.Run("single image stays backward compatible", func(t *testing.T) { + blocks, err := handleExplicitBlockImages("/.boot/rootfs", "/") + assert.NoError(t, err) + assert.Len(t, blocks, 1) + assert.Equal(t, "rootfs", blocks[0].ID) + }) + + t.Run("empty block image yields no devices", func(t *testing.T) { + blocks, err := handleExplicitBlockImages("", "") + assert.NoError(t, err) + assert.Empty(t, blocks) + }) + + t.Run("mismatched counts return an error", func(t *testing.T) { + _, err := handleExplicitBlockImages("/a,/b", "/data") + assert.Error(t, err) + }) + + t.Run("rootfs combined with other block devices returns an error", func(t *testing.T) { + _, err := handleExplicitBlockImages("/.boot/rootfs,/.boot/data", "/,/data") + assert.Error(t, err) + }) + + t.Run("empty entries return an error", func(t *testing.T) { + _, err := handleExplicitBlockImages("/a,,/c", "/1,/2,/3") + assert.Error(t, err) + + _, err = handleExplicitBlockImages("/a,/b", "/1,") + assert.Error(t, err) + }) +} + +func TestNoRootfsMultipleBlockDevs(t *testing.T) { + n := noRootfs{ + annotBlockPath: "/.boot/disk1,/.boot/disk2", + annotBlockMountPoint: "/data1,/data2", + } + blocks, err := n.getBlockDevs() + assert.NoError(t, err) + assert.Len(t, blocks, 2) + assert.Equal(t, "/.boot/disk1", blocks[0].Source) + assert.Equal(t, "/.boot/disk2", blocks[1].Source) + assert.NotEqual(t, blocks[0].ID, blocks[1].ID, "multiple block devices must have unique IDs") +} diff --git a/pkg/unikontainers/rootfs.go b/pkg/unikontainers/rootfs.go index bd1c800f0..70e4f4294 100644 --- a/pkg/unikontainers/rootfs.go +++ b/pkg/unikontainers/rootfs.go @@ -140,18 +140,26 @@ func (n noRootfs) getMounts() ([]specs.Mount, error) { } func (n noRootfs) getBlockDevs() ([]types.BlockDevParams, error) { - blkImgs := []types.BlockDevParams{} - blockFromAnnot, err := handleExplicitBlockImage(n.annotBlockPath, + blocks, err := handleExplicitBlockImages(n.annotBlockPath, n.annotBlockMountPoint) if err != nil { return nil, err } - if blockFromAnnot.Source != "" && blockFromAnnot.MountPoint != "/" { - // TODO: Add proper support for multiple block Images from the container's - // image. This requires adding more annotations too. - blockFromAnnot.ID = "annot_vol" - blkImgs = append(blkImgs, blockFromAnnot) + blkImgs := []types.BlockDevParams{} + for i, b := range blocks { + if b.Source == "" || b.MountPoint == "/" { + continue + } + // This ID is just a placeholder. MirageOS/Solo5 overrides it with + // the solo5BlkDev annotation. We keep it unique so other monitors + // don't end up with clashing block device IDs. + if len(blocks) == 1 { + b.ID = "annot_vol" + } else { + b.ID = fmt.Sprintf("annot_vol%d", i) + } + blkImgs = append(blkImgs, b) } return blkImgs, nil diff --git a/pkg/unikontainers/types/types.go b/pkg/unikontainers/types/types.go index f5d668b0d..aa94c273b 100644 --- a/pkg/unikontainers/types/types.go +++ b/pkg/unikontainers/types/types.go @@ -90,7 +90,7 @@ type UnikernelParams struct { Version string // The version of the unikernel InitrdPath string // The path to the initrd of the unikernel NetDevName string // The name of the guest network device declared at build time - BlkDevName string // The name of the guest block device declared at build time + BlkDevName string // Ordered comma separated block device names declared at build time Net NetDevParams Block []BlockDevParams Rootfs RootfsParams // Information about rootfs diff --git a/pkg/unikontainers/unikernels/mirage.go b/pkg/unikontainers/unikernels/mirage.go index 341f2ecae..26149ec77 100644 --- a/pkg/unikontainers/unikernels/mirage.go +++ b/pkg/unikontainers/unikernels/mirage.go @@ -24,12 +24,12 @@ import ( const MirageUnikernel string = "mirage" type Mirage struct { - Command string - Monitor string - Net MirageNet - Block []MirageBlock - netDevName string - blkDevName string + Command string + Monitor string + Net MirageNet + Block []MirageBlock + netDevName string + blkDevNames []string } type MirageNet struct { @@ -73,22 +73,24 @@ func (m *Mirage) MonitorBlockCli() []types.MonitorBlockArgs { } switch m.Monitor { case "hvt", "spt": - // TODO: Explore options for multiple block devices in MirageOS - // over Solo5-spt and Solo5-hvt. Solo5 expects to use as an ID - // a specific name which the guest is also aware of in order to - // attach the respective block. As a result, urunc needs to know - // the correct ID to set, which is not straightforward. Therefore, - // there are two options. Either we read the Solo5 manifest or, - // we require specific IDs. Till we decide about that, we will - // use a single block device. We also need to find some use cases - // where multiple block devices are configured in MirageOS and check - // how MirageOS handles/configures them. - return []types.MonitorBlockArgs{ - { - ID: m.blkDevName, - Path: m.Block[0].HostPath, - }, + // Solo5 attaches each block device using an ID that the guest + // also knows from its manifest. Image builders declare those IDs + // at build time through the solo5BlkDev annotation as an ordered + // comma separated list, which we map positionally onto the block + // devices here. When no ID is given for a device, we fall back to + // the historical default of "storage". + args := make([]types.MonitorBlockArgs, 0, len(m.Block)) + for i, blk := range m.Block { + id := "storage" + if i < len(m.blkDevNames) { + id = m.blkDevNames[i] + } + args = append(args, types.MonitorBlockArgs{ + ID: id, + Path: blk.HostPath, + }) } + return args default: return nil } @@ -136,9 +138,11 @@ func (m *Mirage) Init(data types.UnikernelParams) error { m.netDevName = "service" } if data.BlkDevName != "" { - m.blkDevName = data.BlkDevName - } else { - m.blkDevName = "storage" + names := strings.Split(data.BlkDevName, ",") + for i := range names { + names[i] = strings.TrimSpace(names[i]) + } + m.blkDevNames = names } return nil diff --git a/pkg/unikontainers/unikernels/mirage_test.go b/pkg/unikontainers/unikernels/mirage_test.go index 7a01b8a23..1d26f85e1 100644 --- a/pkg/unikontainers/unikernels/mirage_test.go +++ b/pkg/unikontainers/unikernels/mirage_test.go @@ -125,3 +125,60 @@ func TestMirageBlkDevName(t *testing.T) { assert.Equal(t, "storage", args[0].ID) }) } + +func TestMirageMultipleBlkDevs(t *testing.T) { + t.Run("maps an ordered id list positionally to block devices", func(t *testing.T) { + t.Parallel() + m := &Mirage{} + err := m.Init(types.UnikernelParams{ + Monitor: "hvt", + BlkDevName: "storage,data", + Block: []types.BlockDevParams{ + {Source: "/path/to/rootfs"}, + {Source: "/path/to/data"}, + }, + }) + assert.NoError(t, err) + args := m.MonitorBlockCli() + assert.Len(t, args, 2) + assert.Equal(t, "storage", args[0].ID) + assert.Equal(t, "/path/to/rootfs", args[0].Path) + assert.Equal(t, "data", args[1].ID) + assert.Equal(t, "/path/to/data", args[1].Path) + }) + + t.Run("trims whitespace around each id", func(t *testing.T) { + t.Parallel() + m := &Mirage{} + err := m.Init(types.UnikernelParams{ + Monitor: "hvt", + BlkDevName: "storage, data", + Block: []types.BlockDevParams{ + {Source: "/a"}, + {Source: "/b"}, + }, + }) + assert.NoError(t, err) + args := m.MonitorBlockCli() + assert.Len(t, args, 2) + assert.Equal(t, "data", args[1].ID) + }) + + t.Run("falls back to storage for devices without an id", func(t *testing.T) { + t.Parallel() + m := &Mirage{} + err := m.Init(types.UnikernelParams{ + Monitor: "hvt", + BlkDevName: "storage", + Block: []types.BlockDevParams{ + {Source: "/a"}, + {Source: "/b"}, + }, + }) + assert.NoError(t, err) + args := m.MonitorBlockCli() + assert.Len(t, args, 2) + assert.Equal(t, "storage", args[0].ID) + assert.Equal(t, "storage", args[1].ID) + }) +}