From 58c6b87ab0700cb72201bc81bb100ac81feb7e48 Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 25 Aug 2026 10:39:15 +0800 Subject: [PATCH 1/2] review: name the restore integrity check and keep the Firecracker method set contiguous asl functypedup: the integrity callback was spelled in two signatures; asl methodinterleave: launchProcess resumed the Firecracker method set after the cloneLeaseControl block, which only its producers may trail. --- hypervisor/baseconfig.go | 3 +-- hypervisor/firecracker/start.go | 10 +++++----- hypervisor/snapshot.go | 5 ++++- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/hypervisor/baseconfig.go b/hypervisor/baseconfig.go index b2a39345..bdec370b 100644 --- a/hypervisor/baseconfig.go +++ b/hypervisor/baseconfig.go @@ -5,7 +5,6 @@ import ( "time" "github.com/cocoonstack/cocoon/config" - "github.com/cocoonstack/cocoon/types" "github.com/cocoonstack/cocoon/utils" ) @@ -76,7 +75,7 @@ func (c *BaseConfig) LoadAndValidateMeta(dir string) (*SnapshotMeta, error) { } // PreflightRestore runs the shared restore preflight against this backend's managed roots. -func (c *BaseConfig) PreflightRestore(srcDir string, rec *VMRecord, integrity func(srcDir string, sidecar []*types.StorageConfig) error) (*SnapshotMeta, error) { +func (c *BaseConfig) PreflightRestore(srcDir string, rec *VMRecord, integrity IntegrityCheck) (*SnapshotMeta, error) { return PreflightRestore(srcDir, c.RootDir, c.Config.RunDir, rec, integrity) } diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index 036f8564..169b3ece 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -122,6 +122,11 @@ func (fc *Firecracker) configureVM(ctx context.Context, hc *http.Client, rec *hy return nil } +func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, sockPath, netnsPath string, deferQuota bool) (int, error) { + pid, _, err := fc.launchProcessWithLeases(ctx, rec, sockPath, netnsPath, nil, deferQuota) + return pid, err +} + // cloneLeaseControl is the parent's half of the relay lease protocol; a nil control (no leases held) no-ops. type cloneLeaseControl struct { commands *os.File @@ -149,11 +154,6 @@ func (c *cloneLeaseControl) commit() error { return waitRelayLeaseResponse(c.responses, relayLeaseCommitAck, "commit") } -func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMRecord, sockPath, netnsPath string, deferQuota bool) (int, error) { - pid, _, err := fc.launchProcessWithLeases(ctx, rec, sockPath, netnsPath, nil, deferQuota) - return pid, err -} - // launchProcessWithLeases keeps inherited source-VM leases in the console relay until the caller confirms clone setup. func (fc *Firecracker) launchProcessWithLeases(ctx context.Context, rec *hypervisor.VMRecord, sockPath, netnsPath string, leaseFiles []*os.File, deferQuota bool) (int, *cloneLeaseControl, error) { logger := log.WithFunc("firecracker.launchProcessWithLeases") diff --git a/hypervisor/snapshot.go b/hypervisor/snapshot.go index 61f59e60..0f70b995 100644 --- a/hypervisor/snapshot.go +++ b/hypervisor/snapshot.go @@ -23,6 +23,9 @@ type SnapshotMeta struct { BootConfig *types.BootConfig `json:"boot_config,omitempty"` } +// IntegrityCheck verifies the snapshot files under srcDir against the sidecar's storage configs. +type IntegrityCheck func(srcDir string, sidecar []*types.StorageConfig) error + // RecordSnapshot generates a snapshot ID and records it on the VM's record. func (b *Backend) RecordSnapshot(ctx context.Context, vmID string) (string, error) { snapID := utils.GenerateID() @@ -235,7 +238,7 @@ func PopulateFromSrc(runDir, srcDir string, clean func(string) error, clone func } // PreflightRestore loads and validates the sidecar, runs the backend integrity check and asserts the snapshot role sequence prefixes rec; the validated meta is returned so later phases skip re-reading it. -func PreflightRestore(srcDir, rootDir, runDir string, rec *VMRecord, integrity func(srcDir string, sidecar []*types.StorageConfig) error) (*SnapshotMeta, error) { +func PreflightRestore(srcDir, rootDir, runDir string, rec *VMRecord, integrity IntegrityCheck) (*SnapshotMeta, error) { meta, err := LoadAndValidateMeta(srcDir, rootDir, runDir) if err != nil { return nil, err From 17943a237c688a8fa54aa6b498a2d82e35bbb44d Mon Sep 17 00:00:00 2001 From: CMGS Date: Tue, 25 Aug 2026 11:02:39 +0800 Subject: [PATCH 2/2] review: move cloneLeaseControl next to the relay protocol it drives The parent's half of the lease protocol lived inside start.go's Firecracker method set; moving it beside the child's half in relay.go keeps the launch sequence contiguous without reordering it. --- hypervisor/firecracker/relay.go | 27 +++++++++++++++++++++++++++ hypervisor/firecracker/start.go | 27 --------------------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/hypervisor/firecracker/relay.go b/hypervisor/firecracker/relay.go index a4308cbe..cf23af7e 100644 --- a/hypervisor/firecracker/relay.go +++ b/hypervisor/firecracker/relay.go @@ -68,6 +68,33 @@ func (b *broadcaster) readLoop() { } } +// cloneLeaseControl is the parent's half of the relay lease protocol; a nil control (no leases held) no-ops. +type cloneLeaseControl struct { + commands *os.File + responses *os.File +} + +func (c *cloneLeaseControl) close() { + if c == nil { + return + } + closeFile(c.commands) + closeFile(c.responses) + c.commands = nil + c.responses = nil +} + +func (c *cloneLeaseControl) commit() error { + if c == nil { + return nil + } + defer c.close() + if _, err := c.commands.Write([]byte{relayCommitLeasesCommand}); err != nil { + return err + } + return waitRelayLeaseResponse(c.responses, relayLeaseCommitAck, "commit") +} + // IsRelayMode returns true when the process was started as a console relay. func IsRelayMode() bool { return os.Getenv(relayEnvKey) == "1" diff --git a/hypervisor/firecracker/start.go b/hypervisor/firecracker/start.go index 169b3ece..2642d3ee 100644 --- a/hypervisor/firecracker/start.go +++ b/hypervisor/firecracker/start.go @@ -127,33 +127,6 @@ func (fc *Firecracker) launchProcess(ctx context.Context, rec *hypervisor.VMReco return pid, err } -// cloneLeaseControl is the parent's half of the relay lease protocol; a nil control (no leases held) no-ops. -type cloneLeaseControl struct { - commands *os.File - responses *os.File -} - -func (c *cloneLeaseControl) close() { - if c == nil { - return - } - closeFile(c.commands) - closeFile(c.responses) - c.commands = nil - c.responses = nil -} - -func (c *cloneLeaseControl) commit() error { - if c == nil { - return nil - } - defer c.close() - if _, err := c.commands.Write([]byte{relayCommitLeasesCommand}); err != nil { - return err - } - return waitRelayLeaseResponse(c.responses, relayLeaseCommitAck, "commit") -} - // launchProcessWithLeases keeps inherited source-VM leases in the console relay until the caller confirms clone setup. func (fc *Firecracker) launchProcessWithLeases(ctx context.Context, rec *hypervisor.VMRecord, sockPath, netnsPath string, leaseFiles []*os.File, deferQuota bool) (int, *cloneLeaseControl, error) { logger := log.WithFunc("firecracker.launchProcessWithLeases")