Skip to content

Commit c5b0421

Browse files
CMGSCMGS
committed
review: move ensureNotPaused below the method set; tighten new comments
ensureNotPaused is a free function and sat between two CloudHypervisor methods — utilities belong below the method set, in the listWith cluster. Comment pass drops what is duplicated in-file or code-derivable: the package doc and prepareCloneDataDisks lose the device-tree fact restated at the call site, Normalize/DeriveID godoc fold to the fs sibling's single-line form, the DiskAttach note stops naming the function it sits on.
1 parent 41edc35 commit c5b0421

4 files changed

Lines changed: 20 additions & 26 deletions

File tree

‎cmd/vm/run.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,7 @@ func (h Handler) prepareClone(ctx context.Context, cmd *cobra.Command, conf *con
334334
}
335335
nics, _ = cmd.Flags().GetInt("nics")
336336
}
337-
// Fast-fail beside the --nics precedent, before network setup and tar
338-
// extraction; fc's clone-extract guard stays as the library backstop.
337+
// Pre-extract fast-fail; fc's clone-extract guard stays as the library backstop.
339338
if len(vmCfg.DataDisks) > 0 && conf.UseFirecracker {
340339
return nil, "", nil, types.NetSetup{}, fmt.Errorf("--data-disk on clone is Cloud Hypervisor only (Firecracker has no disk hotplug): %w", disk.ErrUnsupportedBackend)
341340
}

‎extend/disk/disk.go‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
// Package disk is the runtime attach interface for extra virtio-blk data
2-
// disks backed by existing raw files. Attach is runtime-only: the disk is
3-
// not part of the VM record, snapshot/hibernate of the VM is refused while
4-
// one is attached (detach first), and detach never deletes the backing file.
1+
// Package disk is the runtime attach interface for extra virtio-blk disks
2+
// backed by existing raw files. Attach is runtime-only — snapshot/hibernate
3+
// is refused while one is attached; detach never deletes the backing file.
54
package disk
65

76
import (
@@ -26,8 +25,7 @@ type Spec struct {
2625
ReadOnly bool
2726
}
2827

29-
// Normalize enforces required fields; the name doubles as the guest serial
30-
// (/dev/disk/by-id/virtio-<name>) and the detach key.
28+
// Normalize enforces required fields; Name doubles as the guest serial (/dev/disk/by-id/virtio-<name>) and the detach key.
3129
func (s *Spec) Normalize() error {
3230
if s.Path == "" {
3331
return fmt.Errorf("path is required")
@@ -63,8 +61,7 @@ type Lister interface {
6361
DiskList(ctx context.Context, vmRef string) ([]Attached, error)
6462
}
6563

66-
// DeriveID returns the deterministic CH device id for a disk name (used by
67-
// attach + detach so concurrent attaches collide on CH's id check).
64+
// DeriveID returns the deterministic CH device id for a disk name (used by attach + detach so concurrent attaches collide on CH's id check).
6865
func DeriveID(name string) string {
6966
return diskIDPrefix + name
7067
}

‎hypervisor/cloudhypervisor/clone.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,9 @@ func (ch *CloudHypervisor) restoreAndResumeClone(
191191
return nil
192192
}
193193

194-
// prepareCloneDataDisks creates the --data-disk files requested for a clone.
195-
// The snapshot's device tree cannot grow at restore, so these are hot-added
196-
// after restore, before resume; a name colliding with an inherited disk's
197-
// serial would overwrite its backing file in the clone run dir.
194+
// prepareCloneDataDisks creates the --data-disk files requested for a clone;
195+
// a name colliding with an inherited disk's serial would overwrite its
196+
// backing file in the clone run dir, hence the pre-create scan.
198197
func (ch *CloudHypervisor) prepareCloneDataDisks(ctx context.Context, vmID string, vmCfg *types.VMConfig, existing []*types.StorageConfig) ([]*types.StorageConfig, error) {
199198
if len(vmCfg.DataDisks) == 0 {
200199
return nil, nil

‎hypervisor/cloudhypervisor/extend.go‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@ func (ch *CloudHypervisor) DiskAttach(ctx context.Context, vmRef string, spec di
4141
if err != nil {
4242
return "", err
4343
}
44-
// Route through storageConfigToDisk: the CH fork refuses disks without an
45-
// explicit image_type, and DirectIO/queue semantics must match create-path
46-
// data disks.
44+
// The CH fork refuses disks without an explicit image_type; DirectIO/queue
45+
// semantics must match create-path data disks.
4746
d := storageConfigToDisk(&types.StorageConfig{
4847
Role: types.StorageRoleData, Path: spec.Path, Serial: spec.Name, RO: spec.ReadOnly,
4948
}, vm.Config.CPU, vm.Config.DiskQueueSize, vm.Config.NoDirectIO)
@@ -265,15 +264,6 @@ func (ch *CloudHypervisor) detachWith(
265264
return nil
266265
}
267266

268-
// ensureNotPaused refuses device-set mutations while a capture window is open
269-
// (snapshot/hibernate/fork): mutating mid-capture would desync config and memory.
270-
func ensureNotPaused(info *chVMInfoResponse) error {
271-
if info.State == chStatePaused {
272-
return fmt.Errorf("vm is paused (snapshot or hibernate in flight); retry after it completes")
273-
}
274-
return nil
275-
}
276-
277267
// runningVMClient asserts the CH process is alive and returns an http.Client on its API socket.
278268
func (ch *CloudHypervisor) runningVMClient(ctx context.Context, vmRef string) (*http.Client, error) {
279269
hc, _, _, err := ch.runningVMClientWithRecord(ctx, vmRef)
@@ -300,6 +290,15 @@ func (ch *CloudHypervisor) runningVMClientWithRecord(ctx context.Context, vmRef
300290
}
301291

302292
// listWith returns nil (not error) for stopped VMs so inspect can omit the field.
293+
// ensureNotPaused refuses device-set mutations while a capture window is open
294+
// (snapshot/hibernate/fork): mutating mid-capture would desync config and memory.
295+
func ensureNotPaused(info *chVMInfoResponse) error {
296+
if info.State == chStatePaused {
297+
return fmt.Errorf("vm is paused (snapshot or hibernate in flight); retry after it completes")
298+
}
299+
return nil
300+
}
301+
303302
func listWith[A any](
304303
ctx context.Context, ch *CloudHypervisor, vmRef string,
305304
extract func(*chVMInfoResponse) []A,

0 commit comments

Comments
 (0)