From e76ebeb2cb79d6df1f1d2f6fdf31743970463230 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Tue, 19 May 2026 21:56:35 +0530 Subject: [PATCH 1/7] [internal/hcs] Migrate package from HCS V1 to V2 Presently, we were using HCS V1 (vmcompute) within internal/hcs but it has been deprecated and therefore, we are moving to use computecore within hcs package. Signed-off-by: Harsh Rawat --- .golangci.yml | 4 - internal/gcs/process.go | 8 +- internal/hcs/callback.go | 163 ------- internal/hcs/migration.go | 225 ++------- internal/hcs/migration_test.go | 152 +++--- internal/hcs/notification.go | 189 ++++++++ internal/hcs/operation.go | 60 +++ internal/hcs/process.go | 256 +++++----- internal/hcs/service.go | 12 +- internal/hcs/system.go | 501 +++++++++---------- internal/hcs/utils.go | 14 +- internal/hcs/waithelper.go | 82 ---- internal/vmcompute/doc.go | 1 - internal/vmcompute/vmcompute.go | 637 ------------------------- internal/vmcompute/zsyscall_windows.go | 607 ----------------------- 15 files changed, 746 insertions(+), 2165 deletions(-) delete mode 100644 internal/hcs/callback.go create mode 100644 internal/hcs/notification.go create mode 100644 internal/hcs/operation.go delete mode 100644 internal/hcs/waithelper.go delete mode 100644 internal/vmcompute/doc.go delete mode 100644 internal/vmcompute/vmcompute.go delete mode 100644 internal/vmcompute/zsyscall_windows.go diff --git a/.golangci.yml b/.golangci.yml index b2bf3047db..71794b3cda 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -124,10 +124,6 @@ linters: - staticcheck path: internal/winapi/ text: "ST1003:" - - linters: - - staticcheck - path: internal/vmcompute/ - text: "ST1003:" - linters: - staticcheck path: internal/regstate/ diff --git a/internal/gcs/process.go b/internal/gcs/process.go index 4c2428a657..c047d3c195 100644 --- a/internal/gcs/process.go +++ b/internal/gcs/process.go @@ -187,7 +187,10 @@ func (p *Process) ExitCode() (_ int, err error) { return -1, errors.New("process not exited") } if err := p.waitCall.Err(); err != nil { - return -1, err + var rerr *rpcError + if !errors.As(err, &rerr) || uint32(rerr.result) != hrNotFound { + return -1, err + } } return int(p.waitResp.ExitCode), nil } @@ -274,7 +277,8 @@ func (p *Process) Stdio() (stdin io.Writer, stdout, stderr io.Reader) { // Wait waits for the process (or guest connection) to terminate. func (p *Process) Wait() error { p.waitCall.Wait() - return p.waitCall.Err() + _, err := p.ExitCode() + return err } func (p *Process) waitBackground() { diff --git a/internal/hcs/callback.go b/internal/hcs/callback.go deleted file mode 100644 index 7b27173c3a..0000000000 --- a/internal/hcs/callback.go +++ /dev/null @@ -1,163 +0,0 @@ -//go:build windows - -package hcs - -import ( - "fmt" - "sync" - "syscall" - - "github.com/Microsoft/hcsshim/internal/interop" - "github.com/Microsoft/hcsshim/internal/logfields" - "github.com/Microsoft/hcsshim/internal/vmcompute" - "github.com/sirupsen/logrus" -) - -var ( - nextCallback uintptr - callbackMap = map[uintptr]*notificationWatcherContext{} - callbackMapLock = sync.RWMutex{} - - notificationWatcherCallback = syscall.NewCallback(notificationWatcher) - - // Notifications for HCS_SYSTEM handles - hcsNotificationSystemExited hcsNotification = 0x00000001 - hcsNotificationSystemCreateCompleted hcsNotification = 0x00000002 - hcsNotificationSystemStartCompleted hcsNotification = 0x00000003 - hcsNotificationSystemPauseCompleted hcsNotification = 0x00000004 - hcsNotificationSystemResumeCompleted hcsNotification = 0x00000005 - hcsNotificationSystemCrashReport hcsNotification = 0x00000006 - hcsNotificationSystemSiloJobCreated hcsNotification = 0x00000007 - hcsNotificationSystemSaveCompleted hcsNotification = 0x00000008 - hcsNotificationSystemRdpEnhancedModeStateChanged hcsNotification = 0x00000009 - hcsNotificationSystemShutdownFailed hcsNotification = 0x0000000A - hcsNotificationSystemGetPropertiesCompleted hcsNotification = 0x0000000B - hcsNotificationSystemModifyCompleted hcsNotification = 0x0000000C - hcsNotificationSystemCrashInitiated hcsNotification = 0x0000000D - hcsNotificationSystemGuestConnectionClosed hcsNotification = 0x0000000E - - // Notifications for HCS_PROCESS handles - hcsNotificationProcessExited hcsNotification = 0x00010000 - - // Common notifications - hcsNotificationInvalid hcsNotification = 0x00000000 - hcsNotificationServiceDisconnect hcsNotification = 0x01000000 -) - -type hcsNotification uint32 - -func (hn hcsNotification) String() string { - switch hn { - case hcsNotificationSystemExited: - return "SystemExited" - case hcsNotificationSystemCreateCompleted: - return "SystemCreateCompleted" - case hcsNotificationSystemStartCompleted: - return "SystemStartCompleted" - case hcsNotificationSystemPauseCompleted: - return "SystemPauseCompleted" - case hcsNotificationSystemResumeCompleted: - return "SystemResumeCompleted" - case hcsNotificationSystemCrashReport: - return "SystemCrashReport" - case hcsNotificationSystemSiloJobCreated: - return "SystemSiloJobCreated" - case hcsNotificationSystemSaveCompleted: - return "SystemSaveCompleted" - case hcsNotificationSystemRdpEnhancedModeStateChanged: - return "SystemRdpEnhancedModeStateChanged" - case hcsNotificationSystemShutdownFailed: - return "SystemShutdownFailed" - case hcsNotificationSystemGetPropertiesCompleted: - return "SystemGetPropertiesCompleted" - case hcsNotificationSystemModifyCompleted: - return "SystemModifyCompleted" - case hcsNotificationSystemCrashInitiated: - return "SystemCrashInitiated" - case hcsNotificationSystemGuestConnectionClosed: - return "SystemGuestConnectionClosed" - case hcsNotificationProcessExited: - return "ProcessExited" - case hcsNotificationInvalid: - return "Invalid" - case hcsNotificationServiceDisconnect: - return "ServiceDisconnect" - default: - return fmt.Sprintf("Unknown: %d", hn) - } -} - -type notificationChannel chan error - -type notificationWatcherContext struct { - channels notificationChannels - handle vmcompute.HcsCallback - - systemID string - processID int -} - -type notificationChannels map[hcsNotification]notificationChannel - -func newSystemChannels() notificationChannels { - channels := make(notificationChannels) - for _, notif := range []hcsNotification{ - hcsNotificationServiceDisconnect, - hcsNotificationSystemExited, - hcsNotificationSystemCreateCompleted, - hcsNotificationSystemStartCompleted, - hcsNotificationSystemPauseCompleted, - hcsNotificationSystemResumeCompleted, - hcsNotificationSystemSaveCompleted, - } { - channels[notif] = make(notificationChannel, 1) - } - return channels -} - -func newProcessChannels() notificationChannels { - channels := make(notificationChannels) - for _, notif := range []hcsNotification{ - hcsNotificationServiceDisconnect, - hcsNotificationProcessExited, - } { - channels[notif] = make(notificationChannel, 1) - } - return channels -} - -func closeChannels(channels notificationChannels) { - for _, c := range channels { - close(c) - } -} - -func notificationWatcher(notificationType hcsNotification, callbackNumber uintptr, notificationStatus uintptr, notificationData *uint16) uintptr { - var result error - if int32(notificationStatus) < 0 { - result = interop.Win32FromHresult(notificationStatus) - } - - callbackMapLock.RLock() - context := callbackMap[callbackNumber] - callbackMapLock.RUnlock() - - if context == nil { - return 0 - } - - log := logrus.WithFields(logrus.Fields{ - "notification-type": notificationType.String(), - "system-id": context.systemID, - }) - if context.processID != 0 { - log.Data[logfields.ProcessID] = context.processID - } - log.Debug("HCS notification") - - if channel, ok := context.channels[notificationType]; ok { - channel <- result - } - - return 0 -} diff --git a/internal/hcs/migration.go b/internal/hcs/migration.go index 5328ba5cca..079cb22e4e 100644 --- a/internal/hcs/migration.go +++ b/internal/hcs/migration.go @@ -7,21 +7,16 @@ import ( "encoding/json" "errors" "syscall" - "unsafe" + "time" "github.com/Microsoft/hcsshim/internal/computecore" "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/oc" - "github.com/sirupsen/logrus" "go.opencensus.io/trace" - "golang.org/x/sys/windows" ) -// migrationNotificationBufferSize is the capacity of the LM notification channel. -const migrationNotificationBufferSize = 16 - // MigrationConfig holds parameters for starting a compute system as a live migration // destination, or for initiating the source side of a live migration. type MigrationConfig struct { @@ -31,124 +26,6 @@ type MigrationConfig struct { SessionID uint32 } -// migrationCallback is the syscall callback registered with HcsSetComputeSystemCallback -// for live migration events. It receives events and dispatches them to the channel -// stored in the System via the callbackContext pointer. -var migrationCallback = syscall.NewCallback(migrationCallbackHandler) - -// migrationCallbackHandler is invoked by computecore.dll for live migration events. -// ctx is &computeSystem.migrationNotifyCh, kept alive across the cgo boundary by -// computeSystem.migrationPinner (unpinned only after HcsCloseComputeSystem has -// drained any in-flight callbacks). The notification channel is never closed. -// Skipping the close keeps tear-down trivially safe and removes the only -// thing that could turn a channel send into a panic. -func migrationCallbackHandler(eventPtr uintptr, ctx uintptr) uintptr { - if eventPtr == 0 || ctx == 0 { - return 0 - } - - e := (*computecore.HcsEvent)(unsafe.Pointer(eventPtr)) - ch := *(*chan hcsschema.OperationSystemMigrationNotificationInfo)(unsafe.Pointer(ctx)) - - eventData := "" - if e.EventData != nil { - eventData = windows.UTF16PtrToString(e.EventData) - } - - logrus.WithFields(logrus.Fields{ - "event-type": e.Type.String(), - "event-data": eventData, - }).Debug("HCS migration notification") - - var info hcsschema.OperationSystemMigrationNotificationInfo - if eventData != "" { - if err := json.Unmarshal([]byte(eventData), &info); err != nil { - logrus.WithFields(logrus.Fields{ - "event-type": e.Type.String(), - "event-data": eventData, - logrus.ErrorKey: err, - }).Warn("failed to unmarshal migration notification payload, dropping event") - return 0 - } - } - - // Non-blocking send to avoid blocking the HCS callback thread. - select { - case ch <- info: - default: - logrus.WithField("event-type", e.Type.String()).Warn("migration notification channel full, dropping event") - } - - return 0 -} - -// openMigrationHandle opens a computecore handle to the same system and -// registers a callback for live migration events. It populates -// computeSystem.migrationHandle and computeSystem.migrationNotifyCh. -// -// The caller MUST hold computeSystem.handleLock. -func (computeSystem *System) openMigrationHandle(ctx context.Context) error { - if computeSystem.migrationHandle != 0 { - // Already open — idempotent. - return nil - } - - // Sanity check: the primary handle must be valid. - if computeSystem.handle == 0 { - return ErrAlreadyClosed - } - - // Open a second handle via computecore for LM operations and events. - handle, err := computecore.HcsOpenComputeSystem(ctx, computeSystem.id, syscall.GENERIC_ALL) - if err != nil { - return err - } - - // Create the notification channel and store it on the struct. - computeSystem.migrationHandle = handle - computeSystem.migrationNotifyCh = make(chan hcsschema.OperationSystemMigrationNotificationInfo, migrationNotificationBufferSize) - - // Pin the address of the notification channel field so it stays visible - // to the GC while HCS holds it as a uintptr callback context. Without - // pinning, this would violate cgo's pointer-passing rules. - computeSystem.migrationPinner.Pin(&computeSystem.migrationNotifyCh) - - // Register the callback. - if err := computecore.HcsSetComputeSystemCallback(ctx, handle, computecore.HcsEventOptionEnableLiveMigrationEvents, uintptr(unsafe.Pointer(&computeSystem.migrationNotifyCh)), migrationCallback); err != nil { - computeSystem.migrationPinner.Unpin() - computeSystem.migrationNotifyCh = nil - computeSystem.migrationHandle = 0 - computecore.HcsCloseComputeSystem(ctx, handle) - return err - } - return nil -} - -// closeMigrationHandle unregisters the LM callback and closes the migration -// handle. -// -// The caller MUST hold computeSystem.handleLock. -func (computeSystem *System) closeMigrationHandle(ctx context.Context) { - if computeSystem.migrationHandle == 0 { - return - } - - // Unregister callback by passing zeros, then close the compute system. - // HcsCloseComputeSystem waits for any in-flight callbacks to return, so - // after it completes no callback can still be reading the pinned - // channel pointer and it is safe to Unpin. - _ = computecore.HcsSetComputeSystemCallback(ctx, computeSystem.migrationHandle, computecore.HcsEventOptionNone, 0, 0) - computecore.HcsCloseComputeSystem(ctx, computeSystem.migrationHandle) - computeSystem.migrationHandle = 0 - - computeSystem.migrationPinner.Unpin() - - // Drop the channel reference. The channel is intentionally not closed: - // consumers signal end-of-stream via the System's context, so a close - // would add no information and would only complicate tear-down. - computeSystem.migrationNotifyCh = nil -} - // StartWithMigrationOptions synchronously starts the compute system as a live // migration destination using the provided configuration. func (computeSystem *System) StartWithMigrationOptions(ctx context.Context, config *MigrationConfig) (err error) { @@ -158,6 +35,11 @@ func (computeSystem *System) StartWithMigrationOptions(ctx context.Context, conf operation := "hcs::System::Start" + ctx, span := oc.StartSpan(ctx, operation) + defer span.End() + defer func() { oc.SetSpanStatus(span, err) }() + span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) + computeSystem.handleLock.Lock() defer computeSystem.handleLock.Unlock() @@ -165,43 +47,30 @@ func (computeSystem *System) StartWithMigrationOptions(ctx context.Context, conf return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - // Open the migration handle for LM events and operations. - if err := computeSystem.openMigrationHandle(ctx); err != nil { - return makeSystemError(computeSystem, operation, err, nil) - } - defer func() { - if err != nil { - computeSystem.closeMigrationHandle(ctx) - } - }() - - // Create a computecore operation to track the start request. - op, err := computecore.HcsCreateOperation(ctx, 0, 0) - if err != nil { - return makeSystemError(computeSystem, operation, err, nil) - } - defer computecore.HcsCloseOperation(ctx, op) - - // Attach the live migration socket to the operation. - if err := computecore.HcsAddResourceToOperation(ctx, op, computecore.HcsResourceTypeSocket, resourcepaths.LiveMigrationSocketURI, config.Socket); err != nil { - return makeSystemError(computeSystem, operation, err, nil) - } - - // Build start options with destination migration settings. - options := hcsschema.StartOptions{ + opts, err := json.Marshal(hcsschema.StartOptions{ DestinationMigrationOptions: &hcsschema.MigrationStartOptions{ NetworkSettings: &hcsschema.MigrationNetworkSettings{SessionID: config.SessionID}, }, - } - raw, err := json.Marshal(options) + }) if err != nil { return makeSystemError(computeSystem, operation, err, nil) } - return computeSystem.startV2(ctx, op, string(raw)) + resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + if err := computecore.HcsAddResourceToOperation(ctx, op, computecore.HcsResourceTypeSocket, resourcepaths.LiveMigrationSocketURI, config.Socket); err != nil { + return err + } + return computecore.HcsStartComputeSystem(ctx, computeSystem.handle, op, string(opts)) + }) + if callErr != nil { + return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + } + computeSystem.startTime = time.Now() + return nil } -// InitializeLiveMigrationOnSource initializes a live migration on the source side with the given options. +// InitializeLiveMigrationOnSource prepares the source compute system for a +// live migration. Must be called on the source before StartLiveMigrationOnSource. func (computeSystem *System) InitializeLiveMigrationOnSource(ctx context.Context, options *hcsschema.MigrationInitializeOptions) (err error) { operation := "hcs::System::InitializeLiveMigrationOnSource" @@ -213,15 +82,9 @@ func (computeSystem *System) InitializeLiveMigrationOnSource(ctx context.Context computeSystem.handleLock.Lock() defer computeSystem.handleLock.Unlock() - // Open the migration handle for LM events and operations. - if err = computeSystem.openMigrationHandle(ctx); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + if computeSystem.handle == 0 { + return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - defer func() { - if err != nil { - computeSystem.closeMigrationHandle(ctx) - } - }() if options == nil { options = &hcsschema.MigrationInitializeOptions{} @@ -238,7 +101,7 @@ func (computeSystem *System) InitializeLiveMigrationOnSource(ctx context.Context defer computecore.HcsCloseOperation(ctx, op) // Issue the initialize call and wait for completion. - if err = computecore.HcsInitializeLiveMigrationOnSource(ctx, computeSystem.migrationHandle, op, string(optionsJSON)); err != nil { + if err = computecore.HcsInitializeLiveMigrationOnSource(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { return makeSystemError(computeSystem, operation, err, nil) } if _, err = computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { @@ -247,8 +110,9 @@ func (computeSystem *System) InitializeLiveMigrationOnSource(ctx context.Context return nil } -// StartLiveMigrationOnSource starts the live migration on the source side using the provided -// transport socket and session ID. +// StartLiveMigrationOnSource begins the source-side migration using the given +// transport socket and session ID. Blocks until HCS accepts the start; +// transfer progress is observed via MigrationNotifications. func (computeSystem *System) StartLiveMigrationOnSource(ctx context.Context, config *MigrationConfig) (err error) { if config == nil { return errors.New("migration config must not be nil") @@ -264,7 +128,7 @@ func (computeSystem *System) StartLiveMigrationOnSource(ctx context.Context, con computeSystem.handleLock.Lock() defer computeSystem.handleLock.Unlock() - if computeSystem.migrationHandle == 0 { + if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } @@ -288,7 +152,7 @@ func (computeSystem *System) StartLiveMigrationOnSource(ctx context.Context, con } // Issue the start call and wait for completion. - if err := computecore.HcsStartLiveMigrationOnSource(ctx, computeSystem.migrationHandle, op, string(optionsJSON)); err != nil { + if err := computecore.HcsStartLiveMigrationOnSource(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { return makeSystemError(computeSystem, operation, err, nil) } if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { @@ -309,7 +173,7 @@ func (computeSystem *System) StartLiveMigrationTransfer(ctx context.Context, opt computeSystem.handleLock.Lock() defer computeSystem.handleLock.Unlock() - if computeSystem.migrationHandle == 0 { + if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } @@ -328,7 +192,7 @@ func (computeSystem *System) StartLiveMigrationTransfer(ctx context.Context, opt defer computecore.HcsCloseOperation(ctx, op) // Begin the memory transfer and wait for completion. - if err := computecore.HcsStartLiveMigrationTransfer(ctx, computeSystem.migrationHandle, op, string(optionsJSON)); err != nil { + if err := computecore.HcsStartLiveMigrationTransfer(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { return makeSystemError(computeSystem, operation, err, nil) } if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { @@ -350,7 +214,7 @@ func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, resume b computeSystem.handleLock.Lock() defer computeSystem.handleLock.Unlock() - if computeSystem.migrationHandle == 0 { + if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } @@ -371,26 +235,23 @@ func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, resume b defer computecore.HcsCloseOperation(ctx, op) // Finalize the migration and wait for completion. - if err := computecore.HcsFinalizeLiveMigration(ctx, computeSystem.migrationHandle, op, string(optionsJSON)); err != nil { + if err := computecore.HcsFinalizeLiveMigration(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { return makeSystemError(computeSystem, operation, err, nil) } if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { return makeSystemError(computeSystem, operation, err, nil) } - - // Migration is complete — release the migration handle and callback. - computeSystem.closeMigrationHandle(ctx) return nil } -// MigrationNotifications returns a read-only channel that receives live migration -// event payloads. Returns an error if no migration handle is open. -func (computeSystem *System) MigrationNotifications() (<-chan hcsschema.OperationSystemMigrationNotificationInfo, error) { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() - - if computeSystem.migrationHandle == 0 { - return nil, errors.New("migration handle not open; call StartWithMigrationOptions or InitializeLiveMigrationOnSource first") - } - return computeSystem.migrationNotifyCh, nil +// MigrationNotifications returns a read-only channel of live migration events +// for this System. The channel exists for the System's lifetime and is safe +// to subscribe to before any migration call, so callers do not miss early +// events such as SetupDone. +// +// The channel is never closed; callers signal end-of-stream via their own +// context. Sends are non-blocking (buffer size migrationNotificationBufferSize) +// and events are dropped on overflow, so consumers must drain promptly. +func (computeSystem *System) MigrationNotifications() <-chan hcsschema.OperationSystemMigrationNotificationInfo { + return computeSystem.migrationNotifyCh } diff --git a/internal/hcs/migration_test.go b/internal/hcs/migration_test.go index b5061e04be..9244d6a3bf 100644 --- a/internal/hcs/migration_test.go +++ b/internal/hcs/migration_test.go @@ -17,19 +17,28 @@ import ( // ───────────────────────────────────────────────────────────────────────────── // Test helpers // -// The handler under test reads its arguments as raw uintptrs that originate -// outside the Go heap (HCS hands them to us via a syscall callback). To -// faithfully exercise that contract — and the cgo pointer-passing rules it -// implies — the helpers below allocate the HcsEvent, the UTF-16 EventData -// buffer, and the channel context out of process heap memory via LocalAlloc. -// All allocations are bound to the test's lifetime through t.Cleanup, so the -// individual tests stay free of teardown bookkeeping. +// notificationHandler has the signature (event, ctx uintptr), matching the +// raw HCS syscall callback. The two arguments are built very differently in +// tests: +// +// 1. event — a pointer to an HcsEvent struct that, in production, lives in +// memory HCS allocated outside the Go heap. To honor the cgo rule that +// such pointers must not refer to Go-managed memory, allocCEvent +// allocates the HcsEvent (and any UTF-16 EventData buffer) with +// LocalAlloc rather than using a Go &HcsEvent{}. +// +// 2. ctx — not a pointer at all, but an opaque integer key into the +// package-level notificationContexts map. Real Go pointers can't be +// handed to HCS across the callback boundary, so each registration +// stores its state (channel, etc.) in that map and gives HCS only the +// ID. Tests use registerSystemCtx to insert an entry pointing at their +// channel and pass the returned ID straight into notificationHandler. // ───────────────────────────────────────────────────────────────────────────── -// allocCEvent returns a uintptr to a LocalAlloc'd HcsEvent. If payload is -// non-empty it is encoded as UTF-16 into a second LocalAlloc'd buffer and -// wired up as EventData; otherwise EventData is left nil. -func allocCEvent(t *testing.T, payload string) uintptr { +// allocCEvent returns a uintptr to a LocalAlloc'd HcsEvent of the given type. +// If payload is non-empty it is encoded as UTF-16 into a second LocalAlloc'd +// buffer and wired up as EventData; otherwise EventData is left nil. +func allocCEvent(t *testing.T, eventType computecore.HcsEventType, payload string) uintptr { t.Helper() evtAddr, err := windows.LocalAlloc(windows.LPTR, uint32(unsafe.Sizeof(computecore.HcsEvent{}))) @@ -39,7 +48,7 @@ func allocCEvent(t *testing.T, payload string) uintptr { t.Cleanup(func() { _, _ = windows.LocalFree(windows.Handle(evtAddr)) }) e := (*computecore.HcsEvent)(unsafe.Pointer(evtAddr)) - e.Type = computecore.HcsEventTypeGroupLiveMigration + e.Type = eventType if payload == "" { return evtAddr @@ -63,19 +72,14 @@ func allocCEvent(t *testing.T, payload string) uintptr { return evtAddr } -// allocCChanCtx stores ch in a LocalAlloc'd buffer and returns its address, -// so the handler reads the chan header out of C memory rather than the Go heap -// (matching how HCS delivers the registered callback context). -func allocCChanCtx(t *testing.T, ch chan hcsschema.OperationSystemMigrationNotificationInfo) uintptr { +// registerSystemCtx registers a fresh system-style notificationContext that +// forwards GroupLiveMigration events to ch and returns its lookup ID as a +// uintptr ready to pass to notificationHandler. Cleanup is registered on t. +func registerSystemCtx(t *testing.T, ch chan hcsschema.OperationSystemMigrationNotificationInfo) uintptr { t.Helper() - addr, err := windows.LocalAlloc(windows.LPTR, uint32(unsafe.Sizeof(ch))) - if err != nil { - t.Fatalf("LocalAlloc(ctx): %v", err) - } - t.Cleanup(func() { _, _ = windows.LocalFree(windows.Handle(addr)) }) - - *(*chan hcsschema.OperationSystemMigrationNotificationInfo)(unsafe.Pointer(addr)) = ch - return addr + id := registerNotificationContext("test-system", 0, nil, ch) + t.Cleanup(func() { unregisterNotificationContext(id) }) + return uintptr(id) } // expectNotification fails the test unless want is the next queued value on ch. @@ -104,26 +108,30 @@ func expectNoNotification(t *testing.T, ch <-chan hcsschema.OperationSystemMigra } // ───────────────────────────────────────────────────────────────────────────── -// Nil-argument guards +// Nil / unknown context guards // ───────────────────────────────────────────────────────────────────────────── -// TestMigrationCallbackHandler_NilArgs verifies that the handler is a no-op -// (returns 0, sends nothing on the channel) when either argument is zero. -func TestMigrationCallbackHandler_NilArgs(t *testing.T) { +// TestNotificationHandler_LM_NilOrUnknownArgs verifies that the handler is a +// no-op (returns 0, sends nothing on the channel) when the event pointer is +// zero or the context ID does not resolve to a registered entry. +func TestNotificationHandler_LM_NilOrUnknownArgs(t *testing.T) { ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) + ctx := registerSystemCtx(t, ch) cases := []struct { name string event, ctx uintptr }{ {"BothZero", 0, 0}, - {"EventZero", 0, allocCChanCtx(t, ch)}, - {"CtxZero", allocCEvent(t, ""), 0}, + {"EventZero", 0, ctx}, + // A non-zero but never-registered ID must miss the lookup + // silently rather than dispatch or panic. + {"UnknownCtx", allocCEvent(t, computecore.HcsEventTypeGroupLiveMigration, `{"Event":"SetupDone"}`), ^uintptr(0)}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - if ret := migrationCallbackHandler(tc.event, tc.ctx); ret != 0 { + if ret := notificationHandler(tc.event, tc.ctx); ret != 0 { t.Fatalf("expected 0, got %d", ret) } }) @@ -135,10 +143,10 @@ func TestMigrationCallbackHandler_NilArgs(t *testing.T) { // Payload decoding // ───────────────────────────────────────────────────────────────────────────── -// TestMigrationCallbackHandler_Payloads verifies that real-world HCS +// TestNotificationHandler_LM_Payloads verifies that real-world HCS // GroupLiveMigration JSON payloads — including a nil EventData pointer — are // decoded and forwarded on the notification channel. -func TestMigrationCallbackHandler_Payloads(t *testing.T) { +func TestNotificationHandler_LM_Payloads(t *testing.T) { cases := []struct { name string payload string @@ -201,10 +209,10 @@ func TestMigrationCallbackHandler_Payloads(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) - evt := allocCEvent(t, tc.payload) - ctx := allocCChanCtx(t, ch) + ctx := registerSystemCtx(t, ch) + evt := allocCEvent(t, computecore.HcsEventTypeGroupLiveMigration, tc.payload) - if ret := migrationCallbackHandler(evt, ctx); ret != 0 { + if ret := notificationHandler(evt, ctx); ret != 0 { t.Fatalf("expected 0, got %d", ret) } expectNotification(t, ch, tc.want) @@ -212,30 +220,31 @@ func TestMigrationCallbackHandler_Payloads(t *testing.T) { } } -// TestMigrationCallbackHandler_InvalidJSONDropped verifies that an -// unparseable EventData payload is logged and dropped without sending. -func TestMigrationCallbackHandler_InvalidJSONDropped(t *testing.T) { +// TestNotificationHandler_LM_InvalidJSONDropped verifies that an unparseable +// EventData payload is logged and dropped without sending. +func TestNotificationHandler_LM_InvalidJSONDropped(t *testing.T) { ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) - evt := allocCEvent(t, "not-json") - ctx := allocCChanCtx(t, ch) + ctx := registerSystemCtx(t, ch) + evt := allocCEvent(t, computecore.HcsEventTypeGroupLiveMigration, "not-json") - if ret := migrationCallbackHandler(evt, ctx); ret != 0 { + if ret := notificationHandler(evt, ctx); ret != 0 { t.Fatalf("expected 0, got %d", ret) } expectNoNotification(t, ch) } -// TestMigrationCallbackHandler_AdditionalDetailsDecodes verifies that the -// raw JSON captured in AdditionalDetails for a BlackoutExited event can be -// decoded by the consumer into the concrete BlackoutExitedEventDetails struct. -// This is the contract that motivates modeling AdditionalDetails as +// TestNotificationHandler_LM_AdditionalDetailsDecodes verifies that the raw +// JSON captured in AdditionalDetails for a BlackoutExited event can be +// decoded by the consumer into the concrete BlackoutExitedEventDetails +// struct. This is the contract that motivates modeling AdditionalDetails as // json.RawMessage rather than a typed *interface{}. -func TestMigrationCallbackHandler_AdditionalDetailsDecodes(t *testing.T) { +func TestNotificationHandler_LM_AdditionalDetailsDecodes(t *testing.T) { ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) - evt := allocCEvent(t, `{"Event":"BlackoutExited","Result":"Success","AdditionalDetails":{"BlackoutDurationMilliseconds":1234,"BlackoutStopTimestamp":"2026-04-23T12:34:56Z"}}`) - ctx := allocCChanCtx(t, ch) + ctx := registerSystemCtx(t, ch) + evt := allocCEvent(t, computecore.HcsEventTypeGroupLiveMigration, + `{"Event":"BlackoutExited","Result":"Success","AdditionalDetails":{"BlackoutDurationMilliseconds":1234,"BlackoutStopTimestamp":"2026-04-23T12:34:56Z"}}`) - if ret := migrationCallbackHandler(evt, ctx); ret != 0 { + if ret := notificationHandler(evt, ctx); ret != 0 { t.Fatalf("expected 0, got %d", ret) } @@ -272,15 +281,15 @@ func TestMigrationCallbackHandler_AdditionalDetailsDecodes(t *testing.T) { } } -// TestMigrationCallbackHandler_AdditionalDetailsAbsent verifies that a +// TestNotificationHandler_LM_AdditionalDetailsAbsent verifies that a // payload without an AdditionalDetails field results in a nil // json.RawMessage on the forwarded notification. -func TestMigrationCallbackHandler_AdditionalDetailsAbsent(t *testing.T) { +func TestNotificationHandler_LM_AdditionalDetailsAbsent(t *testing.T) { ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) - evt := allocCEvent(t, `{"Event":"SetupDone"}`) - ctx := allocCChanCtx(t, ch) + ctx := registerSystemCtx(t, ch) + evt := allocCEvent(t, computecore.HcsEventTypeGroupLiveMigration, `{"Event":"SetupDone"}`) - if ret := migrationCallbackHandler(evt, ctx); ret != 0 { + if ret := notificationHandler(evt, ctx); ret != 0 { t.Fatalf("expected 0, got %d", ret) } @@ -294,20 +303,20 @@ func TestMigrationCallbackHandler_AdditionalDetailsAbsent(t *testing.T) { // Backpressure // ───────────────────────────────────────────────────────────────────────────── -// TestMigrationCallbackHandler_FullChannelDropsEvent verifies that when the +// TestNotificationHandler_LM_FullChannelDropsEvent verifies that when the // notification channel is full the handler drops the new event rather than // blocking the HCS callback thread. -func TestMigrationCallbackHandler_FullChannelDropsEvent(t *testing.T) { +func TestNotificationHandler_LM_FullChannelDropsEvent(t *testing.T) { ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) + ctx := registerSystemCtx(t, ch) // Pre-fill the channel so the next send would block. prefill := hcsschema.OperationSystemMigrationNotificationInfo{Event: hcsschema.MigrationEventSetupDone} ch <- prefill - evt := allocCEvent(t, `{"Event":"MigrationDone"}`) - ctx := allocCChanCtx(t, ch) + evt := allocCEvent(t, computecore.HcsEventTypeGroupLiveMigration, `{"Event":"MigrationDone"}`) - if ret := migrationCallbackHandler(evt, ctx); ret != 0 { + if ret := notificationHandler(evt, ctx); ret != 0 { t.Fatalf("expected 0, got %d", ret) } @@ -317,3 +326,26 @@ func TestMigrationCallbackHandler_FullChannelDropsEvent(t *testing.T) { } expectNoNotification(t, ch) } + +// ───────────────────────────────────────────────────────────────────────────── +// Event-type routing +// ───────────────────────────────────────────────────────────────────────────── + +// TestNotificationHandler_NonLMEvent_NotDispatched verifies that a +// non-GroupLiveMigration event does not land on the migration channel even +// when a channel is registered. This guards the dispatch switch in +// notificationHandler. +func TestNotificationHandler_NonLMEvent_NotDispatched(t *testing.T) { + ch := make(chan hcsschema.OperationSystemMigrationNotificationInfo, 1) + ctx := registerSystemCtx(t, ch) + + // SystemExited is a terminal exit event; without a notificationState + // registered (nil above) it must not panic and must not send anything + // onto the migration channel. + evt := allocCEvent(t, computecore.HcsEventTypeSystemExited, `{"Status":0}`) + + if ret := notificationHandler(evt, ctx); ret != 0 { + t.Fatalf("expected 0, got %d", ret) + } + expectNoNotification(t, ch) +} diff --git a/internal/hcs/notification.go b/internal/hcs/notification.go new file mode 100644 index 0000000000..9de68a0184 --- /dev/null +++ b/internal/hcs/notification.go @@ -0,0 +1,189 @@ +//go:build windows + +package hcs + +import ( + "encoding/json" + "sync" + "sync/atomic" + "syscall" + "unsafe" + + "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" + + "github.com/Microsoft/hcsshim/internal/computecore" + hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" + "github.com/Microsoft/hcsshim/internal/logfields" +) + +// migrationNotificationBufferSize is the capacity of a System's live migration +// notification channel. It only needs enough headroom to absorb a short burst +// of HCS-side events between consumer reads; the dispatch in +// notificationHandler is non-blocking and drops on overflow. +const migrationNotificationBufferSize = 16 + +// HCS V2 callbacks take an opaque void* context. Rather than handing HCS a +// live Go pointer, we register a numeric ID that maps to the real context in +// notificationContexts. +// +// Registrations use HcsEventOptionEnableOperationCallbacks | +// HcsEventOptionEnableLiveMigrationEvents. The package never attaches a +// per-operation callback. +var ( + notificationNextID atomic.Uint64 + notificationContexts sync.Map // uint64 -> *notificationContext + + notificationCallback = syscall.NewCallback(notificationHandler) +) + +// notificationState is the rendezvous between the HCS callback thread and +// waitBackground. waitBackground must NOT poll the HCS handle directly: that +// races with Close and can fault. +// +// HCS does not synthesize a final exit notification when HcsCloseProcess / +// HcsCloseComputeSystem unregisters the callback, so a single "wait +// finished" channel cannot distinguish a real exit from a Close. We expose +// two channels and let waitBackground select on them: +// +// - exited: closed by signalExit on a terminal {System,Process}Exited +// event; raw holds the event's JSON payload. +// - closed: closed by signalClosed from Close; nothing further will +// arrive, so waitBackground must return without publishing a +// synthetic exit (which would surface to consumers as exit_code=255). +type notificationState struct { + exitOnce sync.Once + closeOnce sync.Once + exited chan struct{} + closed chan struct{} + raw string +} + +func newNotificationState() *notificationState { + return ¬ificationState{ + exited: make(chan struct{}), + closed: make(chan struct{}), + } +} + +// signalExit records a real exit event and unblocks waitBackground. Safe to +// call multiple times; only the first wins. +func (s *notificationState) signalExit(raw string) { + s.exitOnce.Do(func() { + s.raw = raw + close(s.exited) + }) +} + +// signalClosed unblocks waitBackground without recording an exit. Call from +// Close when no terminal event was observed. Safe to call multiple times. +func (s *notificationState) signalClosed() { + s.closeOnce.Do(func() { close(s.closed) }) +} + +// notificationContext is the per-handle data resolved from the callback's +// opaque ctx. processID == 0 means the callback belongs to a system handle. +type notificationContext struct { + systemID string + processID int // 0 for system handle callbacks + state *notificationState + migrationCh chan<- hcsschema.OperationSystemMigrationNotificationInfo +} + +// registerNotificationContext returns the ID to pass as the void* context to +// HcsSet{ComputeSystem,Process}Callback. The caller must invoke +// unregisterNotificationContext after the HCS handle is closed (HCS guarantees +// no further callbacks fire past close). +// +// migrationCh may be nil; pass non-nil only for system handles that should +// receive live migration notifications. +func registerNotificationContext(systemID string, processID int, state *notificationState, migrationCh chan<- hcsschema.OperationSystemMigrationNotificationInfo) uint64 { + id := notificationNextID.Add(1) + notificationContexts.Store(id, ¬ificationContext{ + systemID: systemID, + processID: processID, + state: state, + migrationCh: migrationCh, + }) + return id +} + +// unregisterNotificationContext drops the mapping for id. No-op for id == 0. +func unregisterNotificationContext(id uint64) { + if id != 0 { + notificationContexts.Delete(id) + } +} + +// notificationHandler is the single syscall callback shared by all HCS system +// and process registrations. It logs the event, signals the owning +// notificationState on terminal exit events, and dispatches live migration +// events to the registered migration channel. The return value is ignored by +// HCS. +func notificationHandler(eventPtr uintptr, ctx uintptr) uintptr { + if eventPtr == 0 { + return 0 + } + e := (*computecore.HcsEvent)(unsafe.Pointer(eventPtr)) + + fields := logrus.Fields{"event-type": e.Type.String()} + var eventData string + if e.EventData != nil { + eventData = windows.UTF16PtrToString(e.EventData) + fields["event-data"] = eventData + } + + source := "system" + v, ok := notificationContexts.Load(uint64(ctx)) + if ok { + nc := v.(*notificationContext) + if nc.systemID != "" { + fields[logfields.ContainerID] = nc.systemID + } + if nc.processID != 0 { + fields[logfields.ProcessID] = nc.processID + source = "process" + } + switch e.Type { + // Only terminal events count as a real exit; do NOT signal + // closed here. + case computecore.HcsEventTypeSystemExited, computecore.HcsEventTypeProcessExited: + if nc.state != nil { + nc.state.signalExit(eventData) + } + case computecore.HcsEventTypeGroupLiveMigration: + // Forward to the system's migration channel, if one was + // registered. Decoding failures and a full channel are + // both logged-and-dropped: the HCS callback thread must + // never block, and a malformed payload can't be acted on. + if nc.migrationCh != nil { + dispatchMigrationEvent(nc.migrationCh, e.Type, eventData) + } + } + } + + logrus.WithFields(fields).Debugf("HCS %s notification", source) + return 0 +} + +// dispatchMigrationEvent decodes a GroupLiveMigration EventData payload and +// non-blocking-sends it on ch. An empty payload yields the zero value (HCS +// occasionally delivers LM events with a nil EventData pointer). +func dispatchMigrationEvent(ch chan<- hcsschema.OperationSystemMigrationNotificationInfo, eventType computecore.HcsEventType, eventData string) { + var info hcsschema.OperationSystemMigrationNotificationInfo + if eventData != "" { + if err := json.Unmarshal([]byte(eventData), &info); err != nil { + logrus.WithFields(logrus.Fields{ + "event-type": eventType.String(), + "event-data": eventData, + logrus.ErrorKey: err, + }).Warn("failed to unmarshal migration notification payload, dropping event") + return + } + } + select { + case ch <- info: + default: + logrus.WithField("event-type", eventType.String()).Warn("migration notification channel full, dropping event") + } +} diff --git a/internal/hcs/operation.go b/internal/hcs/operation.go new file mode 100644 index 0000000000..f664d2f027 --- /dev/null +++ b/internal/hcs/operation.go @@ -0,0 +1,60 @@ +//go:build windows + +package hcs + +import ( + "context" + + "github.com/Microsoft/hcsshim/internal/computecore" +) + +// infiniteTimeout is the milliseconds value passed to +// HcsWaitForOperationResult to wait forever (Win32 INFINITE, 0xFFFFFFFF). +const infiniteTimeout = ^uint32(0) + +// runOperation creates an HCS operation, invokes fn(op), then synchronously +// waits for the operation result. The returned resultDoc is the JSON document +// produced by the tracked HCS API (which on failure may contain a ResultError +// describing the error events). The operation is always closed before return. +// +// ctx is honored only for tracing/logging values. Cancellation is stripped +// via context.WithoutCancel because abandoning the syscall while HCS still +// owns the operation handle leads to use-after-free crashes +// (EXCEPTION_ACCESS_VIOLATION) inside computecore.dll. Callers must not +// rely on ctx to bound the call's duration. +func runOperation(ctx context.Context, fn func(op computecore.HcsOperation) error) (resultDoc string, err error) { + syscallCtx := context.WithoutCancel(ctx) + + // We do not use any operation level callback. + op, err := computecore.HcsCreateOperation(syscallCtx, 0, 0) + if err != nil { + return "", err + } + defer computecore.HcsCloseOperation(syscallCtx, op) + + if fnErr := fn(op); fnErr != nil { + // Attach any result doc HCS already produced for additional context. + doc, _ := computecore.HcsGetOperationResult(syscallCtx, op) + return doc, fnErr + } + return computecore.HcsWaitForOperationResult(syscallCtx, op, infiniteTimeout) +} + +// runProcessOperation is the equivalent of runOperation for HCS APIs that are +// associated with an HCS_PROCESS handle (HcsCreateProcess, HcsGetProcessInfo, +// etc.) and whose operation result includes the HcsProcessInformation struct. +func runProcessOperation(ctx context.Context, fn func(op computecore.HcsOperation) error) (info computecore.HcsProcessInformation, resultDoc string, err error) { + syscallCtx := context.WithoutCancel(ctx) + + op, err := computecore.HcsCreateOperation(syscallCtx, 0, 0) + if err != nil { + return info, "", err + } + defer computecore.HcsCloseOperation(syscallCtx, op) + + if fnErr := fn(op); fnErr != nil { + _, doc, _ := computecore.HcsGetOperationResultAndProcessInfo(syscallCtx, op) + return info, doc, fnErr + } + return computecore.HcsWaitForOperationResultAndProcessInfo(syscallCtx, op, infiniteTimeout) +} diff --git a/internal/hcs/process.go b/internal/hcs/process.go index fef2bf546c..f705e344e4 100644 --- a/internal/hcs/process.go +++ b/internal/hcs/process.go @@ -14,17 +14,17 @@ import ( "go.opencensus.io/trace" + "github.com/Microsoft/hcsshim/internal/computecore" "github.com/Microsoft/hcsshim/internal/cow" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" - "github.com/Microsoft/hcsshim/internal/vmcompute" ) type Process struct { handleLock sync.RWMutex - handle vmcompute.HcsProcess + handle computecore.HcsProcess processID int system *System hasCachedStdio bool @@ -32,9 +32,16 @@ type Process struct { stdin io.WriteCloser stdout io.ReadCloser stderr io.ReadCloser - callbackNumber uintptr killSignalDelivered bool + // notificationID is the lookup key passed as the void* context to + // HcsSetProcessCallback. Zero when no callback is registered. + notificationID uint64 + // notify rendezvous between the HCS notification callback + // (HcsEventProcessExited) and waitBackground. Close also signals it to + // release waitBackground without publishing an exit. + notify *notificationState + closedWaitOnce sync.Once waitBlock chan struct{} exitCode int @@ -43,12 +50,13 @@ type Process struct { var _ cow.Process = &Process{} -func newProcess(process vmcompute.HcsProcess, processID int, computeSystem *System) *Process { +func newProcess(process computecore.HcsProcess, processID int, computeSystem *System) *Process { return &Process{ handle: process, processID: processID, system: computeSystem, waitBlock: make(chan struct{}), + notify: newNotificationState(), } } @@ -92,8 +100,8 @@ func (process *Process) processSignalResult(ctx context.Context, err error) (boo // // For WCOW `guestresource.SignalProcessOptionsWCOW`. func (process *Process) Signal(ctx context.Context, options interface{}) (bool, error) { - process.handleLock.RLock() - defer process.handleLock.RUnlock() + process.handleLock.Lock() + defer process.handleLock.Unlock() operation := "hcs::Process::Signal" @@ -105,20 +113,22 @@ func (process *Process) Signal(ctx context.Context, options interface{}) (bool, if err != nil { return false, err } + optionsStr := string(optionsb) - resultJSON, err := vmcompute.HcsSignalProcess(ctx, process.handle, string(optionsb)) - events := processHcsResult(ctx, resultJSON) - delivered, err := process.processSignalResult(ctx, err) - if err != nil { - err = makeProcessError(process, operation, err, events) + resultJSON, sigErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsSignalProcess(ctx, process.handle, op, optionsStr) + }) + delivered, sigErr := process.processSignalResult(ctx, sigErr) + if sigErr != nil { + sigErr = makeProcessError(process, operation, sigErr, processHcsResult(ctx, resultJSON)) } - return delivered, err + return delivered, sigErr } // Kill signals the process to terminate but does not wait for it to finish terminating. func (process *Process) Kill(ctx context.Context) (bool, error) { - process.handleLock.RLock() - defer process.handleLock.RUnlock() + process.handleLock.Lock() + defer process.handleLock.Unlock() operation := "hcs::Process::Kill" @@ -171,11 +181,13 @@ func (process *Process) Kill(ctx context.Context) (bool, error) { } defer newProcessHandle.Close() - resultJSON, err := vmcompute.HcsTerminateProcess(ctx, newProcessHandle.handle) - if err != nil { + resultJSON, killErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsTerminateProcess(ctx, newProcessHandle.handle, op, "") + }) + if killErr != nil { // We still need to check these two cases, as processes may still be killed by an // external actor (human operator, OOM, random script etc). - if errors.Is(err, os.ErrPermission) || IsAlreadyStopped(err) { + if errors.Is(killErr, os.ErrPermission) || IsAlreadyStopped(killErr) { // There are two cases where it should be safe to ignore an error returned // by HcsTerminateProcess. The first one is cause by the fact that // HcsTerminateProcess ends up calling TerminateProcess in the context @@ -194,21 +206,27 @@ func (process *Process) Kill(ctx context.Context) (bool, error) { return true, nil } } - events := processHcsResult(ctx, resultJSON) - delivered, err := newProcessHandle.processSignalResult(ctx, err) - if err != nil { - err = makeProcessError(newProcessHandle, operation, err, events) + delivered, killErr := newProcessHandle.processSignalResult(ctx, killErr) + if killErr != nil { + killErr = makeProcessError(newProcessHandle, operation, killErr, processHcsResult(ctx, resultJSON)) } process.killSignalDelivered = delivered - return delivered, err + return delivered, killErr } -// waitBackground waits for the process exit notification. Once received sets -// `process.waitError` (if any) and unblocks all `Wait` calls. +// waitBackground blocks until either the HCS callback delivers +// HcsEventProcessExited (real exit, publish exit code) or Close fires +// (release without publishing). It then sets `process.waitError` (if any) +// and unblocks all `Wait` calls. // -// This MUST be called exactly once per `process.handle` but `Wait` is safe to -// call multiple times. +// HCS does not deliver a final notification on HcsCloseProcess — it just +// unregisters the callback — so Close needs its own signal. Publishing a +// synthetic exit on Close would report exit_code=255 to containerd for +// processes that are still running. +// +// This MUST be called exactly once per `process.handle` but `Wait` is safe +// to call multiple times. func (process *Process) waitBackground() { operation := "hcs::Process::waitBackground" ctx, span := oc.StartSpan(context.Background(), operation) @@ -217,40 +235,24 @@ func (process *Process) waitBackground() { trace.StringAttribute("cid", process.SystemID()), trace.Int64Attribute("pid", int64(process.processID))) - var ( - err error - exitCode = -1 - propertiesJSON string - resultJSON string - ) - - err = waitForNotification(ctx, process.callbackNumber, hcsNotificationProcessExited, nil) - if err != nil { - err = makeProcessError(process, operation, err, nil) - log.G(ctx).WithError(err).Error("failed wait") - } else { - process.handleLock.RLock() - defer process.handleLock.RUnlock() - - // Make sure we didn't race with Close() here - if process.handle != 0 { - propertiesJSON, resultJSON, err = vmcompute.HcsGetProcessProperties(ctx, process.handle) - events := processHcsResult(ctx, resultJSON) - if err != nil { - err = makeProcessError(process, operation, err, events) - } else { - properties := &hcsschema.ProcessStatus{} - err = json.Unmarshal([]byte(propertiesJSON), properties) - if err != nil { - err = makeProcessError(process, operation, err, nil) - } else { - if properties.LastWaitResult != 0 { - log.G(ctx).WithField("wait-result", properties.LastWaitResult).Warning("non-zero last wait result") - } else { - exitCode = int(properties.ExitCode) - } - } - } + select { + case <-process.notify.closed: + log.G(ctx).Debug("process waitBackground returning without exit notification (handle closed)") + return + case <-process.notify.exited: + } + + // Real exit notification path: parse ProcessStatus and publish. + exitCode := -1 + var err error + if raw := process.notify.raw; raw != "" { + properties := &hcsschema.ProcessStatus{} + if uErr := json.Unmarshal([]byte(raw), properties); uErr != nil { + err = makeProcessError(process, operation, uErr, nil) + } else if properties.LastWaitResult != 0 { + log.G(ctx).WithField("wait-result", properties.LastWaitResult).Warning("non-zero last wait result") + } else { + exitCode = int(properties.ExitCode) } } log.G(ctx).WithField("exitCode", exitCode).Debug("process exited") @@ -282,8 +284,8 @@ func (process *Process) stopped() bool { // ResizeConsole resizes the console of the process. func (process *Process) ResizeConsole(ctx context.Context, width, height uint16) error { - process.handleLock.RLock() - defer process.handleLock.RUnlock() + process.handleLock.Lock() + defer process.handleLock.Unlock() operation := "hcs::Process::ResizeConsole" @@ -302,11 +304,13 @@ func (process *Process) ResizeConsole(ctx context.Context, width, height uint16) if err != nil { return err } + modifyRequestStr := string(modifyRequestb) - resultJSON, err := vmcompute.HcsModifyProcess(ctx, process.handle, string(modifyRequestb)) - events := processHcsResult(ctx, resultJSON) - if err != nil { - return makeProcessError(process, operation, err, events) + resultJSON, modErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsModifyProcess(ctx, process.handle, op, modifyRequestStr) + }) + if modErr != nil { + return makeProcessError(process, operation, modErr, processHcsResult(ctx, resultJSON)) } return nil @@ -336,8 +340,8 @@ func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.R trace.StringAttribute("cid", process.SystemID()), trace.Int64Attribute("pid", int64(process.processID))) - process.handleLock.RLock() - defer process.handleLock.RUnlock() + process.handleLock.Lock() + defer process.handleLock.Unlock() if process.handle == 0 { return nil, nil, nil, makeProcessError(process, operation, ErrAlreadyClosed, nil) @@ -352,10 +356,11 @@ func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.R return stdin, stdout, stderr, nil } - processInfo, resultJSON, err := vmcompute.HcsGetProcessInfo(ctx, process.handle) - events := processHcsResult(ctx, resultJSON) + processInfo, resultJSON, err := runProcessOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsGetProcessInfo(ctx, process.handle, op) + }) if err != nil { - return nil, nil, nil, makeProcessError(process, operation, err, events) + return nil, nil, nil, makeProcessError(process, operation, err, processHcsResult(ctx, resultJSON)) } pipes, err := makeOpenFiles([]syscall.Handle{processInfo.StdInput, processInfo.StdOutput, processInfo.StdError}) @@ -385,14 +390,14 @@ func (process *Process) CloseStdin(ctx context.Context) (err error) { trace.StringAttribute("cid", process.SystemID()), trace.Int64Attribute("pid", int64(process.processID))) - process.handleLock.RLock() - defer process.handleLock.RUnlock() + process.handleLock.Lock() + defer process.handleLock.Unlock() if process.handle == 0 { return makeProcessError(process, operation, ErrAlreadyClosed, nil) } - //HcsModifyProcess request to close stdin will fail if the process has already exited + // HcsModifyProcess request to close stdin will fail if the process has already exited if !process.stopped() { modifyRequest := hcsschema.ProcessModifyRequest{ Operation: guestrequest.CloseProcessHandle, @@ -405,11 +410,13 @@ func (process *Process) CloseStdin(ctx context.Context) (err error) { if err != nil { return err } + modifyRequestStr := string(modifyRequestb) - resultJSON, err := vmcompute.HcsModifyProcess(ctx, process.handle, string(modifyRequestb)) - events := processHcsResult(ctx, resultJSON) - if err != nil { - return makeProcessError(process, operation, err, events) + resultJSON, modErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsModifyProcess(ctx, process.handle, op, modifyRequestStr) + }) + if modErr != nil { + return makeProcessError(process, operation, modErr, processHcsResult(ctx, resultJSON)) } } @@ -505,78 +512,37 @@ func (process *Process) Close() (err error) { } process.stdioLock.Unlock() - if err = process.unregisterCallback(ctx); err != nil { - return makeProcessError(process, operation, err, nil) - } + // HcsCloseProcess internally unregisters our notification callback + // and drains in-flight invocations before tearing the handle down. + computecore.HcsCloseProcess(ctx, process.handle) + unregisterNotificationContext(process.notificationID) + process.notificationID = 0 - if err = vmcompute.HcsCloseProcess(ctx, process.handle); err != nil { - return makeProcessError(process, operation, err, nil) - } + // Wake waitBackground so its goroutine can return. We do NOT close + // process.waitBlock: Wait() and ExitCode() treat that channel + // closing as proof the process exited, and would publish exitCode=-1 + // (rendered as 255 by containerd) for a process that may still be + // running. + process.notify.signalClosed() process.handle = 0 - process.closedWaitOnce.Do(func() { - process.exitCode = -1 - process.waitError = ErrAlreadyClosed - close(process.waitBlock) - }) - - return nil -} - -func (process *Process) registerCallback(ctx context.Context) error { - callbackContext := ¬ificationWatcherContext{ - channels: newProcessChannels(), - systemID: process.SystemID(), - processID: process.processID, - } - - callbackMapLock.Lock() - callbackNumber := nextCallback - nextCallback++ - callbackMap[callbackNumber] = callbackContext - callbackMapLock.Unlock() - - callbackHandle, err := vmcompute.HcsRegisterProcessCallback(ctx, process.handle, notificationWatcherCallback, callbackNumber) - if err != nil { - return err - } - callbackContext.handle = callbackHandle - process.callbackNumber = callbackNumber return nil } -func (process *Process) unregisterCallback(ctx context.Context) error { - callbackNumber := process.callbackNumber - - callbackMapLock.RLock() - callbackContext := callbackMap[callbackNumber] - callbackMapLock.RUnlock() - - if callbackContext == nil { - return nil - } - - handle := callbackContext.handle - - if handle == 0 { - return nil - } - - // vmcompute.HcsUnregisterProcessCallback has its own synchronization to - // wait for all callbacks to complete. We must NOT hold the callbackMapLock. - err := vmcompute.HcsUnregisterProcessCallback(ctx, handle) - if err != nil { - return err - } - - closeChannels(callbackContext.channels) - - callbackMapLock.Lock() - delete(callbackMap, callbackNumber) - callbackMapLock.Unlock() - - handle = 0 //nolint:ineffassign - - return nil +// registerNotification registers the package-wide HCS notification callback +// on this process handle. Failures are logged only: missing the diagnostic +// callback must not break process operation. +func (process *Process) registerNotification(ctx context.Context) { + id := registerNotificationContext(process.SystemID(), process.processID, process.notify, nil) + if err := computecore.HcsSetProcessCallback( + ctx, process.handle, + computecore.HcsEventOptionNone, + uintptr(id), notificationCallback, + ); err != nil { + unregisterNotificationContext(id) + log.G(ctx).WithError(err).Warn("failed to register HCS process notification callback") + return + } + process.notificationID = id } diff --git a/internal/hcs/service.go b/internal/hcs/service.go index a46b0051df..371f00460c 100644 --- a/internal/hcs/service.go +++ b/internal/hcs/service.go @@ -6,8 +6,8 @@ import ( "context" "encoding/json" + "github.com/Microsoft/hcsshim/internal/computecore" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" - "github.com/Microsoft/hcsshim/internal/vmcompute" ) // GetServiceProperties returns properties of the host compute service. @@ -18,10 +18,9 @@ func GetServiceProperties(ctx context.Context, q hcsschema.PropertyQuery) (*hcss if err != nil { return nil, err } - propertiesJSON, resultJSON, err := vmcompute.HcsGetServiceProperties(ctx, string(queryb)) - events := processHcsResult(ctx, resultJSON) + propertiesJSON, err := computecore.HcsGetServiceProperties(ctx, string(queryb)) if err != nil { - return nil, &HcsError{Op: operation, Err: err, Events: events} + return nil, &HcsError{Op: operation, Err: err, Events: processHcsResult(ctx, propertiesJSON)} } if propertiesJSON == "" { @@ -42,10 +41,9 @@ func ModifyServiceSettings(ctx context.Context, settings hcsschema.ModificationR if err != nil { return err } - resultJSON, err := vmcompute.HcsModifyServiceSettings(ctx, string(settingsJSON)) - events := processHcsResult(ctx, resultJSON) + resultJSON, err := computecore.HcsModifyServiceSettings(ctx, string(settingsJSON)) if err != nil { - return &HcsError{Op: operation, Err: err, Events: events} + return &HcsError{Op: operation, Err: err, Events: processHcsResult(ctx, resultJSON)} } return nil } diff --git a/internal/hcs/system.go b/internal/hcs/system.go index 869a5f3e7a..300c22abe8 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -7,7 +7,6 @@ import ( "encoding/json" "errors" "fmt" - "runtime" "strings" "sync" "syscall" @@ -21,17 +20,22 @@ import ( "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/logfields" "github.com/Microsoft/hcsshim/internal/oc" - "github.com/Microsoft/hcsshim/internal/timeout" - "github.com/Microsoft/hcsshim/internal/vmcompute" "github.com/sirupsen/logrus" "go.opencensus.io/trace" ) type System struct { - handleLock sync.RWMutex - handle vmcompute.HcsSystem - id string - callbackNumber uintptr + handleLock sync.RWMutex + handle computecore.HcsSystem + id string + + // notificationID is the lookup key passed as the void* context to + // HcsSetComputeSystemCallback. Zero when no callback is registered. + notificationID uint64 + // notify rendezvous between the HCS notification callback + // (HcsEventSystemExited) and waitBackground. Close also signals it to + // release waitBackground without publishing an exit. + notify *notificationState closedWaitOnce sync.Once waitBlock chan struct{} @@ -41,14 +45,10 @@ type System struct { startTime time.Time stopTime time.Time - // Live Migration specific fields. - migrationHandle computecore.HcsSystem + // migrationNotifyCh delivers live migration events from + // notificationHandler. Never closed (callers signal end-of-stream + // via their own context); sends are non-blocking and drop on overflow. migrationNotifyCh chan hcsschema.OperationSystemMigrationNotificationInfo - // migrationPinner pins &migrationNotifyCh while it is registered as the - // callback context with HCS, so the GC sees the cgo-held uintptr as a - // live reference. Unpinned in closeMigrationHandle after HCS guarantees - // no further callbacks will fire. - migrationPinner runtime.Pinner } var _ cow.Container = &System{} @@ -56,8 +56,10 @@ var _ cow.ProcessHost = &System{} func newSystem(id string) *System { return &System{ - id: id, - waitBlock: make(chan struct{}), + id: id, + waitBlock: make(chan struct{}), + notify: newNotificationState(), + migrationNotifyCh: make(chan hcsschema.OperationSystemMigrationNotificationInfo, migrationNotificationBufferSize), } } @@ -70,8 +72,6 @@ func siloNameFmt(containerID string) string { func CreateComputeSystem(ctx context.Context, id string, hcsDocumentInterface interface{}) (_ *System, err error) { operation := "hcs::CreateComputeSystem" - // hcsCreateComputeSystemContext is an async operation. Start the outer span - // here to measure the full create time. ctx, span := oc.StartSpan(ctx, operation) defer span.End() defer func() { oc.SetSpanStatus(span, err) }() @@ -86,36 +86,27 @@ func CreateComputeSystem(ctx context.Context, id string, hcsDocumentInterface in hcsDocument := string(hcsDocumentB) - var ( - identity syscall.Handle - resultJSON string - createError error - ) - computeSystem.handle, resultJSON, createError = vmcompute.HcsCreateComputeSystem(ctx, id, hcsDocument, identity) - if createError == nil || IsPending(createError) { - defer func() { - if err != nil { - computeSystem.Close() - } - }() - if err = computeSystem.registerCallback(ctx); err != nil { - // Terminate the compute system if it still exists. We're okay to - // ignore a failure here. + // On any error after this point, tear down the compute system and + // release the handle. Terminate is guarded (no-op when handle == 0 + // or already stopped) so this is safe on every failure path, + // including a synchronous create failure. + defer func() { + if err != nil { _ = computeSystem.Terminate(ctx) - return nil, makeSystemError(computeSystem, operation, err, nil) + computeSystem.Close() } - } + }() - events, err := processAsyncHcsResult(ctx, createError, resultJSON, computeSystem.callbackNumber, - hcsNotificationSystemCreateCompleted, &timeout.SystemCreate) - if err != nil { - if errors.Is(err, ErrTimeout) { - // Terminate the compute system if it still exists. We're okay to - // ignore a failure here. - _ = computeSystem.Terminate(ctx) - } - return nil, makeSystemError(computeSystem, operation, err, events) + resultJSON, createErr := runOperation(ctx, func(op computecore.HcsOperation) error { + var hErr error + computeSystem.handle, hErr = computecore.HcsCreateComputeSystem(ctx, id, hcsDocument, op, nil) + return hErr + }) + if createErr != nil { + return nil, makeSystemError(computeSystem, operation, createErr, processHcsResult(ctx, resultJSON)) } + + computeSystem.registerNotification(ctx) go computeSystem.waitBackground() if err = computeSystem.getCachedProperties(ctx); err != nil { return nil, err @@ -128,10 +119,9 @@ func OpenComputeSystem(ctx context.Context, id string) (*System, error) { operation := "hcs::OpenComputeSystem" computeSystem := newSystem(id) - handle, resultJSON, err := vmcompute.HcsOpenComputeSystem(ctx, id) - events := processHcsResult(ctx, resultJSON) + handle, err := computecore.HcsOpenComputeSystem(ctx, id, syscall.GENERIC_ALL) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, events) + return nil, makeSystemError(computeSystem, operation, err, nil) } computeSystem.handle = handle defer func() { @@ -139,9 +129,7 @@ func OpenComputeSystem(ctx context.Context, id string) (*System, error) { computeSystem.Close() } }() - if err = computeSystem.registerCallback(ctx); err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) - } + computeSystem.registerNotification(ctx) go computeSystem.waitBackground() if err = computeSystem.getCachedProperties(ctx); err != nil { return nil, err @@ -149,6 +137,25 @@ func OpenComputeSystem(ctx context.Context, id string) (*System, error) { return computeSystem, nil } +// registerNotification registers the package-wide HCS notification callback +// on this system's primary handle. Failures are logged only: the callback +// is purely diagnostic and must not prevent the system from being used. +// Must be called BEFORE waitBackground starts so notifications are not +// missed. +func (computeSystem *System) registerNotification(ctx context.Context) { + id := registerNotificationContext(computeSystem.id, 0, computeSystem.notify, computeSystem.migrationNotifyCh) + if err := computecore.HcsSetComputeSystemCallback( + ctx, computeSystem.handle, + computecore.HcsEventOptionEnableOperationCallbacks|computecore.HcsEventOptionEnableLiveMigrationEvents, + uintptr(id), notificationCallback, + ); err != nil { + unregisterNotificationContext(id) + log.G(ctx).WithError(err).Warn("failed to register HCS system notification callback") + return + } + computeSystem.notificationID = id +} + func (computeSystem *System) getCachedProperties(ctx context.Context) error { props, err := computeSystem.Properties(ctx) if err != nil { @@ -184,84 +191,49 @@ func GetComputeSystems(ctx context.Context, q schema1.ComputeSystemQuery) ([]sch if err != nil { return nil, err } + query := string(queryb) - computeSystemsJSON, resultJSON, err := vmcompute.HcsEnumerateComputeSystems(ctx, string(queryb)) - events := processHcsResult(ctx, resultJSON) + resultJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsEnumerateComputeSystems(ctx, query, op) + }) if err != nil { - return nil, &HcsError{Op: operation, Err: err, Events: events} + return nil, &HcsError{Op: operation, Err: err, Events: processHcsResult(ctx, resultJSON)} } - - if computeSystemsJSON == "" { + if resultJSON == "" { return nil, ErrUnexpectedValue } computeSystems := []schema1.ContainerProperties{} - if err = json.Unmarshal([]byte(computeSystemsJSON), &computeSystems); err != nil { + if err = json.Unmarshal([]byte(resultJSON), &computeSystems); err != nil { return nil, err } return computeSystems, nil } -// Start synchronously starts the computeSystem using HCS V1 API. +// Start synchronously starts the computeSystem. func (computeSystem *System) Start(ctx context.Context) (err error) { operation := "hcs::System::Start" - // hcsStartComputeSystemContext is an async operation. Start the outer span - // here to measure the full start time. ctx, span := oc.StartSpan(ctx, operation) defer span.End() defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() - // prevent starting an exited system because waitblock we do not recreate waitBlock - // or rerun waitBackground, so we have no way to be notified of it closing again + // Prevent starting an exited system: we do not recreate waitBlock or + // rerun waitBackground, so we have no way to be notified of it closing again. if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - resultJSON, err := vmcompute.HcsStartComputeSystem(ctx, computeSystem.handle, "") - events, err := processAsyncHcsResult(ctx, err, resultJSON, computeSystem.callbackNumber, - hcsNotificationSystemStartCompleted, &timeout.SystemStart) - if err != nil { - return makeSystemError(computeSystem, operation, err, events) - } - computeSystem.startTime = time.Now() - return nil -} - -// startV2 is the implementation used by StartWithMigrationOptions to start the compute system -// using HCS V2 APIs. -// The caller provides a pre-created computecore operation (with any resources already -// attached) and the JSON-encoded options string to pass to HcsStartComputeSystem. -// -// The caller MUST hold computeSystem.handleLock and verify the handle is valid -// before calling this method. -func (computeSystem *System) startV2(ctx context.Context, op computecore.HcsOperation, opts string) (err error) { - operation := "hcs::System::Start" - - // hcsStartComputeSystemContext is an async operation. Start the outer span - // here to measure the full start time. - ctx, span := oc.StartSpan(ctx, operation) - defer span.End() - defer func() { oc.SetSpanStatus(span, err) }() - span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - - if err := computecore.HcsStartComputeSystem( - ctx, - computecore.HcsSystem(computeSystem.handle), - op, - opts, - ); err != nil { - return makeSystemError(computeSystem, operation, err, nil) - } - - if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsStartComputeSystem(ctx, computeSystem.handle, op, "") + }) + if callErr != nil { + return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) } - computeSystem.startTime = time.Now() return nil } @@ -273,8 +245,8 @@ func (computeSystem *System) ID() string { // Shutdown requests a compute system shutdown. func (computeSystem *System) Shutdown(ctx context.Context) error { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() operation := "hcs::System::Shutdown" @@ -282,21 +254,22 @@ func (computeSystem *System) Shutdown(ctx context.Context) error { return nil } - resultJSON, err := vmcompute.HcsShutdownComputeSystem(ctx, computeSystem.handle, "") - events := processHcsResult(ctx, resultJSON) + resultJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsShutDownComputeSystem(ctx, computeSystem.handle, op, "") + }) if err != nil && !errors.Is(err, ErrVmcomputeAlreadyStopped) && !errors.Is(err, ErrComputeSystemDoesNotExist) && !errors.Is(err, ErrVmcomputeOperationPending) { - return makeSystemError(computeSystem, operation, err, events) + return makeSystemError(computeSystem, operation, err, processHcsResult(ctx, resultJSON)) } return nil } // Terminate requests a compute system terminate. func (computeSystem *System) Terminate(ctx context.Context) error { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() operation := "hcs::System::Terminate" @@ -304,19 +277,28 @@ func (computeSystem *System) Terminate(ctx context.Context) error { return nil } - resultJSON, err := vmcompute.HcsTerminateComputeSystem(ctx, computeSystem.handle, "") - events := processHcsResult(ctx, resultJSON) + resultJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsTerminateComputeSystem(ctx, computeSystem.handle, op, "") + }) if err != nil && !errors.Is(err, ErrVmcomputeAlreadyStopped) && !errors.Is(err, ErrComputeSystemDoesNotExist) && !errors.Is(err, ErrVmcomputeOperationPending) { - return makeSystemError(computeSystem, operation, err, events) + return makeSystemError(computeSystem, operation, err, processHcsResult(ctx, resultJSON)) } return nil } -// waitBackground waits for the compute system exit notification. Once received -// sets `computeSystem.waitError` (if any) and unblocks all `Wait` calls. +// waitBackground blocks until either the HCS callback delivers +// HcsEventSystemExited (real exit, publish status) or Close fires (release +// without publishing). It then sets `computeSystem.waitError` (if any) and +// unblocks all `Wait` calls. +// +// HCS does not deliver a final notification on HcsCloseComputeSystem — it +// just unregisters the callback — so Close needs its own signal. +// Prematurely closing WaitChannel() causes `hcsExec.waitForContainerExit` +// to Kill the container's running process and report exitCode=-1 +// (rendered as 255). // // This MUST be called exactly once per `computeSystem.handle` but `Wait` is // safe to call multiple times. @@ -326,15 +308,26 @@ func (computeSystem *System) waitBackground() { defer span.End() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - err := waitForNotification(ctx, computeSystem.callbackNumber, hcsNotificationSystemExited, nil) - if err == nil { - log.G(ctx).Debug("system exited") - } else if errors.Is(err, ErrVmcomputeUnexpectedExit) { - log.G(ctx).Debug("unexpected system exit") - computeSystem.exitError = makeSystemError(computeSystem, operation, err, nil) - err = nil - } else { - err = makeSystemError(computeSystem, operation, err, nil) + select { + case <-computeSystem.notify.closed: + log.G(ctx).Debug("system waitBackground returning without exit notification (handle closed)") + return + case <-computeSystem.notify.exited: + } + + // Real exit notification path: parse SystemExitStatus and publish. + var err error + if raw := computeSystem.notify.raw; raw != "" { + var status struct { + Status int32 `json:"Status"` + ExitType string `json:"ExitType"` + } + if uErr := json.Unmarshal([]byte(raw), &status); uErr != nil { + log.G(ctx).WithError(uErr).WithField("exit-data", raw).Warning("failed to parse SystemExitStatus") + } else if status.ExitType == "UnexpectedExit" { + log.G(ctx).Debug("unexpected system exit") + computeSystem.exitError = makeSystemError(computeSystem, operation, ErrVmcomputeUnexpectedExit, nil) + } } computeSystem.closedWaitOnce.Do(func() { computeSystem.waitError = err @@ -393,8 +386,8 @@ func (computeSystem *System) ExitError() error { // Properties returns the requested container properties targeting a V1 schema container. func (computeSystem *System) Properties(ctx context.Context, types ...schema1.PropertyType) (*schema1.ContainerProperties, error) { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() operation := "hcs::System::Properties" @@ -406,11 +399,13 @@ func (computeSystem *System) Properties(ctx context.Context, types ...schema1.Pr if err != nil { return nil, makeSystemError(computeSystem, operation, err, nil) } + query := string(queryBytes) - propertiesJSON, resultJSON, err := vmcompute.HcsGetComputeSystemProperties(ctx, computeSystem.handle, string(queryBytes)) - events := processHcsResult(ctx, resultJSON) + propertiesJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsGetComputeSystemProperties(ctx, computeSystem.handle, op, query) + }) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, events) + return nil, makeSystemError(computeSystem, operation, err, processHcsResult(ctx, propertiesJSON)) } if propertiesJSON == "" { @@ -547,11 +542,13 @@ func (computeSystem *System) hcsPropertiesV2Query(ctx context.Context, types []h if err != nil { return nil, makeSystemError(computeSystem, operation, err, nil) } + query := string(queryBytes) - propertiesJSON, resultJSON, err := vmcompute.HcsGetComputeSystemProperties(ctx, computeSystem.handle, string(queryBytes)) - events := processHcsResult(ctx, resultJSON) + propertiesJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsGetComputeSystemProperties(ctx, computeSystem.handle, op, query) + }) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, events) + return nil, makeSystemError(computeSystem, operation, err, processHcsResult(ctx, propertiesJSON)) } if propertiesJSON == "" { @@ -567,8 +564,8 @@ func (computeSystem *System) hcsPropertiesV2Query(ctx context.Context, types []h // PropertiesV2 returns the requested compute systems properties targeting a V2 schema compute system. func (computeSystem *System) PropertiesV2(ctx context.Context, types ...hcsschema.PropertyType) (_ *hcsschema.Properties, err error) { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() // Let HCS tally up the total for VM based queries instead of querying ourselves. if computeSystem.typ != "container" { @@ -631,8 +628,8 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) @@ -648,11 +645,13 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. if err != nil { return nil, makeSystemError(computeSystem, operation, err, nil) } + queryStr := string(queryBytes) - propertiesJSON, resultJSON, err := vmcompute.HcsGetComputeSystemProperties(ctx, computeSystem.handle, string(queryBytes)) - events := processHcsResult(ctx, resultJSON) + propertiesJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsGetComputeSystemProperties(ctx, computeSystem.handle, op, queryStr) + }) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, events) + return nil, makeSystemError(computeSystem, operation, err, processHcsResult(ctx, propertiesJSON)) } if propertiesJSON == "" { @@ -671,27 +670,24 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. func (computeSystem *System) Pause(ctx context.Context) (err error) { operation := "hcs::System::Pause" - // hcsPauseComputeSystemContext is an async operation. Start the outer span - // here to measure the full pause time. ctx, span := oc.StartSpan(ctx, operation) defer span.End() defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - resultJSON, err := vmcompute.HcsPauseComputeSystem(ctx, computeSystem.handle, "") - events, err := processAsyncHcsResult(ctx, err, resultJSON, computeSystem.callbackNumber, - hcsNotificationSystemPauseCompleted, &timeout.SystemPause) - if err != nil { - return makeSystemError(computeSystem, operation, err, events) + resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsPauseComputeSystem(ctx, computeSystem.handle, op, "") + }) + if callErr != nil { + return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) } - return nil } @@ -699,27 +695,24 @@ func (computeSystem *System) Pause(ctx context.Context) (err error) { func (computeSystem *System) Resume(ctx context.Context) (err error) { operation := "hcs::System::Resume" - // hcsResumeComputeSystemContext is an async operation. Start the outer span - // here to measure the full restore time. ctx, span := oc.StartSpan(ctx, operation) defer span.End() defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - resultJSON, err := vmcompute.HcsResumeComputeSystem(ctx, computeSystem.handle, "") - events, err := processAsyncHcsResult(ctx, err, resultJSON, computeSystem.callbackNumber, - hcsNotificationSystemResumeCompleted, &timeout.SystemResume) - if err != nil { - return makeSystemError(computeSystem, operation, err, events) + resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsResumeComputeSystem(ctx, computeSystem.handle, op, "") + }) + if callErr != nil { + return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) } - return nil } @@ -727,8 +720,6 @@ func (computeSystem *System) Resume(ctx context.Context) (err error) { func (computeSystem *System) Save(ctx context.Context, options interface{}) (err error) { operation := "hcs::System::Save" - // hcsSaveComputeSystemContext is an async operation. Start the outer span - // here to measure the full save time. ctx, span := oc.StartSpan(ctx, operation) defer span.End() defer func() { oc.SetSpanStatus(span, err) }() @@ -738,27 +729,37 @@ func (computeSystem *System) Save(ctx context.Context, options interface{}) (err if err != nil { return err } + saveOptionsStr := string(saveOptions) - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - result, err := vmcompute.HcsSaveComputeSystem(ctx, computeSystem.handle, string(saveOptions)) - events, err := processAsyncHcsResult(ctx, err, result, computeSystem.callbackNumber, - hcsNotificationSystemSaveCompleted, &timeout.SystemSave) - if err != nil { - return makeSystemError(computeSystem, operation, err, events) + resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsSaveComputeSystem(ctx, computeSystem.handle, op, saveOptionsStr) + }) + if callErr != nil { + return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) } - return nil } -func (computeSystem *System) createProcess(ctx context.Context, operation string, c interface{}) (*Process, *vmcompute.HcsProcessInformation, error) { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() +// createProcess launches a process in the compute system and returns its +// handle plus the stdio info HCS produced. +// +// On process-isolated containers (observed on WS2022) HcsCreateProcess can +// complete synchronously and leave the tracking operation in a state where +// the wait spuriously fails (e.g. E_INVALIDARG) even though HCS handed back +// a valid process handle. Recover via HcsGetProcessInfo, but only on wait +// failure — re-fetching after a successful wait races a short-lived process +// exit and surfaces as HCS_E_PROCESS_ALREADY_STOPPED, which would wrongly +// fail the create. +func (computeSystem *System) createProcess(ctx context.Context, operation string, c interface{}) (*Process, *computecore.HcsProcessInformation, error) { + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { return nil, nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) @@ -768,21 +769,42 @@ func (computeSystem *System) createProcess(ctx context.Context, operation string if err != nil { return nil, nil, makeSystemError(computeSystem, operation, err, nil) } - configuration := string(configurationb) - processInfo, processHandle, resultJSON, err := vmcompute.HcsCreateProcess(ctx, computeSystem.handle, configuration) - events := processHcsResult(ctx, resultJSON) - if err != nil { - if v2, ok := c.(*hcsschema.ProcessParameters); ok { - operation += ": " + v2.CommandLine - } else if v1, ok := c.(*schema1.ProcessConfig); ok { - operation += ": " + v1.CommandLine + + // Tag the operation label with the offending command line so error logs + // identify what HCS rejected. + switch v := c.(type) { + case *hcsschema.ProcessParameters: + operation += ": " + v.CommandLine + case *schema1.ProcessConfig: + operation += ": " + v.CommandLine + } + + var processHandle computecore.HcsProcess + processInfo, resultJSON, createErr := runProcessOperation(ctx, func(op computecore.HcsOperation) error { + var hErr error + processHandle, hErr = computecore.HcsCreateProcess(ctx, computeSystem.handle, configuration, op, nil) + return hErr + }) + if createErr != nil { + // No handle means the create itself failed; only a failed wait with + // a live handle is the recoverable sync-completion case. + if processHandle == 0 { + return nil, nil, makeSystemError(computeSystem, operation, createErr, processHcsResult(ctx, resultJSON)) + } + + log.G(ctx).WithError(createErr).Debug("HcsCreateProcess wait failed; falling back to HcsGetProcessInfo") + processInfo, resultJSON, createErr = runProcessOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsGetProcessInfo(ctx, processHandle, op) + }) + if createErr != nil { + computecore.HcsCloseProcess(ctx, processHandle) + return nil, nil, makeSystemError(computeSystem, operation, createErr, processHcsResult(ctx, resultJSON)) } - return nil, nil, makeSystemError(computeSystem, operation, err, events) } - log.G(ctx).WithField("pid", processInfo.ProcessId).Debug("created process pid") - return newProcess(processHandle, int(processInfo.ProcessId), computeSystem), &processInfo, nil + log.G(ctx).WithField("pid", processInfo.ProcessID).Debug("created process pid") + return newProcess(processHandle, int(processInfo.ProcessID), computeSystem), &processInfo, nil } // CreateProcess launches a new process within the computeSystem. @@ -807,9 +829,7 @@ func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) ( process.stderr = pipes[2] process.hasCachedStdio = true - if err = process.registerCallback(ctx); err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) - } + process.registerNotification(ctx) go process.waitBackground() return process, nil @@ -817,8 +837,8 @@ func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) ( // OpenProcess gets an interface to an existing process within the computeSystem. func (computeSystem *System) OpenProcess(ctx context.Context, pid int) (*Process, error) { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() operation := "hcs::System::OpenProcess" @@ -826,16 +846,13 @@ func (computeSystem *System) OpenProcess(ctx context.Context, pid int) (*Process return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) } - processHandle, resultJSON, err := vmcompute.HcsOpenProcess(ctx, computeSystem.handle, uint32(pid)) - events := processHcsResult(ctx, resultJSON) + processHandle, err := computecore.HcsOpenProcess(ctx, computeSystem.handle, uint32(pid), syscall.GENERIC_ALL) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, events) + return nil, makeSystemError(computeSystem, operation, err, nil) } process := newProcess(processHandle, pid, computeSystem) - if err = process.registerCallback(ctx); err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) - } + process.registerNotification(ctx) go process.waitBackground() return process, nil @@ -865,89 +882,28 @@ func (computeSystem *System) CloseCtx(ctx context.Context) (err error) { return nil } - if err = computeSystem.unregisterCallback(ctx); err != nil { - return makeSystemError(computeSystem, operation, err, nil) - } + // HcsCloseComputeSystem internally unregisters our notification + // callback and drains in-flight invocations before tearing the handle + // down. + computecore.HcsCloseComputeSystem(ctx, computeSystem.handle) + unregisterNotificationContext(computeSystem.notificationID) + computeSystem.notificationID = 0 - err = vmcompute.HcsCloseComputeSystem(ctx, computeSystem.handle) - if err != nil { - return makeSystemError(computeSystem, operation, err, nil) - } + // Wake waitBackground so it can return. We do NOT close + // computeSystem.waitBlock: that channel is the public "system + // exited" signal (via WaitChannel/Wait), and closing it for a + // still-running system fakes an exit to callers. + computeSystem.notify.signalClosed() computeSystem.handle = 0 - computeSystem.closedWaitOnce.Do(func() { - computeSystem.waitError = ErrAlreadyClosed - close(computeSystem.waitBlock) - }) - - // Clean up migration handle if it was opened. - computeSystem.closeMigrationHandle(ctx) - - return nil -} - -func (computeSystem *System) registerCallback(ctx context.Context) error { - callbackContext := ¬ificationWatcherContext{ - channels: newSystemChannels(), - systemID: computeSystem.id, - } - - callbackMapLock.Lock() - callbackNumber := nextCallback - nextCallback++ - callbackMap[callbackNumber] = callbackContext - callbackMapLock.Unlock() - - callbackHandle, err := vmcompute.HcsRegisterComputeSystemCallback(ctx, computeSystem.handle, - notificationWatcherCallback, callbackNumber) - if err != nil { - return err - } - callbackContext.handle = callbackHandle - computeSystem.callbackNumber = callbackNumber - - return nil -} - -func (computeSystem *System) unregisterCallback(ctx context.Context) error { - callbackNumber := computeSystem.callbackNumber - - callbackMapLock.RLock() - callbackContext := callbackMap[callbackNumber] - callbackMapLock.RUnlock() - - if callbackContext == nil { - return nil - } - - handle := callbackContext.handle - - if handle == 0 { - return nil - } - - // hcsUnregisterComputeSystemCallback has its own synchronization - // to wait for all callbacks to complete. We must NOT hold the callbackMapLock. - err := vmcompute.HcsUnregisterComputeSystemCallback(ctx, handle) - if err != nil { - return err - } - - closeChannels(callbackContext.channels) - - callbackMapLock.Lock() - delete(callbackMap, callbackNumber) - callbackMapLock.Unlock() - - handle = 0 //nolint:ineffassign return nil } // Modify the System by sending a request to HCS func (computeSystem *System) Modify(ctx context.Context, config interface{}) error { - computeSystem.handleLock.RLock() - defer computeSystem.handleLock.RUnlock() + computeSystem.handleLock.Lock() + defer computeSystem.handleLock.Unlock() operation := "hcs::System::Modify" @@ -961,12 +917,13 @@ func (computeSystem *System) Modify(ctx context.Context, config interface{}) err } requestJSON := string(requestBytes) - resultJSON, err := vmcompute.HcsModifyComputeSystem(ctx, computeSystem.handle, requestJSON) - events := processHcsResult(ctx, resultJSON) - if err != nil { - return makeSystemError(computeSystem, operation, err, events) - } + resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + return computecore.HcsModifyComputeSystem(ctx, computeSystem.handle, op, requestJSON, 0) + }) + if callErr != nil { + return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + } return nil } diff --git a/internal/hcs/utils.go b/internal/hcs/utils.go index 76eb2be7cf..71207f58db 100644 --- a/internal/hcs/utils.go +++ b/internal/hcs/utils.go @@ -14,12 +14,20 @@ import ( "golang.org/x/sys/windows" ) -// makeOpenFiles calls winio.NewOpenFile for each handle in a slice but closes all the handles -// if there is an error. +// makeOpenFiles wraps each handle in input as an overlapped I/O file, returning +// a parallel slice (typically the stdin/stdout/stderr of a compute-system +// process). On any wrap failure, all opened files are closed and the +// remaining unwrapped handles are closed before returning the error. +// +// Handles equal to 0 (NULL) or INVALID_HANDLE_VALUE are treated as "not +// present" and map to a nil entry in the result. HCS APIs such as +// HcsGetProcessInfo use INVALID_HANDLE_VALUE to indicate std streams the +// caller did not request; passing that value to NewOpenFile would +// incorrectly bind the sentinel as if it were a valid pipe handle. func makeOpenFiles(hs []syscall.Handle) (_ []io.ReadWriteCloser, err error) { fs := make([]io.ReadWriteCloser, len(hs)) for i, h := range hs { - if h != syscall.Handle(0) { + if h != syscall.Handle(0) && h != syscall.Handle(windows.InvalidHandle) { if err == nil { fs[i], err = winio.NewOpenFile(windows.Handle(h)) } diff --git a/internal/hcs/waithelper.go b/internal/hcs/waithelper.go deleted file mode 100644 index 3a51ed1955..0000000000 --- a/internal/hcs/waithelper.go +++ /dev/null @@ -1,82 +0,0 @@ -//go:build windows - -package hcs - -import ( - "context" - "time" - - "github.com/Microsoft/hcsshim/internal/log" -) - -func processAsyncHcsResult( - ctx context.Context, - err error, - resultJSON string, - callbackNumber uintptr, - expectedNotification hcsNotification, - timeout *time.Duration, -) ([]ErrorEvent, error) { - events := processHcsResult(ctx, resultJSON) - if IsPending(err) { - return nil, waitForNotification(ctx, callbackNumber, expectedNotification, timeout) - } - - return events, err -} - -func waitForNotification( - ctx context.Context, - callbackNumber uintptr, - expectedNotification hcsNotification, - timeout *time.Duration, -) error { - callbackMapLock.RLock() - if _, ok := callbackMap[callbackNumber]; !ok { - callbackMapLock.RUnlock() - log.G(ctx).WithField("callbackNumber", callbackNumber).Error("failed to waitForNotification: callbackNumber does not exist in callbackMap") - return ErrHandleClose - } - channels := callbackMap[callbackNumber].channels - callbackMapLock.RUnlock() - - expectedChannel := channels[expectedNotification] - if expectedChannel == nil { - log.G(ctx).WithField("type", expectedNotification).Error("unknown notification type in waitForNotification") - return ErrInvalidNotificationType - } - - var c <-chan time.Time - if timeout != nil { - timer := time.NewTimer(*timeout) - c = timer.C - defer timer.Stop() - } - - select { - case err, ok := <-expectedChannel: - if !ok { - return ErrHandleClose - } - return err - case err, ok := <-channels[hcsNotificationSystemExited]: - if !ok { - return ErrHandleClose - } - // If the expected notification is hcsNotificationSystemExited which of the two selects - // chosen is random. Return the raw error if hcsNotificationSystemExited is expected - if channels[hcsNotificationSystemExited] == expectedChannel { - return err - } - return ErrUnexpectedContainerExit - case _, ok := <-channels[hcsNotificationServiceDisconnect]: - if !ok { - return ErrHandleClose - } - // hcsNotificationServiceDisconnect should never be an expected notification - // it does not need the same handling as hcsNotificationSystemExited - return ErrUnexpectedProcessAbort - case <-c: - return ErrTimeout - } -} diff --git a/internal/vmcompute/doc.go b/internal/vmcompute/doc.go deleted file mode 100644 index 9dd00c8128..0000000000 --- a/internal/vmcompute/doc.go +++ /dev/null @@ -1 +0,0 @@ -package vmcompute diff --git a/internal/vmcompute/vmcompute.go b/internal/vmcompute/vmcompute.go deleted file mode 100644 index 5819dc6df3..0000000000 --- a/internal/vmcompute/vmcompute.go +++ /dev/null @@ -1,637 +0,0 @@ -//go:build windows - -package vmcompute - -import ( - gcontext "context" - "syscall" - "time" - - "github.com/sirupsen/logrus" - "go.opencensus.io/trace" - - "github.com/Microsoft/hcsshim/internal/interop" - "github.com/Microsoft/hcsshim/internal/log" - "github.com/Microsoft/hcsshim/internal/logfields" - "github.com/Microsoft/hcsshim/internal/oc" - "github.com/Microsoft/hcsshim/internal/timeout" -) - -//go:generate go tool github.com/Microsoft/go-winio/tools/mkwinsyscall -output zsyscall_windows.go vmcompute.go - -//sys hcsEnumerateComputeSystems(query string, computeSystems **uint16, result **uint16) (hr error) = vmcompute.HcsEnumerateComputeSystems? -//sys hcsCreateComputeSystem(id string, configuration string, identity syscall.Handle, computeSystem *HcsSystem, result **uint16) (hr error) = vmcompute.HcsCreateComputeSystem? -//sys hcsOpenComputeSystem(id string, computeSystem *HcsSystem, result **uint16) (hr error) = vmcompute.HcsOpenComputeSystem? -//sys hcsCloseComputeSystem(computeSystem HcsSystem) (hr error) = vmcompute.HcsCloseComputeSystem? -//sys hcsStartComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsStartComputeSystem? -//sys hcsShutdownComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsShutdownComputeSystem? -//sys hcsTerminateComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsTerminateComputeSystem? -//sys hcsPauseComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsPauseComputeSystem? -//sys hcsResumeComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsResumeComputeSystem? -//sys hcsGetComputeSystemProperties(computeSystem HcsSystem, propertyQuery string, properties **uint16, result **uint16) (hr error) = vmcompute.HcsGetComputeSystemProperties? -//sys hcsModifyComputeSystem(computeSystem HcsSystem, configuration string, result **uint16) (hr error) = vmcompute.HcsModifyComputeSystem? -//sys hcsModifyServiceSettings(settings string, result **uint16) (hr error) = vmcompute.HcsModifyServiceSettings? -//sys hcsRegisterComputeSystemCallback(computeSystem HcsSystem, callback uintptr, context uintptr, callbackHandle *HcsCallback) (hr error) = vmcompute.HcsRegisterComputeSystemCallback? -//sys hcsUnregisterComputeSystemCallback(callbackHandle HcsCallback) (hr error) = vmcompute.HcsUnregisterComputeSystemCallback? -//sys hcsSaveComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) = vmcompute.HcsSaveComputeSystem? - -//sys hcsCreateProcess(computeSystem HcsSystem, processParameters string, processInformation *HcsProcessInformation, process *HcsProcess, result **uint16) (hr error) = vmcompute.HcsCreateProcess? -//sys hcsOpenProcess(computeSystem HcsSystem, pid uint32, process *HcsProcess, result **uint16) (hr error) = vmcompute.HcsOpenProcess? -//sys hcsCloseProcess(process HcsProcess) (hr error) = vmcompute.HcsCloseProcess? -//sys hcsTerminateProcess(process HcsProcess, result **uint16) (hr error) = vmcompute.HcsTerminateProcess? -//sys hcsSignalProcess(process HcsProcess, options string, result **uint16) (hr error) = vmcompute.HcsSignalProcess? -//sys hcsGetProcessInfo(process HcsProcess, processInformation *HcsProcessInformation, result **uint16) (hr error) = vmcompute.HcsGetProcessInfo? -//sys hcsGetProcessProperties(process HcsProcess, processProperties **uint16, result **uint16) (hr error) = vmcompute.HcsGetProcessProperties? -//sys hcsModifyProcess(process HcsProcess, settings string, result **uint16) (hr error) = vmcompute.HcsModifyProcess? -//sys hcsGetServiceProperties(propertyQuery string, properties **uint16, result **uint16) (hr error) = vmcompute.HcsGetServiceProperties? -//sys hcsRegisterProcessCallback(process HcsProcess, callback uintptr, context uintptr, callbackHandle *HcsCallback) (hr error) = vmcompute.HcsRegisterProcessCallback? -//sys hcsUnregisterProcessCallback(callbackHandle HcsCallback) (hr error) = vmcompute.HcsUnregisterProcessCallback? - -// errVmcomputeOperationPending is an error encountered when the operation is being completed asynchronously -const errVmcomputeOperationPending = syscall.Errno(0xC0370103) - -// HcsSystem is the handle associated with a created compute system. -type HcsSystem syscall.Handle - -// HcsProcess is the handle associated with a created process in a compute -// system. -type HcsProcess syscall.Handle - -// HcsCallback is the handle associated with the function to call when events -// occur. -type HcsCallback syscall.Handle - -// HcsProcessInformation is the structure used when creating or getting process -// info. -type HcsProcessInformation struct { - // ProcessId is the pid of the created process. - ProcessId uint32 - _ uint32 // reserved padding - // StdInput is the handle associated with the stdin of the process. - StdInput syscall.Handle - // StdOutput is the handle associated with the stdout of the process. - StdOutput syscall.Handle - // StdError is the handle associated with the stderr of the process. - StdError syscall.Handle -} - -func execute(ctx gcontext.Context, timeout time.Duration, f func() error) error { - now := time.Now() - if timeout > 0 { - var cancel gcontext.CancelFunc - ctx, cancel = gcontext.WithTimeout(ctx, timeout) - defer cancel() - } - - // if ctx already has prior deadlines, the shortest timeout takes precedence and is used. - // find the true timeout for reporting - // - // this is mostly an issue with (*UtilityVM).Start(context.Context), which sets its - // own (2 minute) timeout. - deadline, ok := ctx.Deadline() - trueTimeout := timeout - if ok { - trueTimeout = deadline.Sub(now) - log.G(ctx).WithFields(logrus.Fields{ - logfields.Timeout: trueTimeout, - "desiredTimeout": timeout, - }).Trace("Executing syscall with deadline") - } - - done := make(chan error, 1) - go func() { - done <- f() - }() - select { - case <-ctx.Done(): - if ctx.Err() == gcontext.DeadlineExceeded { - log.G(ctx).WithField(logfields.Timeout, trueTimeout). - Warning("Syscall did not complete within operation timeout. This may indicate a platform issue. " + - "If it appears to be making no forward progress, obtain the stacks and see if there is a syscall " + - "stuck in the platform API for a significant length of time.") - } - return ctx.Err() - case err := <-done: - return err - } -} - -func HcsEnumerateComputeSystems(ctx gcontext.Context, query string) (computeSystems, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsEnumerateComputeSystems") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("query", query)) - - return computeSystems, result, execute(ctx, timeout.SyscallWatcher, func() error { - var ( - computeSystemsp *uint16 - resultp *uint16 - ) - err := hcsEnumerateComputeSystems(query, &computeSystemsp, &resultp) - if computeSystemsp != nil { - computeSystems = interop.ConvertAndFreeCoTaskMemString(computeSystemsp) - } - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsCreateComputeSystem(ctx gcontext.Context, id string, configuration string, identity syscall.Handle) (computeSystem HcsSystem, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsCreateComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - span.AddAttributes( - trace.StringAttribute("id", id), - trace.StringAttribute("configuration", configuration)) - - return computeSystem, result, execute(ctx, timeout.SystemCreate, func() error { - var resultp *uint16 - err := hcsCreateComputeSystem(id, configuration, identity, &computeSystem, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsOpenComputeSystem(ctx gcontext.Context, id string) (computeSystem HcsSystem, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsOpenComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - - return computeSystem, result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsOpenComputeSystem(id, &computeSystem, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsCloseComputeSystem(ctx gcontext.Context, computeSystem HcsSystem) (hr error) { - ctx, span := oc.StartSpan(ctx, "HcsCloseComputeSystem") - defer span.End() - defer func() { oc.SetSpanStatus(span, hr) }() - - return execute(ctx, timeout.SyscallWatcher, func() error { - return hcsCloseComputeSystem(computeSystem) - }) -} - -func HcsStartComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsStartComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - span.AddAttributes(trace.StringAttribute("options", options)) - - return result, execute(ctx, timeout.SystemStart, func() error { - var resultp *uint16 - err := hcsStartComputeSystem(computeSystem, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsShutdownComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsShutdownComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - span.AddAttributes(trace.StringAttribute("options", options)) - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsShutdownComputeSystem(computeSystem, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsTerminateComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsTerminateComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - span.AddAttributes(trace.StringAttribute("options", options)) - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsTerminateComputeSystem(computeSystem, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsPauseComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsPauseComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - span.AddAttributes(trace.StringAttribute("options", options)) - - return result, execute(ctx, timeout.SystemPause, func() error { - var resultp *uint16 - err := hcsPauseComputeSystem(computeSystem, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsResumeComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsResumeComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - span.AddAttributes(trace.StringAttribute("options", options)) - - return result, execute(ctx, timeout.SystemResume, func() error { - var resultp *uint16 - err := hcsResumeComputeSystem(computeSystem, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsGetComputeSystemProperties(ctx gcontext.Context, computeSystem HcsSystem, propertyQuery string) (properties, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsGetComputeSystemProperties") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("propertyQuery", propertyQuery)) - - return properties, result, execute(ctx, timeout.SyscallWatcher, func() error { - var ( - propertiesp *uint16 - resultp *uint16 - ) - err := hcsGetComputeSystemProperties(computeSystem, propertyQuery, &propertiesp, &resultp) - if propertiesp != nil { - properties = interop.ConvertAndFreeCoTaskMemString(propertiesp) - } - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsModifyComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, configuration string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsModifyComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("configuration", configuration)) - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsModifyComputeSystem(computeSystem, configuration, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsModifyServiceSettings(ctx gcontext.Context, settings string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsModifyServiceSettings") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("settings", settings)) - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsModifyServiceSettings(settings, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsRegisterComputeSystemCallback(ctx gcontext.Context, computeSystem HcsSystem, callback uintptr, context uintptr) (callbackHandle HcsCallback, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsRegisterComputeSystemCallback") - defer span.End() - defer func() { oc.SetSpanStatus(span, hr) }() - - return callbackHandle, execute(ctx, timeout.SyscallWatcher, func() error { - return hcsRegisterComputeSystemCallback(computeSystem, callback, context, &callbackHandle) - }) -} - -func HcsUnregisterComputeSystemCallback(ctx gcontext.Context, callbackHandle HcsCallback) (hr error) { - ctx, span := oc.StartSpan(ctx, "HcsUnregisterComputeSystemCallback") - defer span.End() - defer func() { oc.SetSpanStatus(span, hr) }() - - return execute(ctx, timeout.SyscallWatcher, func() error { - return hcsUnregisterComputeSystemCallback(callbackHandle) - }) -} - -func HcsCreateProcess(ctx gcontext.Context, computeSystem HcsSystem, processParameters string) (processInformation HcsProcessInformation, process HcsProcess, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsCreateProcess") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - if span.IsRecordingEvents() { - // wont handle v1 process parameters - if s, err := log.ScrubProcessParameters(processParameters); err == nil { - span.AddAttributes(trace.StringAttribute("processParameters", s)) - } - } - - return processInformation, process, result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsCreateProcess(computeSystem, processParameters, &processInformation, &process, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsOpenProcess(ctx gcontext.Context, computeSystem HcsSystem, pid uint32) (process HcsProcess, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsOpenProcess") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.Int64Attribute("pid", int64(pid))) - - return process, result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsOpenProcess(computeSystem, pid, &process, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsCloseProcess(ctx gcontext.Context, process HcsProcess) (hr error) { - ctx, span := oc.StartSpan(ctx, "HcsCloseProcess") - defer span.End() - defer func() { oc.SetSpanStatus(span, hr) }() - - return execute(ctx, timeout.SyscallWatcher, func() error { - return hcsCloseProcess(process) - }) -} - -func HcsTerminateProcess(ctx gcontext.Context, process HcsProcess) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsTerminateProcess") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsTerminateProcess(process, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsSignalProcess(ctx gcontext.Context, process HcsProcess, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsSignalProcess") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("options", options)) - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsSignalProcess(process, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsGetProcessInfo(ctx gcontext.Context, process HcsProcess) (processInformation HcsProcessInformation, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsGetProcessInfo") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - - return processInformation, result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsGetProcessInfo(process, &processInformation, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsGetProcessProperties(ctx gcontext.Context, process HcsProcess) (processProperties, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsGetProcessProperties") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - - return processProperties, result, execute(ctx, timeout.SyscallWatcher, func() error { - var ( - processPropertiesp *uint16 - resultp *uint16 - ) - err := hcsGetProcessProperties(process, &processPropertiesp, &resultp) - if processPropertiesp != nil { - processProperties = interop.ConvertAndFreeCoTaskMemString(processPropertiesp) - } - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsModifyProcess(ctx gcontext.Context, process HcsProcess, settings string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsModifyProcess") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("settings", settings)) - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsModifyProcess(process, settings, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsGetServiceProperties(ctx gcontext.Context, propertyQuery string) (properties, result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsGetServiceProperties") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - oc.SetSpanStatus(span, hr) - }() - span.AddAttributes(trace.StringAttribute("propertyQuery", propertyQuery)) - - return properties, result, execute(ctx, timeout.SyscallWatcher, func() error { - var ( - propertiesp *uint16 - resultp *uint16 - ) - err := hcsGetServiceProperties(propertyQuery, &propertiesp, &resultp) - if propertiesp != nil { - properties = interop.ConvertAndFreeCoTaskMemString(propertiesp) - } - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} - -func HcsRegisterProcessCallback(ctx gcontext.Context, process HcsProcess, callback uintptr, context uintptr) (callbackHandle HcsCallback, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsRegisterProcessCallback") - defer span.End() - defer func() { oc.SetSpanStatus(span, hr) }() - - return callbackHandle, execute(ctx, timeout.SyscallWatcher, func() error { - return hcsRegisterProcessCallback(process, callback, context, &callbackHandle) - }) -} - -func HcsUnregisterProcessCallback(ctx gcontext.Context, callbackHandle HcsCallback) (hr error) { - ctx, span := oc.StartSpan(ctx, "HcsUnregisterProcessCallback") - defer span.End() - defer func() { oc.SetSpanStatus(span, hr) }() - - return execute(ctx, timeout.SyscallWatcher, func() error { - return hcsUnregisterProcessCallback(callbackHandle) - }) -} - -func HcsSaveComputeSystem(ctx gcontext.Context, computeSystem HcsSystem, options string) (result string, hr error) { - ctx, span := oc.StartSpan(ctx, "HcsSaveComputeSystem") - defer span.End() - defer func() { - if result != "" { - span.AddAttributes(trace.StringAttribute("result", result)) - } - if hr != errVmcomputeOperationPending { //nolint:errorlint // explicitly returned - oc.SetSpanStatus(span, hr) - } - }() - - return result, execute(ctx, timeout.SyscallWatcher, func() error { - var resultp *uint16 - err := hcsSaveComputeSystem(computeSystem, options, &resultp) - if resultp != nil { - result = interop.ConvertAndFreeCoTaskMemString(resultp) - } - return err - }) -} diff --git a/internal/vmcompute/zsyscall_windows.go b/internal/vmcompute/zsyscall_windows.go deleted file mode 100644 index 67779de50b..0000000000 --- a/internal/vmcompute/zsyscall_windows.go +++ /dev/null @@ -1,607 +0,0 @@ -//go:build windows - -// Code generated by 'go generate' using "github.com/Microsoft/go-winio/tools/mkwinsyscall"; DO NOT EDIT. - -package vmcompute - -import ( - "syscall" - "unsafe" - - "golang.org/x/sys/windows" -) - -var _ unsafe.Pointer - -// Do the interface allocations only once for common -// Errno values. -const ( - errnoERROR_IO_PENDING = 997 -) - -var ( - errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING) - errERROR_EINVAL error = syscall.EINVAL -) - -// errnoErr returns common boxed Errno values, to prevent -// allocations at runtime. -func errnoErr(e syscall.Errno) error { - switch e { - case 0: - return errERROR_EINVAL - case errnoERROR_IO_PENDING: - return errERROR_IO_PENDING - } - return e -} - -var ( - modvmcompute = windows.NewLazySystemDLL("vmcompute.dll") - - procHcsCloseComputeSystem = modvmcompute.NewProc("HcsCloseComputeSystem") - procHcsCloseProcess = modvmcompute.NewProc("HcsCloseProcess") - procHcsCreateComputeSystem = modvmcompute.NewProc("HcsCreateComputeSystem") - procHcsCreateProcess = modvmcompute.NewProc("HcsCreateProcess") - procHcsEnumerateComputeSystems = modvmcompute.NewProc("HcsEnumerateComputeSystems") - procHcsGetComputeSystemProperties = modvmcompute.NewProc("HcsGetComputeSystemProperties") - procHcsGetProcessInfo = modvmcompute.NewProc("HcsGetProcessInfo") - procHcsGetProcessProperties = modvmcompute.NewProc("HcsGetProcessProperties") - procHcsGetServiceProperties = modvmcompute.NewProc("HcsGetServiceProperties") - procHcsModifyComputeSystem = modvmcompute.NewProc("HcsModifyComputeSystem") - procHcsModifyProcess = modvmcompute.NewProc("HcsModifyProcess") - procHcsModifyServiceSettings = modvmcompute.NewProc("HcsModifyServiceSettings") - procHcsOpenComputeSystem = modvmcompute.NewProc("HcsOpenComputeSystem") - procHcsOpenProcess = modvmcompute.NewProc("HcsOpenProcess") - procHcsPauseComputeSystem = modvmcompute.NewProc("HcsPauseComputeSystem") - procHcsRegisterComputeSystemCallback = modvmcompute.NewProc("HcsRegisterComputeSystemCallback") - procHcsRegisterProcessCallback = modvmcompute.NewProc("HcsRegisterProcessCallback") - procHcsResumeComputeSystem = modvmcompute.NewProc("HcsResumeComputeSystem") - procHcsSaveComputeSystem = modvmcompute.NewProc("HcsSaveComputeSystem") - procHcsShutdownComputeSystem = modvmcompute.NewProc("HcsShutdownComputeSystem") - procHcsSignalProcess = modvmcompute.NewProc("HcsSignalProcess") - procHcsStartComputeSystem = modvmcompute.NewProc("HcsStartComputeSystem") - procHcsTerminateComputeSystem = modvmcompute.NewProc("HcsTerminateComputeSystem") - procHcsTerminateProcess = modvmcompute.NewProc("HcsTerminateProcess") - procHcsUnregisterComputeSystemCallback = modvmcompute.NewProc("HcsUnregisterComputeSystemCallback") - procHcsUnregisterProcessCallback = modvmcompute.NewProc("HcsUnregisterProcessCallback") -) - -func hcsCloseComputeSystem(computeSystem HcsSystem) (hr error) { - hr = procHcsCloseComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsCloseComputeSystem.Addr(), uintptr(computeSystem)) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsCloseProcess(process HcsProcess) (hr error) { - hr = procHcsCloseProcess.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsCloseProcess.Addr(), uintptr(process)) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsCreateComputeSystem(id string, configuration string, identity syscall.Handle, computeSystem *HcsSystem, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(id) - if hr != nil { - return - } - var _p1 *uint16 - _p1, hr = syscall.UTF16PtrFromString(configuration) - if hr != nil { - return - } - return _hcsCreateComputeSystem(_p0, _p1, identity, computeSystem, result) -} - -func _hcsCreateComputeSystem(id *uint16, configuration *uint16, identity syscall.Handle, computeSystem *HcsSystem, result **uint16) (hr error) { - hr = procHcsCreateComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsCreateComputeSystem.Addr(), uintptr(unsafe.Pointer(id)), uintptr(unsafe.Pointer(configuration)), uintptr(identity), uintptr(unsafe.Pointer(computeSystem)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsCreateProcess(computeSystem HcsSystem, processParameters string, processInformation *HcsProcessInformation, process *HcsProcess, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(processParameters) - if hr != nil { - return - } - return _hcsCreateProcess(computeSystem, _p0, processInformation, process, result) -} - -func _hcsCreateProcess(computeSystem HcsSystem, processParameters *uint16, processInformation *HcsProcessInformation, process *HcsProcess, result **uint16) (hr error) { - hr = procHcsCreateProcess.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsCreateProcess.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(processParameters)), uintptr(unsafe.Pointer(processInformation)), uintptr(unsafe.Pointer(process)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsEnumerateComputeSystems(query string, computeSystems **uint16, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(query) - if hr != nil { - return - } - return _hcsEnumerateComputeSystems(_p0, computeSystems, result) -} - -func _hcsEnumerateComputeSystems(query *uint16, computeSystems **uint16, result **uint16) (hr error) { - hr = procHcsEnumerateComputeSystems.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsEnumerateComputeSystems.Addr(), uintptr(unsafe.Pointer(query)), uintptr(unsafe.Pointer(computeSystems)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsGetComputeSystemProperties(computeSystem HcsSystem, propertyQuery string, properties **uint16, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(propertyQuery) - if hr != nil { - return - } - return _hcsGetComputeSystemProperties(computeSystem, _p0, properties, result) -} - -func _hcsGetComputeSystemProperties(computeSystem HcsSystem, propertyQuery *uint16, properties **uint16, result **uint16) (hr error) { - hr = procHcsGetComputeSystemProperties.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsGetComputeSystemProperties.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(propertyQuery)), uintptr(unsafe.Pointer(properties)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsGetProcessInfo(process HcsProcess, processInformation *HcsProcessInformation, result **uint16) (hr error) { - hr = procHcsGetProcessInfo.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsGetProcessInfo.Addr(), uintptr(process), uintptr(unsafe.Pointer(processInformation)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsGetProcessProperties(process HcsProcess, processProperties **uint16, result **uint16) (hr error) { - hr = procHcsGetProcessProperties.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsGetProcessProperties.Addr(), uintptr(process), uintptr(unsafe.Pointer(processProperties)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsGetServiceProperties(propertyQuery string, properties **uint16, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(propertyQuery) - if hr != nil { - return - } - return _hcsGetServiceProperties(_p0, properties, result) -} - -func _hcsGetServiceProperties(propertyQuery *uint16, properties **uint16, result **uint16) (hr error) { - hr = procHcsGetServiceProperties.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsGetServiceProperties.Addr(), uintptr(unsafe.Pointer(propertyQuery)), uintptr(unsafe.Pointer(properties)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsModifyComputeSystem(computeSystem HcsSystem, configuration string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(configuration) - if hr != nil { - return - } - return _hcsModifyComputeSystem(computeSystem, _p0, result) -} - -func _hcsModifyComputeSystem(computeSystem HcsSystem, configuration *uint16, result **uint16) (hr error) { - hr = procHcsModifyComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsModifyComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(configuration)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsModifyProcess(process HcsProcess, settings string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(settings) - if hr != nil { - return - } - return _hcsModifyProcess(process, _p0, result) -} - -func _hcsModifyProcess(process HcsProcess, settings *uint16, result **uint16) (hr error) { - hr = procHcsModifyProcess.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsModifyProcess.Addr(), uintptr(process), uintptr(unsafe.Pointer(settings)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsModifyServiceSettings(settings string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(settings) - if hr != nil { - return - } - return _hcsModifyServiceSettings(_p0, result) -} - -func _hcsModifyServiceSettings(settings *uint16, result **uint16) (hr error) { - hr = procHcsModifyServiceSettings.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsModifyServiceSettings.Addr(), uintptr(unsafe.Pointer(settings)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsOpenComputeSystem(id string, computeSystem *HcsSystem, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(id) - if hr != nil { - return - } - return _hcsOpenComputeSystem(_p0, computeSystem, result) -} - -func _hcsOpenComputeSystem(id *uint16, computeSystem *HcsSystem, result **uint16) (hr error) { - hr = procHcsOpenComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsOpenComputeSystem.Addr(), uintptr(unsafe.Pointer(id)), uintptr(unsafe.Pointer(computeSystem)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsOpenProcess(computeSystem HcsSystem, pid uint32, process *HcsProcess, result **uint16) (hr error) { - hr = procHcsOpenProcess.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsOpenProcess.Addr(), uintptr(computeSystem), uintptr(pid), uintptr(unsafe.Pointer(process)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsPauseComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsPauseComputeSystem(computeSystem, _p0, result) -} - -func _hcsPauseComputeSystem(computeSystem HcsSystem, options *uint16, result **uint16) (hr error) { - hr = procHcsPauseComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsPauseComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsRegisterComputeSystemCallback(computeSystem HcsSystem, callback uintptr, context uintptr, callbackHandle *HcsCallback) (hr error) { - hr = procHcsRegisterComputeSystemCallback.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsRegisterComputeSystemCallback.Addr(), uintptr(computeSystem), uintptr(callback), uintptr(context), uintptr(unsafe.Pointer(callbackHandle))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsRegisterProcessCallback(process HcsProcess, callback uintptr, context uintptr, callbackHandle *HcsCallback) (hr error) { - hr = procHcsRegisterProcessCallback.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsRegisterProcessCallback.Addr(), uintptr(process), uintptr(callback), uintptr(context), uintptr(unsafe.Pointer(callbackHandle))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsResumeComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsResumeComputeSystem(computeSystem, _p0, result) -} - -func _hcsResumeComputeSystem(computeSystem HcsSystem, options *uint16, result **uint16) (hr error) { - hr = procHcsResumeComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsResumeComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsSaveComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsSaveComputeSystem(computeSystem, _p0, result) -} - -func _hcsSaveComputeSystem(computeSystem HcsSystem, options *uint16, result **uint16) (hr error) { - hr = procHcsSaveComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsSaveComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsShutdownComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsShutdownComputeSystem(computeSystem, _p0, result) -} - -func _hcsShutdownComputeSystem(computeSystem HcsSystem, options *uint16, result **uint16) (hr error) { - hr = procHcsShutdownComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsShutdownComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsSignalProcess(process HcsProcess, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsSignalProcess(process, _p0, result) -} - -func _hcsSignalProcess(process HcsProcess, options *uint16, result **uint16) (hr error) { - hr = procHcsSignalProcess.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsSignalProcess.Addr(), uintptr(process), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsStartComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsStartComputeSystem(computeSystem, _p0, result) -} - -func _hcsStartComputeSystem(computeSystem HcsSystem, options *uint16, result **uint16) (hr error) { - hr = procHcsStartComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsStartComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsTerminateComputeSystem(computeSystem HcsSystem, options string, result **uint16) (hr error) { - var _p0 *uint16 - _p0, hr = syscall.UTF16PtrFromString(options) - if hr != nil { - return - } - return _hcsTerminateComputeSystem(computeSystem, _p0, result) -} - -func _hcsTerminateComputeSystem(computeSystem HcsSystem, options *uint16, result **uint16) (hr error) { - hr = procHcsTerminateComputeSystem.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsTerminateComputeSystem.Addr(), uintptr(computeSystem), uintptr(unsafe.Pointer(options)), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsTerminateProcess(process HcsProcess, result **uint16) (hr error) { - hr = procHcsTerminateProcess.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsTerminateProcess.Addr(), uintptr(process), uintptr(unsafe.Pointer(result))) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsUnregisterComputeSystemCallback(callbackHandle HcsCallback) (hr error) { - hr = procHcsUnregisterComputeSystemCallback.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsUnregisterComputeSystemCallback.Addr(), uintptr(callbackHandle)) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} - -func hcsUnregisterProcessCallback(callbackHandle HcsCallback) (hr error) { - hr = procHcsUnregisterProcessCallback.Find() - if hr != nil { - return - } - r0, _, _ := syscall.SyscallN(procHcsUnregisterProcessCallback.Addr(), uintptr(callbackHandle)) - if int32(r0) < 0 { - if r0&0x1fff0000 == 0x00070000 { - r0 &= 0xffff - } - hr = syscall.Errno(r0) - } - return -} From 5e7dd9c50233bce0e367483b2672813cb0ff997f Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 23 May 2026 21:16:09 +0530 Subject: [PATCH 2/7] address review comments Signed-off-by: Harsh Rawat --- internal/hcs/errors.go | 63 ++++++++++---- internal/hcs/migration.go | 50 +++++------ internal/hcs/notification.go | 21 +++-- internal/hcs/operation.go | 64 ++++++++++++-- internal/hcs/process.go | 62 +++++++------- internal/hcs/service.go | 6 +- internal/hcs/system.go | 157 +++++++++++++++++++---------------- 7 files changed, 257 insertions(+), 166 deletions(-) diff --git a/internal/hcs/errors.go b/internal/hcs/errors.go index 3e10f5c7e0..61b3f62a5d 100644 --- a/internal/hcs/errors.go +++ b/internal/hcs/errors.go @@ -99,15 +99,36 @@ type ErrorEvent struct { EventID uint16 `json:"EventId,omitempty"` Flags uint32 `json:"Flags,omitempty"` Source string `json:"Source,omitempty"` - //Data []EventData `json:"Data,omitempty"` // Omit this as HCS doesn't encode this well. It's more confusing to include. It is however logged in debug mode (see processHcsResult function) + //Data []EventData `json:"Data,omitempty"` // Omit this as HCS doesn't encode this well. It's more confusing to include. } +// hcsResult mirrors the HCS [ResultError] document and implements [error] +// so callers can recover it via [errors.As]. The unexported cause keeps +// [errors.Is] working against the underlying syscall errno. +// +// [ResultError]: https://learn.microsoft.com/en-us/virtualization/api/hcs/schemareference#ResultError type hcsResult struct { - Error int32 - ErrorMessage string + // ErrorCode mirrors the "Error" JSON field; renamed to avoid colliding + // with the [hcsResult.Error] method. + ErrorCode int32 `json:"Error,omitempty"` + ErrorMessage string `json:"ErrorMessage,omitempty"` ErrorEvents []ErrorEvent `json:"ErrorEvents,omitempty"` + + cause error } +func (r *hcsResult) Error() string { + if r.cause != nil { + return r.cause.Error() + } + if r.ErrorMessage != "" { + return r.ErrorMessage + } + return fmt.Sprintf("hcs result: 0x%08x", uint32(r.ErrorCode)) +} + +func (r *hcsResult) Unwrap() error { return r.cause } + func (ev *ErrorEvent) String() string { evs := "[Event Detail: " + ev.Message if ev.StackTrace != "" { @@ -129,14 +150,26 @@ func (ev *ErrorEvent) String() string { return evs } -func processHcsResult(ctx context.Context, resultJSON string) []ErrorEvent { - if resultJSON != "" { - result := &hcsResult{} - if err := json.Unmarshal([]byte(resultJSON), result); err != nil { - log.G(ctx).WithError(err).Warning("Could not unmarshal HCS result") - return nil - } - return result.ErrorEvents +// wrapHcsResult attaches the parsed HCS ResultError document to err so +// callers can recover it via `errors.As(err, new(*hcsResult))`. Returns +// err unchanged when the document is empty or unparseable. +func wrapHcsResult(ctx context.Context, err error, resultJSON string) error { + if err == nil || resultJSON == "" { + return err + } + r := &hcsResult{cause: err} + if jerr := json.Unmarshal([]byte(resultJSON), r); jerr != nil { + log.G(ctx).WithError(jerr).Warning("Could not unmarshal HCS result") + return err + } + return r +} + +// eventsFromError returns the ErrorEvents from any wrapped HCS result in err. +func eventsFromError(err error) []ErrorEvent { + var r *hcsResult + if errors.As(err, &r) { + return r.ErrorEvents } return nil } @@ -201,7 +234,7 @@ func (e *SystemError) Error() string { return s } -func makeSystemError(system *System, op string, err error, events []ErrorEvent) error { +func makeSystemError(system *System, op string, err error) error { // Don't double wrap errors var e *SystemError if errors.As(err, &e) { @@ -213,7 +246,7 @@ func makeSystemError(system *System, op string, err error, events []ErrorEvent) HcsError: HcsError{ Op: op, Err: err, - Events: events, + Events: eventsFromError(err), }, } } @@ -235,7 +268,7 @@ func (e *ProcessError) Error() string { return s } -func makeProcessError(process *Process, op string, err error, events []ErrorEvent) error { +func makeProcessError(process *Process, op string, err error) error { // Don't double wrap errors var e *ProcessError if errors.As(err, &e) { @@ -247,7 +280,7 @@ func makeProcessError(process *Process, op string, err error, events []ErrorEven HcsError: HcsError{ Op: op, Err: err, - Events: events, + Events: eventsFromError(err), }, } } diff --git a/internal/hcs/migration.go b/internal/hcs/migration.go index 079cb22e4e..89ebdc8e7b 100644 --- a/internal/hcs/migration.go +++ b/internal/hcs/migration.go @@ -44,7 +44,7 @@ func (computeSystem *System) StartWithMigrationOptions(ctx context.Context, conf defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } opts, err := json.Marshal(hcsschema.StartOptions{ @@ -53,17 +53,17 @@ func (computeSystem *System) StartWithMigrationOptions(ctx context.Context, conf }, }) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } - resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + _, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { if err := computecore.HcsAddResourceToOperation(ctx, op, computecore.HcsResourceTypeSocket, resourcepaths.LiveMigrationSocketURI, config.Socket); err != nil { return err } return computecore.HcsStartComputeSystem(ctx, computeSystem.handle, op, string(opts)) }) if callErr != nil { - return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, callErr) } computeSystem.startTime = time.Now() return nil @@ -83,7 +83,7 @@ func (computeSystem *System) InitializeLiveMigrationOnSource(ctx context.Context defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } if options == nil { @@ -91,21 +91,21 @@ func (computeSystem *System) InitializeLiveMigrationOnSource(ctx context.Context } optionsJSON, err := json.Marshal(options) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } op, err := computecore.HcsCreateOperation(ctx, 0, 0) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } defer computecore.HcsCloseOperation(ctx, op) // Issue the initialize call and wait for completion. if err = computecore.HcsInitializeLiveMigrationOnSource(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } if _, err = computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } return nil } @@ -129,18 +129,18 @@ func (computeSystem *System) StartLiveMigrationOnSource(ctx context.Context, con defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } op, err := computecore.HcsCreateOperation(ctx, 0, 0) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } defer computecore.HcsCloseOperation(ctx, op) // Attach the migration socket to the operation before starting. if err := computecore.HcsAddResourceToOperation(ctx, op, computecore.HcsResourceTypeSocket, resourcepaths.LiveMigrationSocketURI, config.Socket); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } options := hcsschema.MigrationStartOptions{ @@ -148,15 +148,15 @@ func (computeSystem *System) StartLiveMigrationOnSource(ctx context.Context, con } optionsJSON, err := json.Marshal(options) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } // Issue the start call and wait for completion. if err := computecore.HcsStartLiveMigrationOnSource(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } return nil } @@ -174,7 +174,7 @@ func (computeSystem *System) StartLiveMigrationTransfer(ctx context.Context, opt defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } if options == nil { @@ -182,21 +182,21 @@ func (computeSystem *System) StartLiveMigrationTransfer(ctx context.Context, opt } optionsJSON, err := json.Marshal(options) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } op, err := computecore.HcsCreateOperation(ctx, 0, 0) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } defer computecore.HcsCloseOperation(ctx, op) // Begin the memory transfer and wait for completion. if err := computecore.HcsStartLiveMigrationTransfer(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } return nil } @@ -215,7 +215,7 @@ func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, resume b defer computeSystem.handleLock.Unlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } // Choose whether to resume or stop the VM after migration. @@ -225,21 +225,21 @@ func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, resume b } optionsJSON, err := json.Marshal(hcsschema.MigrationFinalizedOptions{FinalizedOperation: finalOp}) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } op, err := computecore.HcsCreateOperation(ctx, 0, 0) if err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } defer computecore.HcsCloseOperation(ctx, op) // Finalize the migration and wait for completion. if err := computecore.HcsFinalizeLiveMigration(ctx, computeSystem.handle, op, string(optionsJSON)); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } if _, err := computecore.HcsWaitForOperationResult(ctx, op, 0xFFFFFFFF); err != nil { - return makeSystemError(computeSystem, operation, err, nil) + return makeSystemError(computeSystem, operation, err) } return nil } diff --git a/internal/hcs/notification.go b/internal/hcs/notification.go index 9de68a0184..fbe8726a0e 100644 --- a/internal/hcs/notification.go +++ b/internal/hcs/notification.go @@ -27,9 +27,8 @@ const migrationNotificationBufferSize = 16 // live Go pointer, we register a numeric ID that maps to the real context in // notificationContexts. // -// Registrations use HcsEventOptionEnableOperationCallbacks | -// HcsEventOptionEnableLiveMigrationEvents. The package never attaches a -// per-operation callback. +// Registrations use HcsEventOptionEnableLiveMigrationEvents. +// The package never attaches a per-operation callback. var ( notificationNextID atomic.Uint64 notificationContexts sync.Map // uint64 -> *notificationContext @@ -56,7 +55,7 @@ type notificationState struct { closeOnce sync.Once exited chan struct{} closed chan struct{} - raw string + raw json.RawMessage } func newNotificationState() *notificationState { @@ -68,7 +67,7 @@ func newNotificationState() *notificationState { // signalExit records a real exit event and unblocks waitBackground. Safe to // call multiple times; only the first wins. -func (s *notificationState) signalExit(raw string) { +func (s *notificationState) signalExit(raw json.RawMessage) { s.exitOnce.Do(func() { s.raw = raw close(s.exited) @@ -149,7 +148,7 @@ func notificationHandler(eventPtr uintptr, ctx uintptr) uintptr { // closed here. case computecore.HcsEventTypeSystemExited, computecore.HcsEventTypeProcessExited: if nc.state != nil { - nc.state.signalExit(eventData) + nc.state.signalExit(json.RawMessage(eventData)) } case computecore.HcsEventTypeGroupLiveMigration: // Forward to the system's migration channel, if one was @@ -157,7 +156,7 @@ func notificationHandler(eventPtr uintptr, ctx uintptr) uintptr { // both logged-and-dropped: the HCS callback thread must // never block, and a malformed payload can't be acted on. if nc.migrationCh != nil { - dispatchMigrationEvent(nc.migrationCh, e.Type, eventData) + dispatchMigrationEvent(nc.migrationCh, e.Type, json.RawMessage(eventData)) } } } @@ -169,13 +168,13 @@ func notificationHandler(eventPtr uintptr, ctx uintptr) uintptr { // dispatchMigrationEvent decodes a GroupLiveMigration EventData payload and // non-blocking-sends it on ch. An empty payload yields the zero value (HCS // occasionally delivers LM events with a nil EventData pointer). -func dispatchMigrationEvent(ch chan<- hcsschema.OperationSystemMigrationNotificationInfo, eventType computecore.HcsEventType, eventData string) { +func dispatchMigrationEvent(ch chan<- hcsschema.OperationSystemMigrationNotificationInfo, eventType computecore.HcsEventType, eventData json.RawMessage) { var info hcsschema.OperationSystemMigrationNotificationInfo - if eventData != "" { - if err := json.Unmarshal([]byte(eventData), &info); err != nil { + if len(eventData) > 0 { + if err := json.Unmarshal(eventData, &info); err != nil { logrus.WithFields(logrus.Fields{ "event-type": eventType.String(), - "event-data": eventData, + "event-data": string(eventData), logrus.ErrorKey: err, }).Warn("failed to unmarshal migration notification payload, dropping event") return diff --git a/internal/hcs/operation.go b/internal/hcs/operation.go index f664d2f027..a1a61415b7 100644 --- a/internal/hcs/operation.go +++ b/internal/hcs/operation.go @@ -4,6 +4,7 @@ package hcs import ( "context" + "time" "github.com/Microsoft/hcsshim/internal/computecore" ) @@ -12,17 +13,36 @@ import ( // HcsWaitForOperationResult to wait forever (Win32 INFINITE, 0xFFFFFFFF). const infiniteTimeout = ^uint32(0) +// waitTimeoutMs derives the TimeoutMs argument for HcsWaitForOperationResult +// from ctx's deadline. With no deadline (or one already past) it falls back +// to INFINITE / a minimal poll respectively. +func waitTimeoutMs(ctx context.Context) uint32 { + dl, ok := ctx.Deadline() + if !ok { + return infiniteTimeout + } + d := time.Until(dl) + if d <= 0 { + return 0 + } + ms := d.Milliseconds() + if ms >= int64(infiniteTimeout) { + return infiniteTimeout - 1 + } + return uint32(ms) +} + // runOperation creates an HCS operation, invokes fn(op), then synchronously // waits for the operation result. The returned resultDoc is the JSON document // produced by the tracked HCS API (which on failure may contain a ResultError // describing the error events). The operation is always closed before return. // -// ctx is honored only for tracing/logging values. Cancellation is stripped -// via context.WithoutCancel because abandoning the syscall while HCS still -// owns the operation handle leads to use-after-free crashes -// (EXCEPTION_ACCESS_VIOLATION) inside computecore.dll. Callers must not -// rely on ctx to bound the call's duration. +// The wait is bounded by ctx's deadline (if any); otherwise it waits forever. +// ctx cancellation is otherwise stripped via context.WithoutCancel because +// abandoning the syscall while HCS still owns the operation handle leads to +// use-after-free crashes (EXCEPTION_ACCESS_VIOLATION) inside computecore.dll. func runOperation(ctx context.Context, fn func(op computecore.HcsOperation) error) (resultDoc string, err error) { + timeoutMs := waitTimeoutMs(ctx) syscallCtx := context.WithoutCancel(ctx) // We do not use any operation level callback. @@ -35,15 +55,17 @@ func runOperation(ctx context.Context, fn func(op computecore.HcsOperation) erro if fnErr := fn(op); fnErr != nil { // Attach any result doc HCS already produced for additional context. doc, _ := computecore.HcsGetOperationResult(syscallCtx, op) - return doc, fnErr + return doc, wrapHcsResult(ctx, fnErr, doc) } - return computecore.HcsWaitForOperationResult(syscallCtx, op, infiniteTimeout) + doc, waitErr := computecore.HcsWaitForOperationResult(syscallCtx, op, timeoutMs) + return doc, wrapHcsResult(ctx, waitErr, doc) } // runProcessOperation is the equivalent of runOperation for HCS APIs that are // associated with an HCS_PROCESS handle (HcsCreateProcess, HcsGetProcessInfo, // etc.) and whose operation result includes the HcsProcessInformation struct. func runProcessOperation(ctx context.Context, fn func(op computecore.HcsOperation) error) (info computecore.HcsProcessInformation, resultDoc string, err error) { + timeoutMs := waitTimeoutMs(ctx) syscallCtx := context.WithoutCancel(ctx) op, err := computecore.HcsCreateOperation(syscallCtx, 0, 0) @@ -54,7 +76,31 @@ func runProcessOperation(ctx context.Context, fn func(op computecore.HcsOperatio if fnErr := fn(op); fnErr != nil { _, doc, _ := computecore.HcsGetOperationResultAndProcessInfo(syscallCtx, op) - return info, doc, fnErr + return info, doc, wrapHcsResult(ctx, fnErr, doc) + } + info, doc, waitErr := computecore.HcsWaitForOperationResultAndProcessInfo(syscallCtx, op, timeoutMs) + return info, doc, wrapHcsResult(ctx, waitErr, doc) +} + +// submitOperation creates an operation, invokes fn(op) to hand the request +// to HCS, and returns without waiting for completion. The operation handle +// is closed immediately; per the HCS V2 contract this is safe while the +// request is in flight (HCS continues running it and discards the result). +// +// This preserves the V1 fire-and-forget semantics of HcsShutdownComputeSystem +// and HcsTerminateComputeSystem, where the shim only needs to know the +// request was accepted (callers observe completion via the system exit +// notification, not the operation result). +func submitOperation(ctx context.Context, fn func(op computecore.HcsOperation) error) error { + syscallCtx := context.WithoutCancel(ctx) + op, err := computecore.HcsCreateOperation(syscallCtx, 0, 0) + if err != nil { + return err + } + defer computecore.HcsCloseOperation(syscallCtx, op) + if fnErr := fn(op); fnErr != nil { + doc, _ := computecore.HcsGetOperationResult(syscallCtx, op) + return wrapHcsResult(ctx, fnErr, doc) } - return computecore.HcsWaitForOperationResultAndProcessInfo(syscallCtx, op, infiniteTimeout) + return nil } diff --git a/internal/hcs/process.go b/internal/hcs/process.go index f705e344e4..57eaffaf58 100644 --- a/internal/hcs/process.go +++ b/internal/hcs/process.go @@ -100,13 +100,13 @@ func (process *Process) processSignalResult(ctx context.Context, err error) (boo // // For WCOW `guestresource.SignalProcessOptionsWCOW`. func (process *Process) Signal(ctx context.Context, options interface{}) (bool, error) { - process.handleLock.Lock() - defer process.handleLock.Unlock() + process.handleLock.RLock() + defer process.handleLock.RUnlock() operation := "hcs::Process::Signal" if process.handle == 0 { - return false, makeProcessError(process, operation, ErrAlreadyClosed, nil) + return false, makeProcessError(process, operation, ErrAlreadyClosed) } optionsb, err := json.Marshal(options) @@ -115,29 +115,29 @@ func (process *Process) Signal(ctx context.Context, options interface{}) (bool, } optionsStr := string(optionsb) - resultJSON, sigErr := runOperation(ctx, func(op computecore.HcsOperation) error { + _, sigErr := runOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsSignalProcess(ctx, process.handle, op, optionsStr) }) delivered, sigErr := process.processSignalResult(ctx, sigErr) if sigErr != nil { - sigErr = makeProcessError(process, operation, sigErr, processHcsResult(ctx, resultJSON)) + sigErr = makeProcessError(process, operation, sigErr) } return delivered, sigErr } // Kill signals the process to terminate but does not wait for it to finish terminating. func (process *Process) Kill(ctx context.Context) (bool, error) { - process.handleLock.Lock() - defer process.handleLock.Unlock() + process.handleLock.RLock() + defer process.handleLock.RUnlock() operation := "hcs::Process::Kill" if process.handle == 0 { - return false, makeProcessError(process, operation, ErrAlreadyClosed, nil) + return false, makeProcessError(process, operation, ErrAlreadyClosed) } if process.stopped() { - return false, makeProcessError(process, operation, ErrProcessAlreadyStopped, nil) + return false, makeProcessError(process, operation, ErrProcessAlreadyStopped) } if process.killSignalDelivered { @@ -181,7 +181,7 @@ func (process *Process) Kill(ctx context.Context) (bool, error) { } defer newProcessHandle.Close() - resultJSON, killErr := runOperation(ctx, func(op computecore.HcsOperation) error { + _, killErr := runOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsTerminateProcess(ctx, newProcessHandle.handle, op, "") }) if killErr != nil { @@ -208,7 +208,7 @@ func (process *Process) Kill(ctx context.Context) (bool, error) { } delivered, killErr := newProcessHandle.processSignalResult(ctx, killErr) if killErr != nil { - killErr = makeProcessError(newProcessHandle, operation, killErr, processHcsResult(ctx, resultJSON)) + killErr = makeProcessError(newProcessHandle, operation, killErr) } process.killSignalDelivered = delivered @@ -245,10 +245,10 @@ func (process *Process) waitBackground() { // Real exit notification path: parse ProcessStatus and publish. exitCode := -1 var err error - if raw := process.notify.raw; raw != "" { + if raw := process.notify.raw; len(raw) > 0 { properties := &hcsschema.ProcessStatus{} - if uErr := json.Unmarshal([]byte(raw), properties); uErr != nil { - err = makeProcessError(process, operation, uErr, nil) + if uErr := json.Unmarshal(raw, properties); uErr != nil { + err = makeProcessError(process, operation, uErr) } else if properties.LastWaitResult != 0 { log.G(ctx).WithField("wait-result", properties.LastWaitResult).Warning("non-zero last wait result") } else { @@ -284,13 +284,13 @@ func (process *Process) stopped() bool { // ResizeConsole resizes the console of the process. func (process *Process) ResizeConsole(ctx context.Context, width, height uint16) error { - process.handleLock.Lock() - defer process.handleLock.Unlock() + process.handleLock.RLock() + defer process.handleLock.RUnlock() operation := "hcs::Process::ResizeConsole" if process.handle == 0 { - return makeProcessError(process, operation, ErrAlreadyClosed, nil) + return makeProcessError(process, operation, ErrAlreadyClosed) } modifyRequest := hcsschema.ProcessModifyRequest{ Operation: guestrequest.ModifyProcessConsoleSize, @@ -306,11 +306,11 @@ func (process *Process) ResizeConsole(ctx context.Context, width, height uint16) } modifyRequestStr := string(modifyRequestb) - resultJSON, modErr := runOperation(ctx, func(op computecore.HcsOperation) error { + _, modErr := runOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsModifyProcess(ctx, process.handle, op, modifyRequestStr) }) if modErr != nil { - return makeProcessError(process, operation, modErr, processHcsResult(ctx, resultJSON)) + return makeProcessError(process, operation, modErr) } return nil @@ -320,7 +320,7 @@ func (process *Process) ResizeConsole(ctx context.Context, width, height uint16) // already terminated. func (process *Process) ExitCode() (int, error) { if !process.stopped() { - return -1, makeProcessError(process, "hcs::Process::ExitCode", ErrInvalidProcessState, nil) + return -1, makeProcessError(process, "hcs::Process::ExitCode", ErrInvalidProcessState) } if process.waitError != nil { return -1, process.waitError @@ -340,11 +340,11 @@ func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.R trace.StringAttribute("cid", process.SystemID()), trace.Int64Attribute("pid", int64(process.processID))) - process.handleLock.Lock() - defer process.handleLock.Unlock() + process.handleLock.RLock() + defer process.handleLock.RUnlock() if process.handle == 0 { - return nil, nil, nil, makeProcessError(process, operation, ErrAlreadyClosed, nil) + return nil, nil, nil, makeProcessError(process, operation, ErrAlreadyClosed) } process.stdioLock.Lock() @@ -356,16 +356,16 @@ func (process *Process) StdioLegacy() (_ io.WriteCloser, _ io.ReadCloser, _ io.R return stdin, stdout, stderr, nil } - processInfo, resultJSON, err := runProcessOperation(ctx, func(op computecore.HcsOperation) error { + processInfo, _, err := runProcessOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsGetProcessInfo(ctx, process.handle, op) }) if err != nil { - return nil, nil, nil, makeProcessError(process, operation, err, processHcsResult(ctx, resultJSON)) + return nil, nil, nil, makeProcessError(process, operation, err) } pipes, err := makeOpenFiles([]syscall.Handle{processInfo.StdInput, processInfo.StdOutput, processInfo.StdError}) if err != nil { - return nil, nil, nil, makeProcessError(process, operation, err, nil) + return nil, nil, nil, makeProcessError(process, operation, err) } return pipes[0], pipes[1], pipes[2], nil @@ -390,11 +390,11 @@ func (process *Process) CloseStdin(ctx context.Context) (err error) { trace.StringAttribute("cid", process.SystemID()), trace.Int64Attribute("pid", int64(process.processID))) - process.handleLock.Lock() - defer process.handleLock.Unlock() + process.handleLock.RLock() + defer process.handleLock.RUnlock() if process.handle == 0 { - return makeProcessError(process, operation, ErrAlreadyClosed, nil) + return makeProcessError(process, operation, ErrAlreadyClosed) } // HcsModifyProcess request to close stdin will fail if the process has already exited @@ -412,11 +412,11 @@ func (process *Process) CloseStdin(ctx context.Context) (err error) { } modifyRequestStr := string(modifyRequestb) - resultJSON, modErr := runOperation(ctx, func(op computecore.HcsOperation) error { + _, modErr := runOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsModifyProcess(ctx, process.handle, op, modifyRequestStr) }) if modErr != nil { - return makeProcessError(process, operation, modErr, processHcsResult(ctx, resultJSON)) + return makeProcessError(process, operation, modErr) } } diff --git a/internal/hcs/service.go b/internal/hcs/service.go index 371f00460c..ecca23bf2e 100644 --- a/internal/hcs/service.go +++ b/internal/hcs/service.go @@ -20,7 +20,8 @@ func GetServiceProperties(ctx context.Context, q hcsschema.PropertyQuery) (*hcss } propertiesJSON, err := computecore.HcsGetServiceProperties(ctx, string(queryb)) if err != nil { - return nil, &HcsError{Op: operation, Err: err, Events: processHcsResult(ctx, propertiesJSON)} + err = wrapHcsResult(ctx, err, propertiesJSON) + return nil, &HcsError{Op: operation, Err: err, Events: eventsFromError(err)} } if propertiesJSON == "" { @@ -43,7 +44,8 @@ func ModifyServiceSettings(ctx context.Context, settings hcsschema.ModificationR } resultJSON, err := computecore.HcsModifyServiceSettings(ctx, string(settingsJSON)) if err != nil { - return &HcsError{Op: operation, Err: err, Events: processHcsResult(ctx, resultJSON)} + err = wrapHcsResult(ctx, err, resultJSON) + return &HcsError{Op: operation, Err: err, Events: eventsFromError(err)} } return nil } diff --git a/internal/hcs/system.go b/internal/hcs/system.go index 300c22abe8..793d32e254 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -20,6 +20,7 @@ import ( "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/logfields" "github.com/Microsoft/hcsshim/internal/oc" + "github.com/Microsoft/hcsshim/internal/timeout" "github.com/sirupsen/logrus" "go.opencensus.io/trace" ) @@ -97,13 +98,15 @@ func CreateComputeSystem(ctx context.Context, id string, hcsDocumentInterface in } }() - resultJSON, createErr := runOperation(ctx, func(op computecore.HcsOperation) error { + createCtx, cancel := context.WithTimeout(ctx, timeout.SystemCreate) + defer cancel() + _, createErr := runOperation(createCtx, func(op computecore.HcsOperation) error { var hErr error computeSystem.handle, hErr = computecore.HcsCreateComputeSystem(ctx, id, hcsDocument, op, nil) return hErr }) if createErr != nil { - return nil, makeSystemError(computeSystem, operation, createErr, processHcsResult(ctx, resultJSON)) + return nil, makeSystemError(computeSystem, operation, createErr) } computeSystem.registerNotification(ctx) @@ -121,7 +124,7 @@ func OpenComputeSystem(ctx context.Context, id string) (*System, error) { computeSystem := newSystem(id) handle, err := computecore.HcsOpenComputeSystem(ctx, id, syscall.GENERIC_ALL) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } computeSystem.handle = handle defer func() { @@ -146,7 +149,7 @@ func (computeSystem *System) registerNotification(ctx context.Context) { id := registerNotificationContext(computeSystem.id, 0, computeSystem.notify, computeSystem.migrationNotifyCh) if err := computecore.HcsSetComputeSystemCallback( ctx, computeSystem.handle, - computecore.HcsEventOptionEnableOperationCallbacks|computecore.HcsEventOptionEnableLiveMigrationEvents, + computecore.HcsEventOptionEnableLiveMigrationEvents, uintptr(id), notificationCallback, ); err != nil { unregisterNotificationContext(id) @@ -197,7 +200,7 @@ func GetComputeSystems(ctx context.Context, q schema1.ComputeSystemQuery) ([]sch return computecore.HcsEnumerateComputeSystems(ctx, query, op) }) if err != nil { - return nil, &HcsError{Op: operation, Err: err, Events: processHcsResult(ctx, resultJSON)} + return nil, &HcsError{Op: operation, Err: err, Events: eventsFromError(err)} } if resultJSON == "" { return nil, ErrUnexpectedValue @@ -219,20 +222,22 @@ func (computeSystem *System) Start(ctx context.Context) (err error) { defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() // Prevent starting an exited system: we do not recreate waitBlock or // rerun waitBackground, so we have no way to be notified of it closing again. if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } - resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + startCtx, cancel := context.WithTimeout(ctx, timeout.SystemStart) + defer cancel() + _, callErr := runOperation(startCtx, func(op computecore.HcsOperation) error { return computecore.HcsStartComputeSystem(ctx, computeSystem.handle, op, "") }) if callErr != nil { - return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, callErr) } computeSystem.startTime = time.Now() return nil @@ -245,8 +250,8 @@ func (computeSystem *System) ID() string { // Shutdown requests a compute system shutdown. func (computeSystem *System) Shutdown(ctx context.Context) error { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() operation := "hcs::System::Shutdown" @@ -254,22 +259,22 @@ func (computeSystem *System) Shutdown(ctx context.Context) error { return nil } - resultJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + err := submitOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsShutDownComputeSystem(ctx, computeSystem.handle, op, "") }) if err != nil && !errors.Is(err, ErrVmcomputeAlreadyStopped) && !errors.Is(err, ErrComputeSystemDoesNotExist) && !errors.Is(err, ErrVmcomputeOperationPending) { - return makeSystemError(computeSystem, operation, err, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, err) } return nil } // Terminate requests a compute system terminate. func (computeSystem *System) Terminate(ctx context.Context) error { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() operation := "hcs::System::Terminate" @@ -277,14 +282,14 @@ func (computeSystem *System) Terminate(ctx context.Context) error { return nil } - resultJSON, err := runOperation(ctx, func(op computecore.HcsOperation) error { + err := submitOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsTerminateComputeSystem(ctx, computeSystem.handle, op, "") }) if err != nil && !errors.Is(err, ErrVmcomputeAlreadyStopped) && !errors.Is(err, ErrComputeSystemDoesNotExist) && !errors.Is(err, ErrVmcomputeOperationPending) { - return makeSystemError(computeSystem, operation, err, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, err) } return nil } @@ -317,16 +322,16 @@ func (computeSystem *System) waitBackground() { // Real exit notification path: parse SystemExitStatus and publish. var err error - if raw := computeSystem.notify.raw; raw != "" { + if raw := computeSystem.notify.raw; len(raw) > 0 { var status struct { Status int32 `json:"Status"` ExitType string `json:"ExitType"` } - if uErr := json.Unmarshal([]byte(raw), &status); uErr != nil { - log.G(ctx).WithError(uErr).WithField("exit-data", raw).Warning("failed to parse SystemExitStatus") + if uErr := json.Unmarshal(raw, &status); uErr != nil { + log.G(ctx).WithError(uErr).WithField("exit-data", string(raw)).Warning("failed to parse SystemExitStatus") } else if status.ExitType == "UnexpectedExit" { log.G(ctx).Debug("unexpected system exit") - computeSystem.exitError = makeSystemError(computeSystem, operation, ErrVmcomputeUnexpectedExit, nil) + computeSystem.exitError = makeSystemError(computeSystem, operation, ErrVmcomputeUnexpectedExit) } } computeSystem.closedWaitOnce.Do(func() { @@ -386,18 +391,18 @@ func (computeSystem *System) ExitError() error { // Properties returns the requested container properties targeting a V1 schema container. func (computeSystem *System) Properties(ctx context.Context, types ...schema1.PropertyType) (*schema1.ContainerProperties, error) { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() operation := "hcs::System::Properties" if computeSystem.handle == 0 { - return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed) } queryBytes, err := json.Marshal(schema1.PropertyQuery{PropertyTypes: types}) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } query := string(queryBytes) @@ -405,7 +410,7 @@ func (computeSystem *System) Properties(ctx context.Context, types ...schema1.Pr return computecore.HcsGetComputeSystemProperties(ctx, computeSystem.handle, op, query) }) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, processHcsResult(ctx, propertiesJSON)) + return nil, makeSystemError(computeSystem, operation, err) } if propertiesJSON == "" { @@ -413,7 +418,7 @@ func (computeSystem *System) Properties(ctx context.Context, types ...schema1.Pr } properties := &schema1.ContainerProperties{} if err := json.Unmarshal([]byte(propertiesJSON), properties); err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } return properties, nil @@ -535,12 +540,12 @@ func (computeSystem *System) hcsPropertiesV2Query(ctx context.Context, types []h operation := "hcs::System::PropertiesV2" if computeSystem.handle == 0 { - return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed) } queryBytes, err := json.Marshal(hcsschema.PropertyQuery{PropertyTypes: types}) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } query := string(queryBytes) @@ -548,7 +553,7 @@ func (computeSystem *System) hcsPropertiesV2Query(ctx context.Context, types []h return computecore.HcsGetComputeSystemProperties(ctx, computeSystem.handle, op, query) }) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, processHcsResult(ctx, propertiesJSON)) + return nil, makeSystemError(computeSystem, operation, err) } if propertiesJSON == "" { @@ -556,7 +561,7 @@ func (computeSystem *System) hcsPropertiesV2Query(ctx context.Context, types []h } props := &hcsschema.Properties{} if err := json.Unmarshal([]byte(propertiesJSON), props); err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } return props, nil @@ -564,8 +569,8 @@ func (computeSystem *System) hcsPropertiesV2Query(ctx context.Context, types []h // PropertiesV2 returns the requested compute systems properties targeting a V2 schema compute system. func (computeSystem *System) PropertiesV2(ctx context.Context, types ...hcsschema.PropertyType) (_ *hcsschema.Properties, err error) { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() // Let HCS tally up the total for VM based queries instead of querying ourselves. if computeSystem.typ != "container" { @@ -628,11 +633,11 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() if computeSystem.handle == 0 { - return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed) } log.G(ctx).WithFields(logrus.Fields{ @@ -643,7 +648,7 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. queryBytes, err := json.Marshal(query) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } queryStr := string(queryBytes) @@ -651,7 +656,7 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. return computecore.HcsGetComputeSystemProperties(ctx, computeSystem.handle, op, queryStr) }) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, processHcsResult(ctx, propertiesJSON)) + return nil, makeSystemError(computeSystem, operation, err) } if propertiesJSON == "" { @@ -660,7 +665,7 @@ func (computeSystem *System) PropertiesV3(ctx context.Context, query *hcsschema. props := &hcsschema.Properties{} if err := json.Unmarshal([]byte(propertiesJSON), props); err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } return props, nil @@ -675,18 +680,20 @@ func (computeSystem *System) Pause(ctx context.Context) (err error) { defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } - resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + pauseCtx, cancel := context.WithTimeout(ctx, timeout.SystemPause) + defer cancel() + _, callErr := runOperation(pauseCtx, func(op computecore.HcsOperation) error { return computecore.HcsPauseComputeSystem(ctx, computeSystem.handle, op, "") }) if callErr != nil { - return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, callErr) } return nil } @@ -700,18 +707,20 @@ func (computeSystem *System) Resume(ctx context.Context) (err error) { defer func() { oc.SetSpanStatus(span, err) }() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } - resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + resumeCtx, cancel := context.WithTimeout(ctx, timeout.SystemResume) + defer cancel() + _, callErr := runOperation(resumeCtx, func(op computecore.HcsOperation) error { return computecore.HcsResumeComputeSystem(ctx, computeSystem.handle, op, "") }) if callErr != nil { - return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, callErr) } return nil } @@ -731,18 +740,20 @@ func (computeSystem *System) Save(ctx context.Context, options interface{}) (err } saveOptionsStr := string(saveOptions) - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } - resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + saveCtx, cancel := context.WithTimeout(ctx, timeout.SystemSave) + defer cancel() + _, callErr := runOperation(saveCtx, func(op computecore.HcsOperation) error { return computecore.HcsSaveComputeSystem(ctx, computeSystem.handle, op, saveOptionsStr) }) if callErr != nil { - return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, callErr) } return nil } @@ -758,16 +769,16 @@ func (computeSystem *System) Save(ctx context.Context, options interface{}) (err // exit and surfaces as HCS_E_PROCESS_ALREADY_STOPPED, which would wrongly // fail the create. func (computeSystem *System) createProcess(ctx context.Context, operation string, c interface{}) (*Process, *computecore.HcsProcessInformation, error) { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() if computeSystem.handle == 0 { - return nil, nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return nil, nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed) } configurationb, err := json.Marshal(c) if err != nil { - return nil, nil, makeSystemError(computeSystem, operation, err, nil) + return nil, nil, makeSystemError(computeSystem, operation, err) } configuration := string(configurationb) @@ -781,7 +792,7 @@ func (computeSystem *System) createProcess(ctx context.Context, operation string } var processHandle computecore.HcsProcess - processInfo, resultJSON, createErr := runProcessOperation(ctx, func(op computecore.HcsOperation) error { + processInfo, _, createErr := runProcessOperation(ctx, func(op computecore.HcsOperation) error { var hErr error processHandle, hErr = computecore.HcsCreateProcess(ctx, computeSystem.handle, configuration, op, nil) return hErr @@ -790,16 +801,16 @@ func (computeSystem *System) createProcess(ctx context.Context, operation string // No handle means the create itself failed; only a failed wait with // a live handle is the recoverable sync-completion case. if processHandle == 0 { - return nil, nil, makeSystemError(computeSystem, operation, createErr, processHcsResult(ctx, resultJSON)) + return nil, nil, makeSystemError(computeSystem, operation, createErr) } log.G(ctx).WithError(createErr).Debug("HcsCreateProcess wait failed; falling back to HcsGetProcessInfo") - processInfo, resultJSON, createErr = runProcessOperation(ctx, func(op computecore.HcsOperation) error { + processInfo, _, createErr = runProcessOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsGetProcessInfo(ctx, processHandle, op) }) if createErr != nil { computecore.HcsCloseProcess(ctx, processHandle) - return nil, nil, makeSystemError(computeSystem, operation, createErr, processHcsResult(ctx, resultJSON)) + return nil, nil, makeSystemError(computeSystem, operation, createErr) } } @@ -822,7 +833,7 @@ func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) ( pipes, err := makeOpenFiles([]syscall.Handle{processInfo.StdInput, processInfo.StdOutput, processInfo.StdError}) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } process.stdin = pipes[0] process.stdout = pipes[1] @@ -837,18 +848,18 @@ func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) ( // OpenProcess gets an interface to an existing process within the computeSystem. func (computeSystem *System) OpenProcess(ctx context.Context, pid int) (*Process, error) { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() operation := "hcs::System::OpenProcess" if computeSystem.handle == 0 { - return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return nil, makeSystemError(computeSystem, operation, ErrAlreadyClosed) } processHandle, err := computecore.HcsOpenProcess(ctx, computeSystem.handle, uint32(pid), syscall.GENERIC_ALL) if err != nil { - return nil, makeSystemError(computeSystem, operation, err, nil) + return nil, makeSystemError(computeSystem, operation, err) } process := newProcess(processHandle, pid, computeSystem) @@ -902,13 +913,13 @@ func (computeSystem *System) CloseCtx(ctx context.Context) (err error) { // Modify the System by sending a request to HCS func (computeSystem *System) Modify(ctx context.Context, config interface{}) error { - computeSystem.handleLock.Lock() - defer computeSystem.handleLock.Unlock() + computeSystem.handleLock.RLock() + defer computeSystem.handleLock.RUnlock() operation := "hcs::System::Modify" if computeSystem.handle == 0 { - return makeSystemError(computeSystem, operation, ErrAlreadyClosed, nil) + return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } requestBytes, err := json.Marshal(config) @@ -918,11 +929,11 @@ func (computeSystem *System) Modify(ctx context.Context, config interface{}) err requestJSON := string(requestBytes) - resultJSON, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { + _, callErr := runOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsModifyComputeSystem(ctx, computeSystem.handle, op, requestJSON, 0) }) if callErr != nil { - return makeSystemError(computeSystem, operation, callErr, processHcsResult(ctx, resultJSON)) + return makeSystemError(computeSystem, operation, callErr) } return nil } From 2792ab462356a0e263c1b5bee5a8acbc0509d054 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 23 May 2026 23:52:38 +0530 Subject: [PATCH 3/7] fix: improve error handling during process creation recovery Signed-off-by: Harsh Rawat --- internal/hcs/system.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/internal/hcs/system.go b/internal/hcs/system.go index 793d32e254..3b00c46ae0 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -805,10 +805,13 @@ func (computeSystem *System) createProcess(ctx context.Context, operation string } log.G(ctx).WithError(createErr).Debug("HcsCreateProcess wait failed; falling back to HcsGetProcessInfo") - processInfo, _, createErr = runProcessOperation(ctx, func(op computecore.HcsOperation) error { + var recoverErr error + processInfo, _, recoverErr = runProcessOperation(ctx, func(op computecore.HcsOperation) error { return computecore.HcsGetProcessInfo(ctx, processHandle, op) }) - if createErr != nil { + if recoverErr != nil { + // Recovery error (e.g. RPC_E_NULL_CONTEXT_HANDLE on a stale handle) + // hides the real cause; surface the original create-wait error. computecore.HcsCloseProcess(ctx, processHandle) return nil, nil, makeSystemError(computeSystem, operation, createErr) } From 975e13b00d65deea642d26ea734fae37fa586d73 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sun, 24 May 2026 23:50:05 +0530 Subject: [PATCH 4/7] hcs: simplify Close wake-up for waitBackground Previously, Close signaled waitBackground via a dedicated notify.closed channel, and waitBackground had to forward that into waitBlock with the right waitError. This left two channels doing the same job and split the "finalize the wait state" logic across two places. Now Close finalizes the wait state itself: under closedWaitOnce it sets waitError = ErrAlreadyClosed and closes waitBlock directly. waitBackground selects on waitBlock (Close path, return early) or notify.exited (real exit, parse status and publish). The notify.closed channel and its signalClosed helper are removed. Behavior is unchanged: Close still does not synthesize an exit, so Wait/WaitChannel callers see ErrAlreadyClosed instead of a fake exit code. Signed-off-by: Harsh Rawat --- internal/hcs/notification.go | 40 +++++++++--------------------------- internal/hcs/process.go | 16 +++++++-------- internal/hcs/system.go | 15 +++++++------- 3 files changed, 26 insertions(+), 45 deletions(-) diff --git a/internal/hcs/notification.go b/internal/hcs/notification.go index fbe8726a0e..ca0ac17cfc 100644 --- a/internal/hcs/notification.go +++ b/internal/hcs/notification.go @@ -36,37 +36,24 @@ var ( notificationCallback = syscall.NewCallback(notificationHandler) ) -// notificationState is the rendezvous between the HCS callback thread and -// waitBackground. waitBackground must NOT poll the HCS handle directly: that -// races with Close and can fault. -// -// HCS does not synthesize a final exit notification when HcsCloseProcess / -// HcsCloseComputeSystem unregisters the callback, so a single "wait -// finished" channel cannot distinguish a real exit from a Close. We expose -// two channels and let waitBackground select on them: -// -// - exited: closed by signalExit on a terminal {System,Process}Exited -// event; raw holds the event's JSON payload. -// - closed: closed by signalClosed from Close; nothing further will -// arrive, so waitBackground must return without publishing a -// synthetic exit (which would surface to consumers as exit_code=255). +// notificationState carries a real exit event from the HCS callback to the +// owner's waitBackground goroutine. The callback runs on an HCS thread and +// must not block, so it only stores the raw payload and closes `exited`; +// waitBackground parses it. type notificationState struct { - exitOnce sync.Once - closeOnce sync.Once - exited chan struct{} - closed chan struct{} - raw json.RawMessage + exitOnce sync.Once + exited chan struct{} + raw json.RawMessage } func newNotificationState() *notificationState { return ¬ificationState{ exited: make(chan struct{}), - closed: make(chan struct{}), } } -// signalExit records a real exit event and unblocks waitBackground. Safe to -// call multiple times; only the first wins. +// signalExit hands a terminal exit event to waitBackground. Safe to call +// multiple times; only the first call records the payload. func (s *notificationState) signalExit(raw json.RawMessage) { s.exitOnce.Do(func() { s.raw = raw @@ -74,12 +61,6 @@ func (s *notificationState) signalExit(raw json.RawMessage) { }) } -// signalClosed unblocks waitBackground without recording an exit. Call from -// Close when no terminal event was observed. Safe to call multiple times. -func (s *notificationState) signalClosed() { - s.closeOnce.Do(func() { close(s.closed) }) -} - // notificationContext is the per-handle data resolved from the callback's // opaque ctx. processID == 0 means the callback belongs to a system handle. type notificationContext struct { @@ -144,8 +125,7 @@ func notificationHandler(eventPtr uintptr, ctx uintptr) uintptr { source = "process" } switch e.Type { - // Only terminal events count as a real exit; do NOT signal - // closed here. + // Only terminal exit events propagate to waitBackground. case computecore.HcsEventTypeSystemExited, computecore.HcsEventTypeProcessExited: if nc.state != nil { nc.state.signalExit(json.RawMessage(eventData)) diff --git a/internal/hcs/process.go b/internal/hcs/process.go index 57eaffaf58..482de6e011 100644 --- a/internal/hcs/process.go +++ b/internal/hcs/process.go @@ -236,7 +236,7 @@ func (process *Process) waitBackground() { trace.Int64Attribute("pid", int64(process.processID))) select { - case <-process.notify.closed: + case <-process.waitBlock: log.G(ctx).Debug("process waitBackground returning without exit notification (handle closed)") return case <-process.notify.exited: @@ -518,14 +518,14 @@ func (process *Process) Close() (err error) { unregisterNotificationContext(process.notificationID) process.notificationID = 0 - // Wake waitBackground so its goroutine can return. We do NOT close - // process.waitBlock: Wait() and ExitCode() treat that channel - // closing as proof the process exited, and would publish exitCode=-1 - // (rendered as 255 by containerd) for a process that may still be - // running. - process.notify.signalClosed() - + // Release Wait/ExitCode callers with ErrAlreadyClosed + // and unblock waitBackground. process.handle = 0 + process.closedWaitOnce.Do(func() { + process.exitCode = -1 + process.waitError = ErrAlreadyClosed + close(process.waitBlock) + }) return nil } diff --git a/internal/hcs/system.go b/internal/hcs/system.go index 3b00c46ae0..f910fbeaea 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -314,7 +314,7 @@ func (computeSystem *System) waitBackground() { span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) select { - case <-computeSystem.notify.closed: + case <-computeSystem.waitBlock: log.G(ctx).Debug("system waitBackground returning without exit notification (handle closed)") return case <-computeSystem.notify.exited: @@ -903,13 +903,14 @@ func (computeSystem *System) CloseCtx(ctx context.Context) (err error) { unregisterNotificationContext(computeSystem.notificationID) computeSystem.notificationID = 0 - // Wake waitBackground so it can return. We do NOT close - // computeSystem.waitBlock: that channel is the public "system - // exited" signal (via WaitChannel/Wait), and closing it for a - // still-running system fakes an exit to callers. - computeSystem.notify.signalClosed() - + // Release Wait/WaitChannel callers with ErrAlreadyClosed + // and unblock waitBackground. computeSystem.handle = 0 + computeSystem.closedWaitOnce.Do(func() { + computeSystem.waitError = ErrAlreadyClosed + computeSystem.stopTime = time.Now() + close(computeSystem.waitBlock) + }) return nil } From 4ce98cf9cb714204fb1dd4e469c1b0555f9a8b1c Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Mon, 25 May 2026 12:05:22 +0530 Subject: [PATCH 5/7] internal/hcs: surface HcsEventTypeServiceDisconnect as wait failure The V2 notification handler only dispatched System/ProcessExited, so a ServiceDisconnect (no payload) left waitBackground blocked forever and Wait() hung until Close. V1 returned ErrUnexpectedProcessAbort here. notificationState now has two channels -- exit (payload) and abort (error) -- and waitBackground selects on both. ServiceDisconnect routes to signalAbort(ErrUnexpectedProcessAbort), surfaced via make{System, Process}Error. Also stop swallowing HcsSet*Callback failures in registerNotification: return the error so Create/Open{ComputeSystem,Process} can tear down the handle (OpenProcess gains the missing cleanup defer). Signed-off-by: Harsh Rawat --- internal/hcs/notification.go | 42 +++++++++++++++++++-------------- internal/hcs/process.go | 22 ++++++++++-------- internal/hcs/system.go | 45 ++++++++++++++++++++++++------------ 3 files changed, 67 insertions(+), 42 deletions(-) diff --git a/internal/hcs/notification.go b/internal/hcs/notification.go index ca0ac17cfc..627d931c1a 100644 --- a/internal/hcs/notification.go +++ b/internal/hcs/notification.go @@ -36,28 +36,35 @@ var ( notificationCallback = syscall.NewCallback(notificationHandler) ) -// notificationState carries a real exit event from the HCS callback to the -// owner's waitBackground goroutine. The callback runs on an HCS thread and -// must not block, so it only stores the raw payload and closes `exited`; -// waitBackground parses it. +// notificationState rendezvous a terminal HCS event with waitBackground. +// Exactly one of exit (normal exit + payload) or abort (abnormal termination) +// is signaled. Both channels are buffered(1) and closed after send. type notificationState struct { - exitOnce sync.Once - exited chan struct{} - raw json.RawMessage + signalOnce sync.Once + exit chan json.RawMessage + abort chan error } func newNotificationState() *notificationState { return ¬ificationState{ - exited: make(chan struct{}), + exit: make(chan json.RawMessage, 1), + abort: make(chan error, 1), } } -// signalExit hands a terminal exit event to waitBackground. Safe to call -// multiple times; only the first call records the payload. +// signalExit delivers a normal exit payload. First signal wins. func (s *notificationState) signalExit(raw json.RawMessage) { - s.exitOnce.Do(func() { - s.raw = raw - close(s.exited) + s.signalOnce.Do(func() { + s.exit <- raw + close(s.exit) + }) +} + +// signalAbort delivers an abnormal-termination error. First signal wins. +func (s *notificationState) signalAbort(err error) { + s.signalOnce.Do(func() { + s.abort <- err + close(s.abort) }) } @@ -125,16 +132,17 @@ func notificationHandler(eventPtr uintptr, ctx uintptr) uintptr { source = "process" } switch e.Type { - // Only terminal exit events propagate to waitBackground. case computecore.HcsEventTypeSystemExited, computecore.HcsEventTypeProcessExited: if nc.state != nil { nc.state.signalExit(json.RawMessage(eventData)) } + case computecore.HcsEventTypeServiceDisconnect: + if nc.state != nil { + nc.state.signalAbort(ErrUnexpectedProcessAbort) + } case computecore.HcsEventTypeGroupLiveMigration: // Forward to the system's migration channel, if one was - // registered. Decoding failures and a full channel are - // both logged-and-dropped: the HCS callback thread must - // never block, and a malformed payload can't be acted on. + // registered. if nc.migrationCh != nil { dispatchMigrationEvent(nc.migrationCh, e.Type, json.RawMessage(eventData)) } diff --git a/internal/hcs/process.go b/internal/hcs/process.go index 482de6e011..deaef06c03 100644 --- a/internal/hcs/process.go +++ b/internal/hcs/process.go @@ -235,17 +235,19 @@ func (process *Process) waitBackground() { trace.StringAttribute("cid", process.SystemID()), trace.Int64Attribute("pid", int64(process.processID))) + var raw json.RawMessage + exitCode := -1 + var err error select { case <-process.waitBlock: log.G(ctx).Debug("process waitBackground returning without exit notification (handle closed)") return - case <-process.notify.exited: + case raw = <-process.notify.exit: + case abortErr := <-process.notify.abort: + err = makeProcessError(process, operation, abortErr) } - // Real exit notification path: parse ProcessStatus and publish. - exitCode := -1 - var err error - if raw := process.notify.raw; len(raw) > 0 { + if err == nil && len(raw) > 0 { properties := &hcsschema.ProcessStatus{} if uErr := json.Unmarshal(raw, properties); uErr != nil { err = makeProcessError(process, operation, uErr) @@ -531,9 +533,9 @@ func (process *Process) Close() (err error) { } // registerNotification registers the package-wide HCS notification callback -// on this process handle. Failures are logged only: missing the diagnostic -// callback must not break process operation. -func (process *Process) registerNotification(ctx context.Context) { +// on this process handle. Must be called BEFORE waitBackground starts so +// notifications are not missed. +func (process *Process) registerNotification(ctx context.Context) error { id := registerNotificationContext(process.SystemID(), process.processID, process.notify, nil) if err := computecore.HcsSetProcessCallback( ctx, process.handle, @@ -541,8 +543,8 @@ func (process *Process) registerNotification(ctx context.Context) { uintptr(id), notificationCallback, ); err != nil { unregisterNotificationContext(id) - log.G(ctx).WithError(err).Warn("failed to register HCS process notification callback") - return + return err } process.notificationID = id + return nil } diff --git a/internal/hcs/system.go b/internal/hcs/system.go index f910fbeaea..433ec608b2 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -109,7 +109,9 @@ func CreateComputeSystem(ctx context.Context, id string, hcsDocumentInterface in return nil, makeSystemError(computeSystem, operation, createErr) } - computeSystem.registerNotification(ctx) + if err = computeSystem.registerNotification(ctx); err != nil { + return nil, makeSystemError(computeSystem, operation, err) + } go computeSystem.waitBackground() if err = computeSystem.getCachedProperties(ctx); err != nil { return nil, err @@ -126,13 +128,17 @@ func OpenComputeSystem(ctx context.Context, id string) (*System, error) { if err != nil { return nil, makeSystemError(computeSystem, operation, err) } + computeSystem.handle = handle defer func() { if err != nil { computeSystem.Close() } }() - computeSystem.registerNotification(ctx) + if err = computeSystem.registerNotification(ctx); err != nil { + return nil, makeSystemError(computeSystem, operation, err) + } + go computeSystem.waitBackground() if err = computeSystem.getCachedProperties(ctx); err != nil { return nil, err @@ -141,11 +147,9 @@ func OpenComputeSystem(ctx context.Context, id string) (*System, error) { } // registerNotification registers the package-wide HCS notification callback -// on this system's primary handle. Failures are logged only: the callback -// is purely diagnostic and must not prevent the system from being used. -// Must be called BEFORE waitBackground starts so notifications are not -// missed. -func (computeSystem *System) registerNotification(ctx context.Context) { +// on this system's primary handle. Must be called BEFORE waitBackground +// starts so notifications are not missed. +func (computeSystem *System) registerNotification(ctx context.Context) error { id := registerNotificationContext(computeSystem.id, 0, computeSystem.notify, computeSystem.migrationNotifyCh) if err := computecore.HcsSetComputeSystemCallback( ctx, computeSystem.handle, @@ -153,10 +157,10 @@ func (computeSystem *System) registerNotification(ctx context.Context) { uintptr(id), notificationCallback, ); err != nil { unregisterNotificationContext(id) - log.G(ctx).WithError(err).Warn("failed to register HCS system notification callback") - return + return err } computeSystem.notificationID = id + return nil } func (computeSystem *System) getCachedProperties(ctx context.Context) error { @@ -313,16 +317,18 @@ func (computeSystem *System) waitBackground() { defer span.End() span.AddAttributes(trace.StringAttribute("cid", computeSystem.id)) + var raw json.RawMessage + var err error select { case <-computeSystem.waitBlock: log.G(ctx).Debug("system waitBackground returning without exit notification (handle closed)") return - case <-computeSystem.notify.exited: + case raw = <-computeSystem.notify.exit: + case abortErr := <-computeSystem.notify.abort: + err = makeSystemError(computeSystem, operation, abortErr) } - // Real exit notification path: parse SystemExitStatus and publish. - var err error - if raw := computeSystem.notify.raw; len(raw) > 0 { + if err == nil && len(raw) > 0 { var status struct { Status int32 `json:"Status"` ExitType string `json:"ExitType"` @@ -843,7 +849,9 @@ func (computeSystem *System) CreateProcess(ctx context.Context, c interface{}) ( process.stderr = pipes[2] process.hasCachedStdio = true - process.registerNotification(ctx) + if err = process.registerNotification(ctx); err != nil { + return nil, makeSystemError(computeSystem, operation, err) + } go process.waitBackground() return process, nil @@ -866,7 +874,14 @@ func (computeSystem *System) OpenProcess(ctx context.Context, pid int) (*Process } process := newProcess(processHandle, pid, computeSystem) - process.registerNotification(ctx) + defer func() { + if err != nil { + _ = process.Close() + } + }() + if err = process.registerNotification(ctx); err != nil { + return nil, makeSystemError(computeSystem, operation, err) + } go process.waitBackground() return process, nil From d707a4d125544f5862acf004934a2a2cbbf5d913 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Tue, 26 May 2026 11:35:23 +0530 Subject: [PATCH 6/7] hcs: cancel pending HCS operation on wait timeout execute method: ctx/timeout are watchdogs only, callers free handles via defer once execute returns, abandoning an in-flight call crashes computecore.dll, use HcsCancelOperation to bound long operations. In runOperation/runProcessOperation, when HcsWaitForOperationResult returns HCS_E_OPERATION_TIMEOUT, call HcsCancelOperation before the deferred close so the request does not outlive this call. Signed-off-by: Harsh Rawat --- internal/computecore/computecore.go | 58 ++++++++++++++--------------- internal/hcs/operation.go | 20 ++++++++-- 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/internal/computecore/computecore.go b/internal/computecore/computecore.go index e00a287d15..577cbc3081 100644 --- a/internal/computecore/computecore.go +++ b/internal/computecore/computecore.go @@ -8,7 +8,6 @@ import ( "time" "unsafe" - "github.com/sirupsen/logrus" "go.opencensus.io/trace" "github.com/Microsoft/hcsshim/internal/interop" @@ -94,39 +93,40 @@ import ( // errVmcomputeOperationPending is an error encountered when the operation is being completed asynchronously const errVmcomputeOperationPending = syscall.Errno(0xC0370103) +// execute runs f synchronously to completion. ctx and timeout are +// watchdogs only: each emits a one-shot warning when exceeded and the +// wait continues until f returns. +// +// Callers of the wrapping HCS API typically release the handles they +// passed in via defer once the call returns. Abandoning f mid-flight +// would let those defers tear down handles the syscall is still using, +// which has been observed to crash the shim with EXCEPTION_ACCESS_VIOLATION +// inside computecore.dll. Long-running operations should be bounded via +// HcsCancelOperation rather than by returning early. func execute(ctx gcontext.Context, timeout time.Duration, f func() error) error { - now := time.Now() - if timeout > 0 { - var cancel gcontext.CancelFunc - ctx, cancel = gcontext.WithTimeout(ctx, timeout) - defer cancel() - } + done := make(chan error, 1) + go func() { done <- f() }() - deadline, ok := ctx.Deadline() - trueTimeout := timeout - if ok { - trueTimeout = deadline.Sub(now) - log.G(ctx).WithFields(logrus.Fields{ - logfields.Timeout: trueTimeout, - "desiredTimeout": timeout, - }).Trace("Executing syscall with deadline") + var watcher <-chan time.Time + if timeout > 0 { + t := time.NewTimer(timeout) + defer t.Stop() + watcher = t.C } - done := make(chan error, 1) - go func() { - done <- f() - }() - select { - case <-ctx.Done(): - if ctx.Err() == gcontext.DeadlineExceeded { - log.G(ctx).WithField(logfields.Timeout, trueTimeout). - Warning("Syscall did not complete within operation timeout. This may indicate a platform issue. " + - "If it appears to be making no forward progress, obtain the stacks and see if there is a syscall " + - "stuck in the platform API for a significant length of time.") + for { + select { + case err := <-done: + return err + case <-watcher: + log.G(ctx).WithField(logfields.Timeout, timeout). + Warning("HCS syscall exceeded timeout; still waiting to avoid use-after-free in computecore.dll") + watcher = nil + case <-ctx.Done(): + log.G(ctx).WithError(ctx.Err()). + Warning("HCS syscall context canceled; still waiting to avoid use-after-free in computecore.dll") + ctx = gcontext.WithoutCancel(ctx) } - return ctx.Err() - case err := <-done: - return err } } diff --git a/internal/hcs/operation.go b/internal/hcs/operation.go index a1a61415b7..d5ce3beda2 100644 --- a/internal/hcs/operation.go +++ b/internal/hcs/operation.go @@ -4,6 +4,8 @@ package hcs import ( "context" + "errors" + "syscall" "time" "github.com/Microsoft/hcsshim/internal/computecore" @@ -13,6 +15,11 @@ import ( // HcsWaitForOperationResult to wait forever (Win32 INFINITE, 0xFFFFFFFF). const infiniteTimeout = ^uint32(0) +// hcsErrOperationTimeout is HCS_E_OPERATION_TIMEOUT, returned by +// HcsWaitForOperationResult when the wait elapses while HCS is still +// tracking the operation. +const hcsErrOperationTimeout = syscall.Errno(0x80370118) + // waitTimeoutMs derives the TimeoutMs argument for HcsWaitForOperationResult // from ctx's deadline. With no deadline (or one already past) it falls back // to INFINITE / a minimal poll respectively. @@ -38,9 +45,8 @@ func waitTimeoutMs(ctx context.Context) uint32 { // describing the error events). The operation is always closed before return. // // The wait is bounded by ctx's deadline (if any); otherwise it waits forever. -// ctx cancellation is otherwise stripped via context.WithoutCancel because -// abandoning the syscall while HCS still owns the operation handle leads to -// use-after-free crashes (EXCEPTION_ACCESS_VIOLATION) inside computecore.dll. +// ctx cancellation is stripped via context.WithoutCancel; see computecore.execute +// for why HCS syscalls cannot be abandoned mid-flight. func runOperation(ctx context.Context, fn func(op computecore.HcsOperation) error) (resultDoc string, err error) { timeoutMs := waitTimeoutMs(ctx) syscallCtx := context.WithoutCancel(ctx) @@ -58,6 +64,11 @@ func runOperation(ctx context.Context, fn func(op computecore.HcsOperation) erro return doc, wrapHcsResult(ctx, fnErr, doc) } doc, waitErr := computecore.HcsWaitForOperationResult(syscallCtx, op, timeoutMs) + if errors.Is(waitErr, hcsErrOperationTimeout) { + // Wait deadline elapsed but HCS is still tracking the request; + // ask it to abort so the operation does not outlive this call. + _ = computecore.HcsCancelOperation(syscallCtx, op) + } return doc, wrapHcsResult(ctx, waitErr, doc) } @@ -79,6 +90,9 @@ func runProcessOperation(ctx context.Context, fn func(op computecore.HcsOperatio return info, doc, wrapHcsResult(ctx, fnErr, doc) } info, doc, waitErr := computecore.HcsWaitForOperationResultAndProcessInfo(syscallCtx, op, timeoutMs) + if errors.Is(waitErr, hcsErrOperationTimeout) { + _ = computecore.HcsCancelOperation(syscallCtx, op) + } return info, doc, wrapHcsResult(ctx, waitErr, doc) } From 8fbc5e7257b3ad3a7d56763a5082086036ae0916 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Tue, 26 May 2026 11:35:54 +0530 Subject: [PATCH 7/7] hcs: update FinalizeLiveMigration to use options for final operation Signed-off-by: Harsh Rawat --- internal/hcs/migration.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/internal/hcs/migration.go b/internal/hcs/migration.go index 89ebdc8e7b..749671793d 100644 --- a/internal/hcs/migration.go +++ b/internal/hcs/migration.go @@ -201,9 +201,8 @@ func (computeSystem *System) StartLiveMigrationTransfer(ctx context.Context, opt return nil } -// FinalizeLiveMigration completes the live migration workflow. If resume is true the VM -// is resumed on the destination; otherwise it is stopped. -func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, resume bool) (err error) { +// FinalizeLiveMigration completes the live migration workflow. +func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, opts *hcsschema.MigrationFinalizedOptions) (err error) { operation := "hcs::System::FinalizeLiveMigration" ctx, span := oc.StartSpan(ctx, operation) @@ -218,12 +217,10 @@ func (computeSystem *System) FinalizeLiveMigration(ctx context.Context, resume b return makeSystemError(computeSystem, operation, ErrAlreadyClosed) } - // Choose whether to resume or stop the VM after migration. - finalOp := hcsschema.MigrationFinalOperationStop - if resume { - finalOp = hcsschema.MigrationFinalOperationResume + if opts == nil { + opts = &hcsschema.MigrationFinalizedOptions{} } - optionsJSON, err := json.Marshal(hcsschema.MigrationFinalizedOptions{FinalizedOperation: finalOp}) + optionsJSON, err := json.Marshal(opts) if err != nil { return makeSystemError(computeSystem, operation, err) }