Skip to content

Commit 5a0f4a3

Browse files
committed
vm+snapshot: /code+/simplify followups
- ExportToDir: replace inline mkdir + 3-state switch with utils.EnsureDirs (existing 0o750 helper); drop the dead snapshot.json skip in the copy loop since registered DataDir results never carry the envelope (Create doesn't write it; Import removes it via readAndRemoveSnapshotJSON). - run.go: 'nic count mismatch' -> 'NIC count mismatch' (ALL-CAPS acronym rule); fixes both restoreFromDir and the pre-existing legacy Restore message for consistency. - cloneFromDir: copy *config.Config locally before flipping UseFirecracker so the mutation doesn't leak to the caller's shared conf (CLI tolerates it; daemons embedding cocoon would notice). - restoreCmd: PreRunE rejects --force without --from-dir so misuse fails loud instead of silently no-op.
1 parent 98647c6 commit 5a0f4a3

3 files changed

Lines changed: 27 additions & 21 deletions

File tree

cmd/vm/commands.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package vm
22

33
import (
4+
"fmt"
5+
46
"github.com/spf13/cobra"
57

68
cmdcore "github.com/cocoonstack/cocoon/cmd/core"
@@ -114,7 +116,15 @@ func Command(h Actions) *cobra.Command {
114116
Use: "restore [flags] VM [SNAPSHOT]",
115117
Short: "Restore a running VM to a previous snapshot (or a directory via --from-dir)",
116118
Args: cobra.RangeArgs(1, 2),
117-
RunE: h.Restore,
119+
PreRunE: func(cmd *cobra.Command, _ []string) error {
120+
force, _ := cmd.Flags().GetBool("force")
121+
fromDir, _ := cmd.Flags().GetString("from-dir")
122+
if force && fromDir == "" {
123+
return fmt.Errorf("--force only applies with --from-dir")
124+
}
125+
return nil
126+
},
127+
RunE: h.Restore,
118128
}
119129
restoreCmd.Flags().Int("cpu", 0, "boot CPUs (0 = keep current)")
120130
restoreCmd.Flags().String("memory", "", "memory size (empty = keep current)")

cmd/vm/run.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ func (h Handler) Restore(cmd *cobra.Command, args []string) error {
182182
}
183183

184184
if snapInfo.NICs != len(vm.NetworkConfigs) {
185-
return fmt.Errorf("nic count mismatch: vm has %d, snapshot has %d",
185+
return fmt.Errorf("NIC count mismatch: vm has %d, snapshot has %d",
186186
len(vm.NetworkConfigs), snapInfo.NICs)
187187
}
188188

@@ -249,7 +249,7 @@ func (h Handler) restoreFromDir(ctx context.Context, cmd *cobra.Command, conf *c
249249
logger.Warnf(ctx, "snapshot envelope id %s does not belong to VM %s; --force in effect", cfg.ID, vmRef)
250250
}
251251
if cfg.NICs != len(vm.NetworkConfigs) {
252-
return fmt.Errorf("nic count mismatch: vm has %d, snapshot has %d",
252+
return fmt.Errorf("NIC count mismatch: vm has %d, snapshot has %d",
253253
len(vm.NetworkConfigs), cfg.NICs)
254254
}
255255
vmCfg, err := cmdcore.RestoreVMConfigFromFlags(cmd, vm, cfg)
@@ -287,18 +287,21 @@ func (h Handler) cloneFromDir(ctx context.Context, cmd *cobra.Command, conf *con
287287
if err != nil {
288288
return fmt.Errorf("load envelope: %w", err)
289289
}
290+
// Local copy so flipping the backend selection doesn't leak to the caller's
291+
// shared *config.Config (CLI is fine, daemons embedding cocoon would notice).
292+
localConf := *conf
290293
if cfg.Hypervisor != "" {
291-
conf.UseFirecracker = cfg.Hypervisor == string(config.HypervisorFirecracker)
294+
localConf.UseFirecracker = cfg.Hypervisor == string(config.HypervisorFirecracker)
292295
}
293-
hyper, err := cmdcore.InitHypervisor(conf)
296+
hyper, err := cmdcore.InitHypervisor(&localConf)
294297
if err != nil {
295298
return err
296299
}
297300
dcr, ok := hyper.(hypervisor.Direct)
298301
if !ok {
299302
return fmt.Errorf("backend %s does not support direct clone", hyper.Type())
300303
}
301-
return h.cloneFromSrcDir(ctx, cmd, conf, dcr, cfg, dir,
304+
return h.cloneFromSrcDir(ctx, cmd, &localConf, dcr, cfg, dir,
302305
fmt.Sprintf("dir %s", dir), logger)
303306
}
304307

snapshot/localfile/export.go

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@ import (
55
"compress/gzip"
66
"context"
77
"encoding/json"
8-
"errors"
98
"fmt"
109
"io"
11-
"io/fs"
1210
"os"
1311
"path/filepath"
1412
"time"
@@ -40,17 +38,16 @@ func (lf *LocalFile) ExportToDir(ctx context.Context, ref, dir string) error {
4038
if err != nil {
4139
return err
4240
}
41+
if err = utils.EnsureDirs(dir); err != nil {
42+
return err
43+
}
4344
// Reject non-empty targets so the export can't silently merge into an
44-
// unrelated tree; mkdir if absent.
45+
// unrelated tree.
4546
dstEntries, err := os.ReadDir(dir)
46-
switch {
47-
case errors.Is(err, fs.ErrNotExist):
48-
if err = os.MkdirAll(dir, 0o750); err != nil {
49-
return fmt.Errorf("create %s: %w", dir, err)
50-
}
51-
case err != nil:
52-
return fmt.Errorf("stat %s: %w", dir, err)
53-
case len(dstEntries) > 0:
47+
if err != nil {
48+
return fmt.Errorf("read %s: %w", dir, err)
49+
}
50+
if len(dstEntries) > 0 {
5451
return fmt.Errorf("target dir %s is not empty", dir)
5552
}
5653
entries, err := os.ReadDir(dataDir)
@@ -62,10 +59,6 @@ func (lf *LocalFile) ExportToDir(ctx context.Context, ref, dir string) error {
6259
continue
6360
}
6461
name := entry.Name()
65-
if name == snapshot.SnapshotJSONName {
66-
// re-exporting a dir that already has an envelope: ours wins
67-
continue
68-
}
6962
src := filepath.Join(dataDir, name)
7063
dst := filepath.Join(dir, name)
7164
if err = utils.ReflinkCopy(dst, src); err != nil {

0 commit comments

Comments
 (0)