feat(disk): vm disk attach --from-snapshot/--from-dir attaches a snapshot's COW copy

cmd/vm/commands.go

@@ -222,21 +222,26 @@ func buildDiskCommand(h Handler) *cobra.Command {
 
 	attach := &cobra.Command{
 		Use:   useAttachVM,
-		Short: "Hot-attach an existing raw disk file to a running VM",
+		Short: "Hot-attach an existing raw disk file, or a private copy of a snapshot's COW disk (CH only), to a running VM",
 		Args:  cobra.ExactArgs(1),
 		RunE:  h.DiskAttach,
 	}
-	attach.Flags().String("path", "", "absolute path to an existing raw disk file (required; never deleted by cocoon)")
-	attach.Flags().String("name", "", "disk name: guest /dev/disk/by-id/virtio-<name> and the detach key (required)")
+	attach.Flags().String("path", "", "absolute path to an existing raw disk file (never deleted by cocoon)")
+	attach.Flags().String("from-snapshot", "", "attach a read-write copy of this local snapshot's COW disk; the VM must run the snapshot's image")
+	attach.Flags().String("from-dir", "", "attach a read-write copy of the COW disk in this export dir (snapshot.json + its disk_attach_files)")
+	attach.Flags().String("name", "", "disk name: guest /dev/disk/by-id/virtio-<name> and the detach key (required with --path; a snapshot copy is named snap-<snapshot id prefix>)")
 	attach.Flags().Bool("readonly", false, "attach read-only")
 	attach.Flags().String("directio", "auto", "O_DIRECT for the disk: on|off|auto (off for files on tmpfs)")
-	_ = attach.MarkFlagRequired("path")
-	_ = attach.MarkFlagRequired("name")
+	attach.MarkFlagsOneRequired("path", "from-snapshot", "from-dir")
+	attach.MarkFlagsMutuallyExclusive("path", "from-snapshot", "from-dir")
+	attach.MarkFlagsMutuallyExclusive("name", "from-snapshot", "from-dir")
+	attach.MarkFlagsMutuallyExclusive("readonly", "from-snapshot", "from-dir")
+	attach.MarkFlagsMutuallyExclusive("directio", "from-snapshot", "from-dir")
 	cliutil.AddOutputFlag(attach)
 
 	detach := &cobra.Command{
 		Use:   useDetachVM,
-		Short: "Detach a hot-attached disk from a running VM (keeps the backing file)",
+		Short: "Detach a hot-attached disk from a running VM (keeps a --path backing file, removes a snapshot copy)",
 		Args:  cobra.ExactArgs(1),
 		RunE:  h.DiskDetach,
 	}

cmd/vm/disk.go

@@ -1,21 +1,33 @@
 package vm
 
 import (
+	"context"
+	"fmt"
+
 	"github.com/projecteru2/core/log"
 	"github.com/spf13/cobra"
 
+	cmdcore "github.com/cocoonstack/cocoon/cmd/core"
+	"github.com/cocoonstack/cocoon/config"
 	"github.com/cocoonstack/cocoon/extend/disk"
+	"github.com/cocoonstack/cocoon/snapshot"
 	"github.com/cocoonstack/cocoon/types"
 )
 
 type diskResult struct {
-	VM    string   `json:"vm"`
-	Name  string   `json:"name"`
-	ID    string   `json:"id,omitempty"`
-	Hints []string `json:"hints,omitempty"`
+	VM     string   `json:"vm"`
+	Name   string   `json:"name"`
+	ID     string   `json:"id,omitempty"`
+	Format string   `json:"format,omitempty"`
+	Hints  []string `json:"hints,omitempty"`
 }
 
 func (h Handler) DiskAttach(cmd *cobra.Command, args []string) error {
+	fromSnapshot, _ := cmd.Flags().GetString("from-snapshot")
+	fromDir, _ := cmd.Flags().GetString("from-dir")
+	if fromSnapshot != "" || fromDir != "" {
+		return h.diskAttachSnapshot(cmd, args, fromSnapshot, fromDir)
+	}
 	ctx, _, hyper, a, err := h.resolveAttacher[disk.Attacher](cmd, args, "disk attach", disk.ErrUnsupportedBackend)
 	if err != nil {
 		return err
@@ -60,3 +72,67 @@ func (h Handler) DiskDetach(cmd *cobra.Command, args []string) error {
 		printGuestHints(out.Hints)
 	})
 }
+
+func (h Handler) diskAttachSnapshot(cmd *cobra.Command, args []string, fromSnapshot, fromDir string) error {
+	ctx, conf, _, a, err := h.resolveAttacher[disk.SnapshotAttacher](cmd, args, "snapshot disk attach", disk.ErrUnsupportedBackend)
+	if err != nil {
+		return err
+	}
+	dir, cfg, release, err := openSnapshotSource(ctx, conf, fromSnapshot, fromDir)
+	if err != nil {
+		return err
+	}
+	defer release()
+	basePath, err := cloudimgBasePath(ctx, conf, cfg.ImageBlobIDs)
+	if err != nil {
+		return err
+	}
+	name := disk.SnapshotName(cfg.ID)
+	id, format, err := a.DiskAttachSnapshot(ctx, args[0], disk.SnapshotSource{Dir: dir, Name: name, BlobIDs: cfg.ImageBlobIDs, BasePath: basePath})
+	if err != nil {
+		return classifyAttachErr(err)
+	}
+	return outputOrLog(cmd, diskResult{VM: args[0], Name: name, ID: id, Format: format}, func() {
+		log.WithFunc("cmd.vm.disk.attach").Infof(ctx, "attached snapshot disk name=%s id=%s format=%s vm=%s", name, id, format, args[0])
+	})
+}
+
+// openSnapshotSource holds a --from-snapshot read lease until release, so delete/GC cannot reap the dir mid-copy.
+func openSnapshotSource(ctx context.Context, conf *config.Config, fromSnapshot, fromDir string) (string, types.SnapshotConfig, func(), error) {
+	if fromDir != "" {
+		envelope, err := snapshot.ReadSnapshotEnvelope(fromDir)
+		if err != nil {
+			return "", types.SnapshotConfig{}, nil, fmt.Errorf("load envelope: %w", err)
+		}
+		if err = snapshot.VerifyCOWSize(fromDir, envelope.Config); err != nil {
+			return "", types.SnapshotConfig{}, nil, err
+		}
+		return fromDir, envelope.Config, func() {}, nil
+	}
+	snapBackend, err := cmdcore.InitSnapshot(ctx, conf)
+	if err != nil {
+		return "", types.SnapshotConfig{}, nil, err
+	}
+	da, ok := snapBackend.(snapshot.Direct)
+	if !ok {
+		return "", types.SnapshotConfig{}, nil, fmt.Errorf("snapshot backend %s has no local data dir", snapBackend.Type())
+	}
+	dir, cfg, release, err := da.DataDir(ctx, fromSnapshot)
+	if err != nil {
+		return "", types.SnapshotConfig{}, nil, fmt.Errorf("open snapshot %s: %w", fromSnapshot, err)
+	}
+	return dir, cfg, release, nil
+}
+
+func cloudimgBasePath(ctx context.Context, conf *config.Config, blobIDs map[string]struct{}) (string, error) {
+	_, cloudimgStore, err := cmdcore.InitImageBackendsForPull(ctx, conf)
+	if err != nil {
+		return "", err
+	}
+	for hex := range blobIDs {
+		if cloudimgStore.OwnsBlob(hex) {
+			return cloudimgStore.BlobPath(hex), nil
+		}
+	}
+	return "", nil
+}

cmd/vm/disk_test.go

@@ -0,0 +1,48 @@
+package vm
+
+import (
+	"slices"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+func TestDiskAttachFlagGroups(t *testing.T) {
+	tests := []struct {
+		name    string
+		args    []string
+		wantErr bool
+	}{
+		{"path", []string{"--path", "/vols/d.raw", "--name", "d", "--readonly", "--directio", "off"}, false},
+		{"from snapshot", []string{"--from-snapshot", "snap"}, false},
+		{"from dir", []string{"--from-dir", "/exports/snap"}, false},
+		{"name with dir", []string{"--from-dir", "/exports/snap", "--name", "reader"}, true},
+		{"no source", []string{"--name", "d"}, true},
+		{"path and snapshot", []string{"--path", "/vols/d.raw", "--from-snapshot", "snap"}, true},
+		{"snapshot and dir", []string{"--from-snapshot", "snap", "--from-dir", "/exports/snap"}, true},
+		{"readonly snapshot", []string{"--from-snapshot", "snap", "--readonly"}, true},
+		{"directio dir", []string{"--from-dir", "/exports/snap", "--directio", "off"}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			attach := diskSubcommand(t, "attach")
+			if err := attach.ParseFlags(tt.args); err != nil {
+				t.Fatalf("parse flags: %v", err)
+			}
+			err := attach.ValidateFlagGroups()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("ValidateFlagGroups() = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func diskSubcommand(t *testing.T, name string) *cobra.Command {
+	t.Helper()
+	subs := buildDiskCommand(Handler{}).Commands()
+	i := slices.IndexFunc(subs, func(c *cobra.Command) bool { return c.Name() == name })
+	if i < 0 {
+		t.Fatalf("disk %s subcommand missing", name)
+	}
+	return subs[i]
+}

docs/devices.md

@@ -72,10 +72,12 @@ Flags:
 
 | Flag | Default | Description |
 |------|---------|-------------|
-| `--path` | required | Absolute path to an existing raw disk file |
-| `--name` | required | Guest serial and detach key (`^[a-z][a-z0-9_-]{0,19}$`; the `cocoon-` prefix is reserved) |
-| `--readonly` | `false` | Attach read-only |
-| `--directio` | `auto` | O_DIRECT for the disk: `on`/`off`/`auto` (use `off` for files on tmpfs); Cloud Hypervisor only, Firecracker ignores it with a warning |
+| `--path` | empty | Absolute path to an existing raw disk file; exactly one of `--path`, `--from-snapshot`, `--from-dir` is required |
+| `--from-snapshot` | empty | Cloud Hypervisor only: attach a private read-write copy of this local snapshot's COW disk (`raw` for OCI, `qcow2` for cloudimg, reported as `format`); the VM must run the snapshot's image, layers in the same order; detach, stop and `vm rm` remove the copy |
+| `--from-dir` | empty | As `--from-snapshot`, from an export dir that holds `snapshot.json` and the files its `disk_attach_files` lists, possibly exported on another node |
+| `--name` | required with `--path` | Guest serial and detach key (`^[a-z][a-z0-9_-]{0,19}$`; the `cocoon-` prefix is reserved); not with `--from-snapshot`/`--from-dir`, whose copy is always named `snap-` plus the first 8 characters of the snapshot id, so two snapshots never share a name and a repeat of the same snapshot is a no-op |
+| `--readonly` | `false` | Attach read-only; not with `--from-snapshot`/`--from-dir` |
+| `--directio` | `auto` | O_DIRECT for the disk: `on`/`off`/`auto` (use `off` for files on tmpfs); Cloud Hypervisor only, Firecracker ignores it with a warning; not with `--from-snapshot`/`--from-dir` |
 
 On Cloud Hypervisor, detach blocks until the guest acks the ACPI eject (up to
 30s — Windows can take 10–20s), so when it returns the slot, the name, and the

extend/disk/disk.go

@@ -1,4 +1,4 @@
-// Package disk is the runtime attach interface for extra virtio-blk disks backed by existing raw files; attach is runtime-only (snapshot/hibernate refuse while one is attached) and detach never deletes the backing file.
+// Package disk is the runtime attach interface for extra virtio-blk disks backed by existing raw files or a snapshot's COW copy; attach is runtime-only (snapshot/hibernate refuse while one is attached) and detach deletes only cocoon's own copy.
 package disk
 
 import (
@@ -11,7 +11,11 @@ import (
 	"github.com/cocoonstack/cocoon/types"
 )
 
-const diskIDPrefix = "cocoon-disk-"
+const (
+	diskIDPrefix       = "cocoon-disk-"
+	snapshotNamePrefix = "snap-"
+	snapshotNameIDLen  = 8
+)
 
 // ErrUnsupportedBackend signals the backend cannot hot-plug virtio-blk disks (Firecracker on the MMIO transport).
 var ErrUnsupportedBackend = errors.New("backend does not support disk attach")
@@ -32,13 +36,22 @@ func (s *Spec) Normalize() error {
 	if !filepath.IsAbs(s.Path) {
 		return fmt.Errorf("path must be absolute, got %q", s.Path)
 	}
-	if s.Name == "" {
-		return fmt.Errorf("name is required")
-	}
-	if !types.ValidDataDiskName(s.Name) {
-		return fmt.Errorf("name %q invalid: must match ^[a-z][a-z0-9_-]{0,19}$", s.Name)
+	return ValidateName(s.Name)
+}
+
+// SnapshotSource names a snapshot dir whose COW disk is copied and attached; BasePath is the local qcow2 base blob, empty for OCI.
+type SnapshotSource struct {
+	Dir      string
+	Name     string
+	BlobIDs  map[string]struct{}
+	BasePath string
+}
+
+func (s *SnapshotSource) Normalize() error {
+	if s.Dir == "" {
+		return fmt.Errorf("snapshot dir is required")
 	}
-	return nil
+	return ValidateName(s.Name)
 }
 
 // Attached is the inspect-time view of one hot-added disk read from the running VMM's config.
@@ -55,11 +68,32 @@ type Attacher interface {
 	DiskDetach(ctx context.Context, vmRef, name string) error
 }
 
+// SnapshotAttacher hot-plugs a private writable copy of a snapshot's COW disk; repeating the call returns the attached disk.
+type SnapshotAttacher interface {
+	DiskAttachSnapshot(ctx context.Context, vmRef string, src SnapshotSource) (deviceID, format string, err error)
+}
+
 // Lister enumerates hot-added disks from running VM state.
 type Lister interface {
 	DiskList(ctx context.Context, vmRef string) ([]Attached, error)
 }
 
+// ValidateName checks a disk name, which doubles as the guest serial and a file name component.
+func ValidateName(name string) error {
+	if name == "" {
+		return fmt.Errorf("name is required")
+	}
+	if !types.ValidDataDiskName(name) {
+		return fmt.Errorf("name %q invalid: must match ^[a-z][a-z0-9_-]{0,19}$", name)
+	}
+	return nil
+}
+
+// SnapshotName is the disk name of a snapshot-sourced attach; derived from the snapshot so two snapshots never collide on one name.
+func SnapshotName(snapID string) string {
+	return snapshotNamePrefix + strings.ToLower(snapID[:min(len(snapID), snapshotNameIDLen)])
+}
+
 // DeriveID returns the deterministic Cloud Hypervisor device id for a disk name (attach and detach share it so concurrent attaches collide on CH's id check); Firecracker derives its own underscore form.
 func DeriveID(name string) string {
 	return diskIDPrefix + name

extend/disk/disk_test.go

@@ -25,6 +25,45 @@ func TestSpecNormalize(t *testing.T) {
 	}
 }
 
+func TestSnapshotSourceNormalize(t *testing.T) {
+	tests := []struct {
+		name    string
+		src     SnapshotSource
+		wantErr bool
+	}{
+		{"valid", SnapshotSource{Dir: "/snap", Name: "snap-abcd"}, false},
+		{"missing dir", SnapshotSource{Name: "snap-abcd"}, true},
+		{"missing name", SnapshotSource{Dir: "/snap"}, true},
+		{"path in name", SnapshotSource{Dir: "/snap", Name: "a/../b"}, true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.src.Normalize()
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Normalize() = %v, wantErr %v", err, tt.wantErr)
+			}
+		})
+	}
+}
+
+func TestSnapshotName(t *testing.T) {
+	tests := []struct {
+		snapID, want string
+	}{
+		{"7QXKZ2MVB4N5LTRWYHC3DAEFGJ", "snap-7qxkz2mv"},
+		{"ABC", "snap-abc"},
+	}
+	for _, tt := range tests {
+		got := SnapshotName(tt.snapID)
+		if got != tt.want {
+			t.Errorf("SnapshotName(%q) = %q, want %q", tt.snapID, got, tt.want)
+		}
+		if err := ValidateName(got); err != nil {
+			t.Errorf("SnapshotName(%q) = %q is not a valid disk name: %v", tt.snapID, got, err)
+		}
+	}
+}
+
 func TestDeriveIDRoundTrip(t *testing.T) {
 	id := DeriveID("vol1")
 	if id != "cocoon-disk-vol1" {