From b7d75a17bae83495315f76926ef42d726a521759 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Thu, 26 Feb 2026 04:26:26 +0530 Subject: [PATCH 1/7] move gcs logging declarations and methods to reusable vmutils pkg Signed-off-by: Harsh Rawat --- internal/tools/uvmboot/lcow.go | 3 +- internal/uvm/create_lcow.go | 50 +-- internal/uvm/create_wcow.go | 10 +- internal/uvm/start.go | 108 +------ internal/uvm/types.go | 9 +- internal/vm/vmutils/gcs_logs.go | 167 ++++++++++ internal/vm/vmutils/gcs_logs_test.go | 438 +++++++++++++++++++++++++++ 7 files changed, 642 insertions(+), 143 deletions(-) create mode 100644 internal/vm/vmutils/gcs_logs.go create mode 100644 internal/vm/vmutils/gcs_logs_test.go diff --git a/internal/tools/uvmboot/lcow.go b/internal/tools/uvmboot/lcow.go index 47265dc3dc..710f6bbd0c 100644 --- a/internal/tools/uvmboot/lcow.go +++ b/internal/tools/uvmboot/lcow.go @@ -20,6 +20,7 @@ import ( "github.com/Microsoft/hcsshim/internal/memory" "github.com/Microsoft/hcsshim/internal/oci" "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" ) const ( @@ -247,7 +248,7 @@ func createLCOWOptions(ctx context.Context, c *cli.Context, id string) (*uvm.Opt if c.IsSet(outputHandlingArgName) { switch strings.ToLower(c.String(outputHandlingArgName)) { case "stdout": - options.OutputHandlerCreator = func(*uvm.Options) uvm.OutputHandler { + options.OutputHandlerCreator = func(string) vmutils.OutputHandler { return func(r io.Reader) { _, _ = io.Copy(os.Stdout, r) } diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 1700b3e3ac..fa8adcaeb8 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -107,28 +107,28 @@ type OptionsLCOW struct { // // It is preferred to use [UpdateBootFilesPath] to change this value and update associated fields. BootFilesPath string - KernelFile string // Filename under `BootFilesPath` for the kernel. Defaults to `kernel` - KernelDirect bool // Skip UEFI and boot directly to `kernel` - RootFSFile string // Filename under `BootFilesPath` for the UVMs root file system. Defaults to `InitrdFile` - KernelBootOptions string // Additional boot options for the kernel - UseGuestConnection bool // Whether the HCS should connect to the UVM's GCS. Defaults to true - ExecCommandLine string // The command line to exec from init. Defaults to GCS - ForwardStdout bool // Whether stdout will be forwarded from the executed program. Defaults to false - ForwardStderr bool // Whether stderr will be forwarded from the executed program. Defaults to true - OutputHandlerCreator OutputHandlerCreator `json:"-"` // Creates an [OutputHandler] that controls how output received over HVSocket from the UVM is handled. Defaults to parsing output as logrus messages - VPMemDeviceCount uint32 // Number of VPMem devices. Defaults to `DefaultVPMEMCount`. Limit at 128. If booting UVM from VHD, device 0 is taken. - VPMemSizeBytes uint64 // Size of the VPMem devices. Defaults to `DefaultVPMemSizeBytes`. - VPMemNoMultiMapping bool // Disables LCOW layer multi mapping - PreferredRootFSType PreferredRootFSType // If `KernelFile` is `InitrdFile` use `PreferredRootFSTypeInitRd`. If `KernelFile` is `VhdFile` use `PreferredRootFSTypeVHD` - EnableColdDiscardHint bool // Whether the HCS should use cold discard hints. Defaults to false - VPCIEnabled bool // Whether the kernel should enable pci - EnableScratchEncryption bool // Whether the scratch should be encrypted - DisableTimeSyncService bool // Disables the time synchronization service - HclEnabled *bool // Whether to enable the host compatibility layer - ExtraVSockPorts []uint32 // Extra vsock ports to allow - AssignedDevices []VPCIDeviceID // AssignedDevices are devices to add on pod boot - PolicyBasedRouting bool // Whether we should use policy based routing when configuring net interfaces in guest - WritableOverlayDirs bool // Whether init should create writable overlay mounts for /var and /etc + KernelFile string // Filename under `BootFilesPath` for the kernel. Defaults to `kernel` + KernelDirect bool // Skip UEFI and boot directly to `kernel` + RootFSFile string // Filename under `BootFilesPath` for the UVMs root file system. Defaults to `InitrdFile` + KernelBootOptions string // Additional boot options for the kernel + UseGuestConnection bool // Whether the HCS should connect to the UVM's GCS. Defaults to true + ExecCommandLine string // The command line to exec from init. Defaults to GCS + ForwardStdout bool // Whether stdout will be forwarded from the executed program. Defaults to false + ForwardStderr bool // Whether stderr will be forwarded from the executed program. Defaults to true + OutputHandlerCreator vmutils.OutputHandlerCreator `json:"-"` // Creates an [OutputHandler] that controls how output received over HVSocket from the UVM is handled. Defaults to parsing output as logrus messages + VPMemDeviceCount uint32 // Number of VPMem devices. Defaults to `DefaultVPMEMCount`. Limit at 128. If booting UVM from VHD, device 0 is taken. + VPMemSizeBytes uint64 // Size of the VPMem devices. Defaults to `DefaultVPMemSizeBytes`. + VPMemNoMultiMapping bool // Disables LCOW layer multi mapping + PreferredRootFSType PreferredRootFSType // If `KernelFile` is `InitrdFile` use `PreferredRootFSTypeInitRd`. If `KernelFile` is `VhdFile` use `PreferredRootFSTypeVHD` + EnableColdDiscardHint bool // Whether the HCS should use cold discard hints. Defaults to false + VPCIEnabled bool // Whether the kernel should enable pci + EnableScratchEncryption bool // Whether the scratch should be encrypted + DisableTimeSyncService bool // Disables the time synchronization service + HclEnabled *bool // Whether to enable the host compatibility layer + ExtraVSockPorts []uint32 // Extra vsock ports to allow + AssignedDevices []VPCIDeviceID // AssignedDevices are devices to add on pod boot + PolicyBasedRouting bool // Whether we should use policy based routing when configuring net interfaces in guest + WritableOverlayDirs bool // Whether init should create writable overlay mounts for /var and /etc } // NewDefaultOptionsLCOW creates the default options for a bootable version of @@ -151,7 +151,7 @@ func NewDefaultOptionsLCOW(id, owner string) *OptionsLCOW { ExecCommandLine: fmt.Sprintf("/bin/gcs -v4 -log-format json -loglevel %s", logrus.StandardLogger().Level.String()), ForwardStdout: false, ForwardStderr: true, - OutputHandlerCreator: parseLogrus, + OutputHandlerCreator: vmutils.ParseGCSLogrus, VPMemDeviceCount: DefaultVPMEMCount, VPMemSizeBytes: DefaultVPMemSizeBytes, VPMemNoMultiMapping: osversion.Get().Build < osversion.V19H1, @@ -927,7 +927,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error // We don't serialize OutputHandlerCreator so if it is missing we need to put it back to the default. if opts.OutputHandlerCreator == nil { - opts.OutputHandlerCreator = parseLogrus + opts.OutputHandlerCreator = vmutils.ParseGCSLogrus } uvm := &UtilityVM{ @@ -1002,7 +1002,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error // Create a socket that the executed program can send to. This is usually // used by GCS to send log data. if opts.ForwardStdout || opts.ForwardStderr { - uvm.outputHandler = opts.OutputHandlerCreator(opts.Options) + uvm.outputHandler = opts.OutputHandlerCreator(opts.ID) uvm.outputProcessingDone = make(chan struct{}) uvm.outputListener, err = uvm.listenVsock(linuxLogVsockPort) if err != nil { diff --git a/internal/uvm/create_wcow.go b/internal/uvm/create_wcow.go index d417b04eb7..a8657e71d7 100644 --- a/internal/uvm/create_wcow.go +++ b/internal/uvm/create_wcow.go @@ -70,9 +70,9 @@ type OptionsWCOW struct { // AdditionalRegistryKeys are Registry keys and their values to additionally add to the uVM. AdditionalRegistryKeys []hcsschema.RegistryValue - OutputHandlerCreator OutputHandlerCreator // Creates an [OutputHandler] that controls how output received over HVSocket from the UVM is handled. Defaults to parsing output as ETW Log events - LogSources string // ETW providers to be set for the logging service - ForwardLogs bool // Whether to forward logs to the host or not + OutputHandlerCreator vmutils.OutputHandlerCreator // Creates an [OutputHandler] that controls how output received over HVSocket from the UVM is handled. Defaults to parsing output as ETW Log events + LogSources string // ETW providers to be set for the logging service + ForwardLogs bool // Whether to forward logs to the host or not } func defaultConfidentialWCOWOSBootFilesPath() string { @@ -111,7 +111,7 @@ func NewDefaultOptionsWCOW(id, owner string) *OptionsWCOW { SecurityPolicyEnabled: false, }, }, - OutputHandlerCreator: parseLogrus, + OutputHandlerCreator: vmutils.ParseGCSLogrus, ForwardLogs: true, // Default to true for WCOW, and set to false for CWCOW in internal/oci/uvm.go SpecToUVMCreateOpts LogSources: "", } @@ -620,7 +620,7 @@ func CreateWCOW(ctx context.Context, opts *OptionsWCOW) (_ *UtilityVM, err error if opts.ForwardLogs { // Create a socket that the executed program can send to. This is usually // used by Log Forward Service to send log data. - uvm.outputHandler = opts.OutputHandlerCreator(opts.Options) + uvm.outputHandler = opts.OutputHandlerCreator(opts.ID) uvm.outputProcessingDone = make(chan struct{}) uvm.outputListener, err = winio.ListenHvsock(&winio.HvsockAddr{ VMID: uvm.RuntimeID(), diff --git a/internal/uvm/start.go b/internal/uvm/start.go index 89e71fda90..c468b96131 100644 --- a/internal/uvm/start.go +++ b/internal/uvm/start.go @@ -3,25 +3,15 @@ package uvm import ( - "bytes" "context" "crypto/rand" - "encoding/json" - "errors" "fmt" "io" "net" "sync" - "time" - - "github.com/sirupsen/logrus" - "golang.org/x/net/netutil" - "golang.org/x/sync/errgroup" - "golang.org/x/sys/windows" "github.com/Microsoft/hcsshim/internal/gcs" "github.com/Microsoft/hcsshim/internal/gcs/prot" - "github.com/Microsoft/hcsshim/internal/hcs" "github.com/Microsoft/hcsshim/internal/hcs/schema1" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" @@ -31,6 +21,9 @@ import ( "github.com/Microsoft/hcsshim/internal/timeout" "github.com/Microsoft/hcsshim/internal/uvm/scsi" "github.com/Microsoft/hcsshim/internal/vm/vmutils" + + "golang.org/x/net/netutil" + "golang.org/x/sync/errgroup" ) // entropyBytes is the number of bytes of random data to send to a Linux UVM @@ -42,101 +35,6 @@ import ( // generally misunderstood. const entropyBytes = 512 -type gcsLogEntryStandard struct { - Time time.Time `json:"time"` - Level logrus.Level `json:"level"` - Message string `json:"msg"` -} - -type gcsLogEntry struct { - gcsLogEntryStandard - Fields map[string]interface{} -} - -// FUTURE-jstarks: Change the GCS log format to include type information -// (e.g. by using a different encoding such as protobuf). -func (e *gcsLogEntry) UnmarshalJSON(b []byte) error { - // Default the log level to info. - e.Level = logrus.InfoLevel - if err := json.Unmarshal(b, &e.gcsLogEntryStandard); err != nil { - return err - } - if err := json.Unmarshal(b, &e.Fields); err != nil { - return err - } - // Do not allow fatal or panic level errors to propagate. - if e.Level < logrus.ErrorLevel { - e.Level = logrus.ErrorLevel - } - if e.Fields["Source"] == "ETW" { - // Windows ETW log entry - // Original ETW Event Data may have "message" or "Message" field instead of "msg" - if msg, ok := e.Fields["message"].(string); ok { - e.Message = msg - delete(e.Fields, "message") - } else if msg, ok := e.Fields["Message"].(string); ok { - e.Message = msg - delete(e.Fields, "Message") - } - } - // Clear special fields. - delete(e.Fields, "time") - delete(e.Fields, "level") - delete(e.Fields, "msg") - // Normalize floats to integers. - for k, v := range e.Fields { - if d, ok := v.(float64); ok && float64(int64(d)) == d { - e.Fields[k] = int64(d) - } - } - return nil -} - -func isDisconnectError(err error) bool { - return hcs.IsAny(err, windows.WSAECONNABORTED, windows.WSAECONNRESET) -} - -func parseLogrus(o *Options) OutputHandler { - vmid := "" - if o != nil { - vmid = o.ID - } - return func(r io.Reader) { - j := json.NewDecoder(r) - e := log.L.Dup() - fields := e.Data - for { - for k := range fields { - delete(fields, k) - } - gcsEntry := gcsLogEntry{Fields: e.Data} - err := j.Decode(&gcsEntry) - if err != nil { - // Something went wrong. Read the rest of the data as a single - // string and log it at once -- it's probably a GCS panic stack. - if !errors.Is(err, io.EOF) && !isDisconnectError(err) { - logrus.WithFields(logrus.Fields{ - logfields.UVMID: vmid, - logrus.ErrorKey: err, - }).Error("gcs log read") - } - rest, _ := io.ReadAll(io.MultiReader(j.Buffered(), r)) - rest = bytes.TrimSpace(rest) - if len(rest) != 0 { - logrus.WithFields(logrus.Fields{ - logfields.UVMID: vmid, - "stderr": string(rest), - }).Error("gcs terminated") - } - break - } - fields[logfields.UVMID] = vmid - fields["vm.time"] = gcsEntry.Time - e.Log(gcsEntry.Level, gcsEntry.Message) - } - } -} - // When using an external GCS connection it is necessary to send a ModifySettings request // for HvSocket so that the GCS can setup some registry keys that are required for running // containers inside the UVM. In non external GCS connection scenarios this is done by the diff --git a/internal/uvm/types.go b/internal/uvm/types.go index a5fbb8014b..a88d1087fb 100644 --- a/internal/uvm/types.go +++ b/internal/uvm/types.go @@ -3,12 +3,12 @@ package uvm import ( - "io" "net" "sync" "sync/atomic" "github.com/Microsoft/go-winio/pkg/guid" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "golang.org/x/sys/windows" "github.com/Microsoft/hcsshim/hcn" @@ -101,7 +101,7 @@ type UtilityVM struct { outputListener net.Listener outputProcessingDone chan struct{} - outputHandler OutputHandler + outputHandler vmutils.OutputHandler entropyListener net.Listener @@ -152,11 +152,6 @@ func (uvm *UtilityVM) ScratchEncryptionEnabled() bool { return uvm.encryptScratch } -// OutputHandler is used to process the output from the program run in the UVM. -type OutputHandler func(io.Reader) - -type OutputHandlerCreator func(*Options) OutputHandler - type WCOWBootFilesType uint8 const ( diff --git a/internal/vm/vmutils/gcs_logs.go b/internal/vm/vmutils/gcs_logs.go new file mode 100644 index 0000000000..c680dfcd76 --- /dev/null +++ b/internal/vm/vmutils/gcs_logs.go @@ -0,0 +1,167 @@ +//go:build windows + +package vmutils + +import ( + "bytes" + "encoding/json" + "io" + "time" + + "github.com/Microsoft/hcsshim/internal/hcs" + "github.com/Microsoft/hcsshim/internal/log" + "github.com/Microsoft/hcsshim/internal/logfields" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "golang.org/x/sys/windows" +) + +// OutputHandler processes the output stream from a program running in a UVM (Utility VM). +// It is responsible for reading, parsing, and handling the output data. +type OutputHandler func(io.Reader) + +// OutputHandlerCreator is a factory function that creates an OutputHandler for a specific VM. +// It takes a VM ID and returns a configured OutputHandler for that VM's output. +type OutputHandlerCreator func(string) OutputHandler + +// Ensure ParseGCSLogrus implements the OutputHandlerCreator type. +var _ OutputHandlerCreator = ParseGCSLogrus + +// ParseGCSLogrus creates an OutputHandler that parses and logs GCS (Guest Compute Service) output. +// It processes JSON-formatted log entries from the GCS and forwards them to the host's logging system. +// +// The handler performs the following operations: +// - Decodes JSON log entries from the GCS output stream +// - Enriches each log entry with VM-specific metadata (VM ID and timestamp) +// - Handles error conditions including disconnections and malformed input +// - Captures and logs panic stacks or other non-JSON output from GCS termination +// +// Parameters: +// - vmID: The unique identifier of the VM whose logs are being processed +// +// Returns: +// - OutputHandler: A configured handler function for processing the VM's log stream +func ParseGCSLogrus(vmID string) OutputHandler { + return func(r io.Reader) { + // Create JSON decoder for streaming log entries + j := json.NewDecoder(r) + + // Duplicate the base logger to avoid interfering with other log operations + e := log.L.Dup() + fields := e.Data + + // Process log entries continuously until error or EOF + for { + // Clear fields from previous iteration + clear(fields) + + // Prepare new log entry with reused fields map + gcsEntry := GCSLogEntry{Fields: fields} + err := j.Decode(&gcsEntry) + + if err != nil { + // Handle decoding errors, EOF, or disconnections + // Log the error unless it's an expected EOF or network disconnect + // (WSAECONNABORTED or WSAECONNRESET indicate expected shutdown/disconnect) + if !errors.Is(err, io.EOF) && !hcs.IsAny(err, windows.WSAECONNABORTED, windows.WSAECONNRESET) { + logrus.WithFields(logrus.Fields{ + logfields.UVMID: vmID, + logrus.ErrorKey: err, + }).Error("gcs log read") + } + + // Read any remaining data (likely a panic stack trace) + // and log it if non-empty + rest, _ := io.ReadAll(io.MultiReader(j.Buffered(), r)) + rest = bytes.TrimSpace(rest) + if len(rest) != 0 { + logrus.WithFields(logrus.Fields{ + logfields.UVMID: vmID, + "stderr": string(rest), + }).Error("gcs terminated") + } + break + } + + // Enrich log entry with VM metadata + fields[logfields.UVMID] = vmID + fields["vm.time"] = gcsEntry.Time + + // Forward the log entry to the host logger with original level and message + e.Log(gcsEntry.Level, gcsEntry.Message) + } + } +} + +// GCSLogEntryStandard represents the standard fields of a GCS log entry. +// These fields are common across all log entries and map directly to JSON fields. +type GCSLogEntryStandard struct { + Time time.Time `json:"time"` + Level logrus.Level `json:"level"` + Message string `json:"msg"` +} + +// GCSLogEntry represents a complete GCS log entry including standard fields +// and any additional custom fields that may be present in the log output. +type GCSLogEntry struct { + GCSLogEntryStandard + Fields map[string]interface{} +} + +// UnmarshalJSON implements json.Unmarshaler for GCSLogEntry. +// It performs custom unmarshaling with the following behaviors: +// - Sets default log level to Info if not specified +// - Clamps log levels to Error or above (prevents Fatal/Panic propagation) +// - Handles ETW (Event Tracing for Windows) log entries with alternate message field names +// - Removes standard fields (time, level, msg) from the Fields map to avoid duplication +// - Normalizes floating-point numbers that are whole numbers to int64 +// +// This method is optimized to minimize allocations and redundant map operations. +func (e *GCSLogEntry) UnmarshalJSON(b []byte) error { + // Default the log level to info. + e.Level = logrus.InfoLevel + + // Unmarshal standard fields first + if err := json.Unmarshal(b, &e.GCSLogEntryStandard); err != nil { + return err + } + + // Unmarshal all fields including custom ones + if err := json.Unmarshal(b, &e.Fields); err != nil { + return err + } + + // Do not allow fatal or panic level errors to propagate. + if e.Level < logrus.ErrorLevel { + e.Level = logrus.ErrorLevel + } + + // Handle ETW (Event Tracing for Windows) log entries that may have + // alternate message field names ("message" or "Message" instead of "msg") + if e.Fields["Source"] == "ETW" { + // Check for alternate message fields and use the first one found + if msg, ok := e.Fields["message"].(string); ok { + e.Message = msg + } else if msg, ok := e.Fields["Message"].(string); ok { + e.Message = msg + } + } + + // Batch delete standard and alternate fields to avoid duplication + // This is more efficient than multiple individual delete calls + fieldsToDelete := []string{"time", "level", "msg", "message", "Message"} + for _, field := range fieldsToDelete { + delete(e.Fields, field) + } + + // Normalize floating-point values that represent whole numbers to int64. + // This reduces type inconsistencies in log field values. + for k, v := range e.Fields { + if d, ok := v.(float64); ok && float64(int64(d)) == d { + e.Fields[k] = int64(d) + } + } + + return nil +} diff --git a/internal/vm/vmutils/gcs_logs_test.go b/internal/vm/vmutils/gcs_logs_test.go new file mode 100644 index 0000000000..68d17970da --- /dev/null +++ b/internal/vm/vmutils/gcs_logs_test.go @@ -0,0 +1,438 @@ +//go:build windows + +package vmutils + +import ( + "bytes" + "encoding/json" + "io" + "strings" + "testing" + "time" + + "github.com/sirupsen/logrus" +) + +func TestGCSLogEntry_UnmarshalJSON(t *testing.T) { + tests := []struct { + name string + input string + expectedLevel logrus.Level + expectedMsg string + expectedFields map[string]interface{} + wantErr bool + }{ + { + name: "basic log entry", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test message"}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "test message", + }, + { + name: "debug level becomes info", + input: `{"time":"2024-01-15T10:30:00Z","level":"debug","msg":"debug message"}`, + expectedLevel: logrus.DebugLevel, + expectedMsg: "debug message", + }, + { + name: "error level stays error", + input: `{"time":"2024-01-15T10:30:00Z","level":"error","msg":"error message"}`, + expectedLevel: logrus.ErrorLevel, + expectedMsg: "error message", + }, + { + name: "fatal level clamped to error", + input: `{"time":"2024-01-15T10:30:00Z","level":"fatal","msg":"fatal message"}`, + expectedLevel: logrus.ErrorLevel, + expectedMsg: "fatal message", + }, + { + name: "panic level clamped to error", + input: `{"time":"2024-01-15T10:30:00Z","level":"panic","msg":"panic message"}`, + expectedLevel: logrus.ErrorLevel, + expectedMsg: "panic message", + }, + { + name: "missing level defaults to info", + input: `{"time":"2024-01-15T10:30:00Z","msg":"message without level"}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "message without level", + }, + { + name: "ETW source with message field", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"","Source":"ETW","message":"etw message"}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "etw message", + }, + { + name: "ETW source with Message field (capitalized)", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"","Source":"ETW","Message":"ETW capitalized message"}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "ETW capitalized message", + }, + { + name: "custom fields preserved", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test","customField":"customValue","numericField":42}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "test", + expectedFields: map[string]interface{}{ + "customField": "customValue", + "numericField": int64(42), // whole numbers normalized to int64 + }, + }, + { + name: "floating point whole number normalized to int64", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test","count":100.0}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "test", + expectedFields: map[string]interface{}{ + "count": int64(100), + }, + }, + { + name: "floating point with decimal preserved", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test","ratio":3.14}`, + expectedLevel: logrus.InfoLevel, + expectedMsg: "test", + expectedFields: map[string]interface{}{ + "ratio": 3.14, + }, + }, + { + name: "invalid json", + input: `{invalid json`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fields := make(map[string]interface{}) + entry := GCSLogEntry{Fields: fields} + + err := json.Unmarshal([]byte(tt.input), &entry) + + if tt.wantErr { + if err == nil { + t.Error("expected error but got none") + } + return + } + + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if entry.Level != tt.expectedLevel { + t.Errorf("level = %v, want %v", entry.Level, tt.expectedLevel) + } + + if entry.Message != tt.expectedMsg { + t.Errorf("message = %q, want %q", entry.Message, tt.expectedMsg) + } + + // Check that standard fields are removed from Fields map + standardFields := []string{"time", "level", "msg", "message", "Message"} + for _, f := range standardFields { + if _, exists := entry.Fields[f]; exists { + t.Errorf("standard field %q should be removed from Fields map", f) + } + } + + // Check expected custom fields + for k, v := range tt.expectedFields { + if entry.Fields[k] != v { + t.Errorf("field %q = %v (%T), want %v (%T)", k, entry.Fields[k], entry.Fields[k], v, v) + } + } + }) + } +} + +func TestGCSLogEntry_UnmarshalJSON_Time(t *testing.T) { + input := `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test"}` + fields := make(map[string]interface{}) + entry := GCSLogEntry{Fields: fields} + + err := json.Unmarshal([]byte(input), &entry) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC) + if !entry.Time.Equal(expectedTime) { + t.Errorf("time = %v, want %v", entry.Time, expectedTime) + } +} + +func TestGCSLogEntryStandard(t *testing.T) { + input := `{"time":"2024-01-15T10:30:00Z","level":"warning","msg":"warning message"}` + var entry GCSLogEntryStandard + + err := json.Unmarshal([]byte(input), &entry) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if entry.Level != logrus.WarnLevel { + t.Errorf("level = %v, want %v", entry.Level, logrus.WarnLevel) + } + + if entry.Message != "warning message" { + t.Errorf("message = %q, want %q", entry.Message, "warning message") + } +} + +func TestParseGCSLogrus(t *testing.T) { + // Create a hook to capture log entries + hook := &testLogHook{} + originalOutput := logrus.StandardLogger().Out + logrus.SetOutput(io.Discard) + logrus.AddHook(hook) + defer func() { + logrus.SetOutput(originalOutput) + // Remove the hook by creating a new logger + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + }() + + tests := []struct { + name string + vmID string + input string + expectedCount int + validate func(t *testing.T, entries []*logrus.Entry) + }{ + { + name: "single log entry", + vmID: "test-vm-1", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test message"}` + "\n", + expectedCount: 1, + validate: func(t *testing.T, entries []*logrus.Entry) { + t.Helper() + if entries[0].Message != "test message" { + t.Errorf("message = %q, want %q", entries[0].Message, "test message") + } + if entries[0].Data["uvm-id"] != "test-vm-1" { + t.Errorf("uvm-id = %v, want %v", entries[0].Data["uvm-id"], "test-vm-1") + } + }, + }, + { + name: "multiple log entries", + vmID: "test-vm-2", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"first"}` + "\n" + `{"time":"2024-01-15T10:30:01Z","level":"warning","msg":"second"}` + "\n", + expectedCount: 2, + validate: func(t *testing.T, entries []*logrus.Entry) { + t.Helper() + if entries[0].Message != "first" { + t.Errorf("first message = %q, want %q", entries[0].Message, "first") + } + if entries[1].Message != "second" { + t.Errorf("second message = %q, want %q", entries[1].Message, "second") + } + }, + }, + { + name: "empty input", + vmID: "test-vm-3", + input: "", + expectedCount: 0, + }, + { + name: "entry with custom fields", + vmID: "test-vm-4", + input: `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test","customKey":"customValue"}` + "\n", + expectedCount: 1, + validate: func(t *testing.T, entries []*logrus.Entry) { + t.Helper() + if entries[0].Data["customKey"] != "customValue" { + t.Errorf("customKey = %v, want %v", entries[0].Data["customKey"], "customValue") + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hook.Reset() + + reader := strings.NewReader(tt.input) + handler := ParseGCSLogrus(tt.vmID) + handler(reader) + + if len(hook.Entries) != tt.expectedCount { + t.Errorf("captured %d entries, want %d", len(hook.Entries), tt.expectedCount) + } + + if tt.validate != nil && len(hook.Entries) > 0 { + tt.validate(t, hook.Entries) + } + }) + } +} + +func TestParseGCSLogrus_InvalidJSON(t *testing.T) { + hook := &testLogHook{} + originalOutput := logrus.StandardLogger().Out + logrus.SetOutput(io.Discard) + logrus.AddHook(hook) + defer func() { + logrus.SetOutput(originalOutput) + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + }() + + // Test with invalid JSON followed by valid trailing content (simulating panic stack) + input := `{"invalid json` + reader := strings.NewReader(input) + + handler := ParseGCSLogrus("test-vm") + handler(reader) + + // Should capture error log for read failure and/or termination message + // The exact behavior depends on whether there's trailing content + // At minimum, should not panic +} + +func TestParseGCSLogrus_TrailingContent(t *testing.T) { + hook := &testLogHook{} + originalOutput := logrus.StandardLogger().Out + logrus.SetOutput(io.Discard) + logrus.AddHook(hook) + defer func() { + logrus.SetOutput(originalOutput) + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + }() + + // Valid entry followed by panic stack trace + input := `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"last message"} +panic: something went wrong +goroutine 1 [running]: +main.main() + /app/main.go:10 +0x45` + + reader := strings.NewReader(input) + handler := ParseGCSLogrus("test-vm") + handler(reader) + + // Should have processed the valid entry and logged the panic stack + found := false + for _, entry := range hook.Entries { + if entry.Message == "last message" { + found = true + break + } + } + if !found { + t.Error("expected to find 'last message' entry") + } + + // Should also have logged the "gcs terminated" error with stderr content + foundTerminated := false + for _, entry := range hook.Entries { + if entry.Message == "gcs terminated" { + foundTerminated = true + stderr, ok := entry.Data["stderr"].(string) + if !ok { + t.Error("stderr field not found or not a string") + } else if !strings.Contains(stderr, "panic: something went wrong") { + t.Errorf("stderr = %q, should contain panic message", stderr) + } + break + } + } + if !foundTerminated { + t.Error("expected to find 'gcs terminated' entry") + } +} + +func TestParseGCSLogrus_VMTimeField(t *testing.T) { + hook := &testLogHook{} + originalOutput := logrus.StandardLogger().Out + logrus.SetOutput(io.Discard) + logrus.AddHook(hook) + defer func() { + logrus.SetOutput(originalOutput) + logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + }() + + input := `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"test"}` + "\n" + reader := strings.NewReader(input) + + handler := ParseGCSLogrus("test-vm") + handler(reader) + + if len(hook.Entries) != 1 { + t.Fatalf("expected 1 entry, got %d", len(hook.Entries)) + } + + vmTime, ok := hook.Entries[0].Data["vm.time"] + if !ok { + t.Error("vm.time field not found") + } + + expectedTime := time.Date(2024, 1, 15, 10, 30, 0, 0, time.UTC) + if vmTimeTyped, ok := vmTime.(time.Time); ok { + if !vmTimeTyped.Equal(expectedTime) { + t.Errorf("vm.time = %v, want %v", vmTimeTyped, expectedTime) + } + } else { + t.Errorf("vm.time is not time.Time, got %T", vmTime) + } +} + +func TestOutputHandlerCreator_Interface(t *testing.T) { + // Verify that ParseGCSLogrus satisfies the OutputHandlerCreator interface + var creator OutputHandlerCreator = ParseGCSLogrus + handler := creator("test-vm") + if handler == nil { + t.Error("ParseGCSLogrus should return a non-nil handler") + } +} + +// testLogHook is a logrus hook that captures log entries for testing +type testLogHook struct { + Entries []*logrus.Entry +} + +func (h *testLogHook) Levels() []logrus.Level { + return logrus.AllLevels +} + +func (h *testLogHook) Fire(entry *logrus.Entry) error { + h.Entries = append(h.Entries, entry) + return nil +} + +func (h *testLogHook) Reset() { + h.Entries = nil +} + +// BenchmarkGCSLogEntry_UnmarshalJSON benchmarks JSON unmarshaling performance +func BenchmarkGCSLogEntry_UnmarshalJSON(b *testing.B) { + input := []byte(`{"time":"2024-01-15T10:30:00Z","level":"info","msg":"benchmark test message","field1":"value1","field2":42,"field3":3.14}`) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + fields := make(map[string]interface{}) + entry := GCSLogEntry{Fields: fields} + _ = json.Unmarshal(input, &entry) + } +} + +// BenchmarkParseGCSLogrus benchmarks the log parsing handler +func BenchmarkParseGCSLogrus(b *testing.B) { + logrus.SetOutput(io.Discard) + + input := `{"time":"2024-01-15T10:30:00Z","level":"info","msg":"benchmark message","field":"value"} +{"time":"2024-01-15T10:30:01Z","level":"warning","msg":"second message","count":42} +{"time":"2024-01-15T10:30:02Z","level":"error","msg":"third message","error":"something failed"} +` + inputBytes := []byte(input) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + reader := bytes.NewReader(inputBytes) + handler := ParseGCSLogrus("bench-vm") + handler(reader) + } +} From 54d90441436bc8d3333dd01650e92ff27706688f Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Thu, 26 Feb 2026 13:27:32 +0530 Subject: [PATCH 2/7] refactor common constants into vmutils package Signed-off-by: Harsh Rawat --- internal/devices/assigned_devices.go | 3 +- internal/hcsoci/devices.go | 3 +- internal/oci/uvm.go | 13 ++-- internal/tools/uvmboot/lcow.go | 14 ++--- internal/uvm/constants.go | 15 ----- internal/uvm/cpugroups.go | 3 - internal/uvm/create.go | 4 +- internal/uvm/create_lcow.go | 72 +++++++---------------- internal/uvm/create_wcow.go | 2 +- internal/uvm/start.go | 11 +--- internal/uvm/types.go | 4 +- internal/uvm/virtual_device.go | 9 --- internal/uvm/vpmem_mapped.go | 3 +- internal/vm/vmutils/constants.go | 68 +++++++++++++++++++++ internal/vm/vmutils/utils.go | 19 ++++++ test/functional/lcow_uvm_test.go | 65 ++++++++++---------- test/functional/uvm_virtualdevice_test.go | 5 +- test/functional/uvm_vpmem_test.go | 4 +- 18 files changed, 173 insertions(+), 144 deletions(-) create mode 100644 internal/vm/vmutils/constants.go create mode 100644 internal/vm/vmutils/utils.go diff --git a/internal/devices/assigned_devices.go b/internal/devices/assigned_devices.go index 50a840b46d..94ab0e130b 100644 --- a/internal/devices/assigned_devices.go +++ b/internal/devices/assigned_devices.go @@ -12,6 +12,7 @@ import ( "github.com/Microsoft/hcsshim/internal/cmd" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/pkg/errors" ) @@ -40,7 +41,7 @@ func AddDevice(ctx context.Context, vm *uvm.UtilityVM, idType, deviceID string, } }() - if uvm.IsValidDeviceType(idType) { + if vmutils.IsValidDeviceType(idType) { vpci, err = vm.AssignDevice(ctx, deviceID, index, "") if err != nil { return vpci, nil, errors.Wrapf(err, "failed to assign device %s of type %s to pod %s", deviceID, idType, vm.ID()) diff --git a/internal/hcsoci/devices.go b/internal/hcsoci/devices.go index 27a53121ef..2990300b06 100644 --- a/internal/hcsoci/devices.go +++ b/internal/hcsoci/devices.go @@ -18,6 +18,7 @@ import ( "github.com/Microsoft/hcsshim/internal/oci" "github.com/Microsoft/hcsshim/internal/resources" "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/Microsoft/hcsshim/osversion" "github.com/Microsoft/hcsshim/pkg/annotations" ) @@ -174,7 +175,7 @@ func handleAssignedDevicesLCOW( // assign device into UVM and create corresponding spec windows devices for _, d := range specDevs { - if !uvm.IsValidDeviceType(d.IDType) { + if !vmutils.IsValidDeviceType(d.IDType) { return resultDevs, closers, errors.Errorf("specified device %s has unsupported type %s", d.ID, d.IDType) } diff --git a/internal/oci/uvm.go b/internal/oci/uvm.go index 9b139ee959..21d5e735c4 100644 --- a/internal/oci/uvm.go +++ b/internal/oci/uvm.go @@ -10,6 +10,7 @@ import ( "strconv" runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/Microsoft/hcsshim/pkg/annotations" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" @@ -156,7 +157,7 @@ func handleAnnotationBootFilesPath(ctx context.Context, a map[string]string, lop func handleAnnotationKernelDirectBoot(ctx context.Context, a map[string]string, lopts *uvm.OptionsLCOW) { lopts.KernelDirect = ParseAnnotationsBool(ctx, a, annotations.KernelDirectBoot, lopts.KernelDirect) if !lopts.KernelDirect { - lopts.KernelFile = uvm.KernelFile + lopts.KernelFile = vmutils.KernelFile } } @@ -166,9 +167,9 @@ func handleAnnotationPreferredRootFSType(ctx context.Context, a map[string]strin lopts.PreferredRootFSType = parseAnnotationsPreferredRootFSType(ctx, a, annotations.PreferredRootFSType, lopts.PreferredRootFSType) switch lopts.PreferredRootFSType { case uvm.PreferredRootFSTypeInitRd: - lopts.RootFSFile = uvm.InitrdFile + lopts.RootFSFile = vmutils.InitrdFile case uvm.PreferredRootFSTypeVHD: - lopts.RootFSFile = uvm.VhdFile + lopts.RootFSFile = vmutils.VhdFile } } @@ -206,7 +207,7 @@ func handleLCOWSecurityPolicy(ctx context.Context, a map[string]string, lopts *u // VPMem not supported by the enlightened kernel for SNP so set count to zero. lopts.VPMemDeviceCount = 0 // set the default GuestState filename. - lopts.GuestStateFilePath = uvm.GuestStateFile + lopts.GuestStateFilePath = vmutils.DefaultGuestStateFile lopts.KernelBootOptions = "" lopts.AllowOvercommit = false lopts.SecurityPolicyEnabled = true @@ -219,7 +220,7 @@ func handleLCOWSecurityPolicy(ctx context.Context, a map[string]string, lopts *u // The default behavior is to use kernel.vmgs and a rootfs-verity.vhd file with Merkle tree appended to ext4 filesystem. lopts.PreferredRootFSType = uvm.PreferredRootFSTypeNA lopts.RootFSFile = "" - lopts.DmVerityRootFsVhd = uvm.DefaultDmVerityRootfsVhd + lopts.DmVerityRootFsVhd = vmutils.DefaultDmVerityRootfsVhd lopts.DmVerityMode = true } @@ -286,7 +287,7 @@ func parseDevices(ctx context.Context, specWindows *specs.Windows) []uvm.VPCIDev extraDevices := []uvm.VPCIDeviceID{} for _, d := range specWindows.Devices { pciID, index := devices.GetDeviceInfoFromPath(d.ID) - if uvm.IsValidDeviceType(d.IDType) { + if vmutils.IsValidDeviceType(d.IDType) { key := uvm.NewVPCIDeviceID(pciID, index) extraDevices = append(extraDevices, key) } else { diff --git a/internal/tools/uvmboot/lcow.go b/internal/tools/uvmboot/lcow.go index 710f6bbd0c..6e2748efc6 100644 --- a/internal/tools/uvmboot/lcow.go +++ b/internal/tools/uvmboot/lcow.go @@ -197,10 +197,10 @@ func createLCOWOptions(ctx context.Context, c *cli.Context, id string) (*uvm.Opt } if c.IsSet(kernelFileArgName) { switch strings.ToLower(c.String(kernelFileArgName)) { - case uvm.KernelFile: - options.KernelFile = uvm.KernelFile - case uvm.UncompressedKernelFile: - options.KernelFile = uvm.UncompressedKernelFile + case vmutils.KernelFile: + options.KernelFile = vmutils.KernelFile + case vmutils.UncompressedKernelFile: + options.KernelFile = vmutils.UncompressedKernelFile default: return nil, unrecognizedError(c.String(kernelFileArgName), kernelFileArgName) } @@ -213,10 +213,10 @@ func createLCOWOptions(ctx context.Context, c *cli.Context, id string) (*uvm.Opt if c.IsSet(rootFSTypeArgName) { switch strings.ToLower(c.String(rootFSTypeArgName)) { case "initrd": - options.RootFSFile = uvm.InitrdFile + options.RootFSFile = vmutils.InitrdFile options.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd case "vhd": - options.RootFSFile = uvm.VhdFile + options.RootFSFile = vmutils.VhdFile options.PreferredRootFSType = uvm.PreferredRootFSTypeVHD case "none": options.RootFSFile = "" @@ -275,7 +275,7 @@ func createLCOWOptions(ctx context.Context, c *cli.Context, id string) (*uvm.Opt options.SecurityPolicyEnforcer = c.String(securityPolicyEnforcerArgName) } if c.IsSet(securityHardwareFlag) { - options.GuestStateFilePath = uvm.GuestStateFile + options.GuestStateFilePath = vmutils.DefaultGuestStateFile options.SecurityPolicyEnabled = true options.AllowOvercommit = false } diff --git a/internal/uvm/constants.go b/internal/uvm/constants.go index 5537cf13d7..01213c1699 100644 --- a/internal/uvm/constants.go +++ b/internal/uvm/constants.go @@ -5,24 +5,9 @@ package uvm import ( "errors" - "github.com/Microsoft/hcsshim/internal/memory" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" ) -const ( - // MaxVPMEMCount is the maximum number of VPMem devices that may be added to an LCOW - // utility VM - MaxVPMEMCount = 128 - - // DefaultVPMEMCount is the default number of VPMem devices that may be added to an LCOW - // utility VM if the create request doesn't specify how many. - DefaultVPMEMCount = 64 - - // DefaultVPMemSizeBytes is the default size of a VPMem device if the create request - // doesn't specify. - DefaultVPMemSizeBytes = 4 * memory.GiB // 4GB -) - var ( errNotSupported = errors.New("not supported") errBadUVMOpts = errors.New("UVM options incorrect") diff --git a/internal/uvm/cpugroups.go b/internal/uvm/cpugroups.go index f93a83ca6e..8f652647b3 100644 --- a/internal/uvm/cpugroups.go +++ b/internal/uvm/cpugroups.go @@ -10,11 +10,8 @@ import ( "github.com/Microsoft/hcsshim/internal/cpugroup" "github.com/Microsoft/hcsshim/internal/hcs/resourcepaths" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" - "github.com/Microsoft/hcsshim/osversion" ) -var errCPUGroupCreateNotSupported = fmt.Errorf("cpu group assignment on create requires a build of %d or higher", osversion.V21H1) - // ReleaseCPUGroup unsets the cpugroup from the VM func (uvm *UtilityVM) ReleaseCPUGroup(ctx context.Context) error { if err := uvm.unsetCPUGroup(ctx); err != nil { diff --git a/internal/uvm/create.go b/internal/uvm/create.go index d4965f6740..45eadde81e 100644 --- a/internal/uvm/create.go +++ b/internal/uvm/create.go @@ -169,8 +169,8 @@ func verifyOptions(_ context.Context, options interface{}) error { if opts.SCSIControllerCount > MaxSCSIControllers { return fmt.Errorf("SCSI controller count can't be more than %d", MaxSCSIControllers) } - if opts.VPMemDeviceCount > MaxVPMEMCount { - return fmt.Errorf("VPMem device count cannot be greater than %d", MaxVPMEMCount) + if opts.VPMemDeviceCount > vmutils.MaxVPMEMCount { + return fmt.Errorf("VPMem device count cannot be greater than %d", vmutils.MaxVPMEMCount) } if opts.VPMemDeviceCount > 0 { if opts.VPMemSizeBytes%4096 != 0 { diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index fa8adcaeb8..96595ae8c4 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -60,34 +60,6 @@ const ( PreferredRootFSTypeInitRd PreferredRootFSType = iota PreferredRootFSTypeVHD PreferredRootFSTypeNA - - entropyVsockPort = 1 - linuxLogVsockPort = 109 -) - -const ( - // InitrdFile is the default file name for an initrd.img used to boot LCOW. - InitrdFile = "initrd.img" - // VhdFile is the default file name for a rootfs.vhd used to boot LCOW. - VhdFile = "rootfs.vhd" - // DefaultDmVerityRootfsVhd is the default file name for a dmverity_rootfs.vhd, - // which is mounted by the GuestStateFile during boot and used as the root file - // system when booting in the SNP case. Similar to layer VHDs, the Merkle tree - // is appended after ext4 filesystem ends. - DefaultDmVerityRootfsVhd = "rootfs.vhd" - // KernelFile is the default file name for a kernel used to boot LCOW. - KernelFile = "kernel" - // UncompressedKernelFile is the default file name for an uncompressed - // kernel used to boot LCOW with KernelDirect. - UncompressedKernelFile = "vmlinux" - // GuestStateFile is the default file name for a vmgs (VM Guest State) file - // which contains the kernel and kernel command which mounts DmVerityVhdFile - // when booting in the SNP case. - GuestStateFile = "kernel.vmgs" - // UVMReferenceInfoFile is the default file name for a COSE_Sign1 - // reference UVM info, which can be made available to workload containers - // and can be used for validation purposes. - UVMReferenceInfoFile = "reference_info.cose" ) type ConfidentialLCOWOptions struct { @@ -143,17 +115,17 @@ func NewDefaultOptionsLCOW(id, owner string) *OptionsLCOW { kernelDirectSupported := osversion.Build() >= 18286 opts := &OptionsLCOW{ Options: newDefaultOptions(id, owner), - KernelFile: KernelFile, + KernelFile: vmutils.KernelFile, KernelDirect: kernelDirectSupported, - RootFSFile: InitrdFile, + RootFSFile: vmutils.InitrdFile, KernelBootOptions: "", UseGuestConnection: true, ExecCommandLine: fmt.Sprintf("/bin/gcs -v4 -log-format json -loglevel %s", logrus.StandardLogger().Level.String()), ForwardStdout: false, ForwardStderr: true, OutputHandlerCreator: vmutils.ParseGCSLogrus, - VPMemDeviceCount: DefaultVPMEMCount, - VPMemSizeBytes: DefaultVPMemSizeBytes, + VPMemDeviceCount: vmutils.DefaultVPMEMCount, + VPMemSizeBytes: vmutils.DefaultVPMemSizeBytes, VPMemNoMultiMapping: osversion.Get().Build < osversion.V19H1, PreferredRootFSType: PreferredRootFSTypeInitRd, EnableColdDiscardHint: false, @@ -163,7 +135,7 @@ func NewDefaultOptionsLCOW(id, owner string) *OptionsLCOW { ConfidentialLCOWOptions: &ConfidentialLCOWOptions{ ConfidentialCommonOptions: &ConfidentialCommonOptions{ SecurityPolicyEnabled: false, - UVMReferenceInfoFile: UVMReferenceInfoFile, + UVMReferenceInfoFile: vmutils.DefaultUVMReferenceInfoFile, }, }, } @@ -196,15 +168,15 @@ func (opts *OptionsLCOW) UpdateBootFilesPath(ctx context.Context, path string) { opts.BootFilesPath = path - if _, err := os.Stat(filepath.Join(opts.BootFilesPath, VhdFile)); err == nil { + if _, err := os.Stat(filepath.Join(opts.BootFilesPath, vmutils.VhdFile)); err == nil { // We have a rootfs.vhd in the boot files path. Use it over an initrd.img - opts.RootFSFile = VhdFile + opts.RootFSFile = vmutils.VhdFile opts.PreferredRootFSType = PreferredRootFSTypeVHD log.G(ctx).WithFields(logrus.Fields{ logfields.UVMID: opts.ID, - VhdFile: filepath.Join(opts.BootFilesPath, VhdFile), - }).Debug("updated LCOW root filesystem to " + VhdFile) + vmutils.VhdFile: filepath.Join(opts.BootFilesPath, vmutils.VhdFile), + }).Debug("updated LCOW root filesystem to " + vmutils.VhdFile) } if opts.KernelDirect { @@ -212,13 +184,13 @@ func (opts *OptionsLCOW) UpdateBootFilesPath(ctx context.Context, path string) { // Default to uncompressed if on box. NOTE: If `kernel` is already // uncompressed and simply named 'kernel' it will still be used // uncompressed automatically. - if _, err := os.Stat(filepath.Join(opts.BootFilesPath, UncompressedKernelFile)); err == nil { - opts.KernelFile = UncompressedKernelFile + if _, err := os.Stat(filepath.Join(opts.BootFilesPath, vmutils.UncompressedKernelFile)); err == nil { + opts.KernelFile = vmutils.UncompressedKernelFile log.G(ctx).WithFields(logrus.Fields{ - logfields.UVMID: opts.ID, - UncompressedKernelFile: filepath.Join(opts.BootFilesPath, UncompressedKernelFile), - }).Debug("updated LCOW kernel file to " + UncompressedKernelFile) + logfields.UVMID: opts.ID, + vmutils.UncompressedKernelFile: filepath.Join(opts.BootFilesPath, vmutils.UncompressedKernelFile), + }).Debug("updated LCOW kernel file to " + vmutils.UncompressedKernelFile) } } } @@ -242,7 +214,7 @@ func fetchProcessor(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (*hc // We can set a cpu group for the VM at creation time in recent builds. if opts.CPUGroupID != "" { if osversion.Build() < osversion.V21H1 { - return nil, errCPUGroupCreateNotSupported + return nil, vmutils.ErrCPUGroupCreateNotSupported } processor.CpuGroup = &hcsschema.CpuGroup{Id: opts.CPUGroupID} } @@ -363,7 +335,7 @@ func makeLCOWVMGSDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ } }() - dmVerityRootFsFullPath := filepath.Join(opts.BundleDirectory, DefaultDmVerityRootfsVhd) + dmVerityRootFsFullPath := filepath.Join(opts.BundleDirectory, vmutils.DefaultDmVerityRootfsVhd) if err := copyfile.CopyFile(ctx, dmVerityRootfsTemplatePath, dmVerityRootFsFullPath, true); err != nil { return nil, fmt.Errorf("failed to copy DM Verity rootfs template file: %w", err) } @@ -425,7 +397,7 @@ func makeLCOWVMGSDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ // entropyVsockPort - 1 is the entropy port, // linuxLogVsockPort - 109 used by vsockexec to log stdout/stderr logging, // 0x40000000 + 1 (LinuxGcsVsockPort + 1) is the bridge (see guestconnectiuon.go) - hvSockets := []uint32{entropyVsockPort, linuxLogVsockPort, prot.LinuxGcsVsockPort, prot.LinuxGcsVsockPort + 1} + hvSockets := []uint32{vmutils.LinuxEntropyVsockPort, vmutils.LinuxLogVsockPort, prot.LinuxGcsVsockPort, prot.LinuxGcsVsockPort + 1} hvSockets = append(hvSockets, opts.ExtraVSockPorts...) for _, whichSocket := range hvSockets { key := winio.VsockServiceID(whichSocket).String() @@ -836,18 +808,18 @@ func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcs } // Inject initial entropy over vsock during init launch. - entropyArgs := fmt.Sprintf("-e %d", entropyVsockPort) + entropyArgs := fmt.Sprintf("-e %d", vmutils.LinuxEntropyVsockPort) // With default options, run GCS with stderr pointing to the vsock port // created below in order to forward guest logs to logrus. execCmdArgs := "/bin/vsockexec" if opts.ForwardStdout { - execCmdArgs += fmt.Sprintf(" -o %d", linuxLogVsockPort) + execCmdArgs += fmt.Sprintf(" -o %d", vmutils.LinuxLogVsockPort) } if opts.ForwardStderr { - execCmdArgs += fmt.Sprintf(" -e %d", linuxLogVsockPort) + execCmdArgs += fmt.Sprintf(" -e %d", vmutils.LinuxLogVsockPort) } if opts.DisableTimeSyncService { @@ -994,7 +966,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error } // Create a socket to inject entropy during boot. - uvm.entropyListener, err = uvm.listenVsock(entropyVsockPort) + uvm.entropyListener, err = uvm.listenVsock(vmutils.LinuxEntropyVsockPort) if err != nil { return nil, err } @@ -1004,7 +976,7 @@ func CreateLCOW(ctx context.Context, opts *OptionsLCOW) (_ *UtilityVM, err error if opts.ForwardStdout || opts.ForwardStderr { uvm.outputHandler = opts.OutputHandlerCreator(opts.ID) uvm.outputProcessingDone = make(chan struct{}) - uvm.outputListener, err = uvm.listenVsock(linuxLogVsockPort) + uvm.outputListener, err = uvm.listenVsock(vmutils.LinuxLogVsockPort) if err != nil { return nil, err } diff --git a/internal/uvm/create_wcow.go b/internal/uvm/create_wcow.go index a8657e71d7..72308cf82b 100644 --- a/internal/uvm/create_wcow.go +++ b/internal/uvm/create_wcow.go @@ -238,7 +238,7 @@ func prepareCommonConfigDoc(ctx context.Context, uvm *UtilityVM, opts *OptionsWC // We can set a cpu group for the VM at creation time in recent builds. if opts.CPUGroupID != "" { if osversion.Build() < osversion.V21H1 { - return nil, errCPUGroupCreateNotSupported + return nil, vmutils.ErrCPUGroupCreateNotSupported } processor.CpuGroup = &hcsschema.CpuGroup{Id: opts.CPUGroupID} } diff --git a/internal/uvm/start.go b/internal/uvm/start.go index c468b96131..c6ba805304 100644 --- a/internal/uvm/start.go +++ b/internal/uvm/start.go @@ -26,15 +26,6 @@ import ( "golang.org/x/sync/errgroup" ) -// entropyBytes is the number of bytes of random data to send to a Linux UVM -// during boot to seed the CRNG. There is not much point in making this too -// large since the random data collected from the host is likely computed from a -// relatively small key (256 bits?), so additional bytes would not actually -// increase the entropy of the guest's pool. However, send enough to convince -// containers that there is a large amount of entropy since this idea is -// generally misunderstood. -const entropyBytes = 512 - // When using an external GCS connection it is necessary to send a ModifySettings request // for HvSocket so that the GCS can setup some registry keys that are required for running // containers inside the UVM. In non external GCS connection scenarios this is done by the @@ -101,7 +92,7 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) { return fmt.Errorf("failed to connect to entropy socket: %w", err) } defer conn.Close() - _, err = io.CopyN(conn, rand.Reader, entropyBytes) + _, err = io.CopyN(conn, rand.Reader, vmutils.LinuxEntropyBytes) if err != nil { e.WithError(err).Error("failed to write entropy") return fmt.Errorf("failed to write entropy: %w", err) diff --git a/internal/uvm/types.go b/internal/uvm/types.go index a88d1087fb..84d08c0f2b 100644 --- a/internal/uvm/types.go +++ b/internal/uvm/types.go @@ -83,8 +83,8 @@ type UtilityVM struct { vpmemMaxCount uint32 // The max number of VPMem devices. vpmemMaxSizeBytes uint64 // The max size of the layer in bytes per vPMem device. vpmemMultiMapping bool // Enable mapping multiple VHDs onto a single VPMem device - vpmemDevicesDefault [MaxVPMEMCount]*vPMemInfoDefault - vpmemDevicesMultiMapped [MaxVPMEMCount]*vPMemInfoMulti + vpmemDevicesDefault [vmutils.MaxVPMEMCount]*vPMemInfoDefault + vpmemDevicesMultiMapped [vmutils.MaxVPMEMCount]*vPMemInfoMulti // SCSI devices that are mapped into a Windows or Linux utility VM SCSIManager *scsi.Manager diff --git a/internal/uvm/virtual_device.go b/internal/uvm/virtual_device.go index 7a0518f926..22d74ccaad 100644 --- a/internal/uvm/virtual_device.go +++ b/internal/uvm/virtual_device.go @@ -16,12 +16,9 @@ import ( ) const ( - GPUDeviceIDType = "gpu" VPCILocationPathIDType = "vpci-location-path" VPCIClassGUIDTypeLegacy = "class" VPCIClassGUIDType = "vpci-class-guid" - VPCIDeviceIDTypeLegacy = "vpci" - VPCIDeviceIDType = "vpci-instance-id" ) // this is the well known channel type GUID defined by VMBUS for all assigned devices @@ -80,12 +77,6 @@ func (vpci *VPCIDevice) Release(ctx context.Context) error { return nil } -func IsValidDeviceType(deviceType string) bool { - return (deviceType == VPCIDeviceIDType) || - (deviceType == VPCIDeviceIDTypeLegacy) || - (deviceType == GPUDeviceIDType) -} - // AssignDevice assigns a vpci device to a uvm. // If the device already exists, the stored VPCIDevice's ref count is increased // and the VPCIDevice is returned. diff --git a/internal/uvm/vpmem_mapped.go b/internal/uvm/vpmem_mapped.go index 3513873e8b..9f523bae90 100644 --- a/internal/uvm/vpmem_mapped.go +++ b/internal/uvm/vpmem_mapped.go @@ -16,6 +16,7 @@ import ( "github.com/Microsoft/hcsshim/internal/memory" "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" "github.com/Microsoft/hcsshim/internal/protocol/guestresource" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" ) const ( @@ -53,7 +54,7 @@ func newVPMemMappedDevice(hostPath, uvmPath string, sizeBytes uint64, memReg mem func newPackedVPMemDevice() *vPMemInfoMulti { return &vPMemInfoMulti{ PoolAllocator: memory.NewPoolMemoryAllocator(), - maxSize: DefaultVPMemSizeBytes, + maxSize: vmutils.DefaultVPMemSizeBytes, mappings: make(map[string]*mappedDeviceInfo), maxMappedDeviceCount: MaxMappedDeviceCount, } diff --git a/internal/vm/vmutils/constants.go b/internal/vm/vmutils/constants.go new file mode 100644 index 0000000000..9a85c2b774 --- /dev/null +++ b/internal/vm/vmutils/constants.go @@ -0,0 +1,68 @@ +//go:build windows + +package vmutils + +import ( + "fmt" + + "github.com/Microsoft/hcsshim/internal/memory" + "github.com/Microsoft/hcsshim/osversion" +) + +const ( + // MaxVPMEMCount is the maximum number of VPMem devices that may be added to an LCOW + // utility VM. + MaxVPMEMCount = 128 + + // DefaultVPMEMCount is the default number of VPMem devices that may be added to an LCOW + // utility VM if the create request doesn't specify how many. + DefaultVPMEMCount = 64 + + // DefaultVPMemSizeBytes is the default size of a VPMem device if the create request + // doesn't specify. + DefaultVPMemSizeBytes = 4 * memory.GiB // 4GB + + // DefaultDmVerityRootfsVhd is the default file name for a dm-verity rootfs VHD, + // mounted by the GuestStateFile during boot and used as the root file system when + // booting in the SNP case. Similar to layer VHDs, the Merkle tree is appended after + // the ext4 filesystem ends. + DefaultDmVerityRootfsVhd = "rootfs.vhd" + // DefaultGuestStateFile is the default file name for a VMGS (VM Guest State) file, + // which contains the kernel and kernel command that mounts DmVerityVhdFile when + // booting in the SNP case. + DefaultGuestStateFile = "kernel.vmgs" + // DefaultUVMReferenceInfoFile is the default file name for a COSE_Sign1 reference + // UVM info file, which can be made available to workload containers and used for + // validation purposes. + DefaultUVMReferenceInfoFile = "reference_info.cose" + + // InitrdFile is the default file name for an initrd.img used to boot LCOW. + InitrdFile = "initrd.img" + // VhdFile is the default file name for a rootfs.vhd used to boot LCOW. + VhdFile = "rootfs.vhd" + // KernelFile is the default file name for a kernel used to boot LCOW. + KernelFile = "kernel" + // UncompressedKernelFile is the default file name for an uncompressed + // kernel used to boot LCOW with KernelDirect. + UncompressedKernelFile = "vmlinux" + + // LinuxEntropyVsockPort is the vsock port used to inject initial entropy + // into the LCOW guest VM during boot. + LinuxEntropyVsockPort = 1 + // LinuxLogVsockPort is the vsock port used by the GCS (Guest Compute Service) + // to forward stdout/stderr log data from the guest to the host. + LinuxLogVsockPort = 109 + // LinuxEntropyBytes is the number of bytes of random data to send to a Linux UVM + // during boot to seed the CRNG. There is not much point in making this too + // large since the random data collected from the host is likely computed from a + // relatively small key (256 bits?), so additional bytes would not actually + // increase the entropy of the guest's pool. However, send enough to convince + // containers that there is a large amount of entropy since this idea is + // generally misunderstood. + LinuxEntropyBytes = 512 +) + +var ( + // ErrCPUGroupCreateNotSupported is returned when a create request specifies a CPU group but the host build doesn't support it. + ErrCPUGroupCreateNotSupported = fmt.Errorf("cpu group assignment on create requires a build of %d or higher", osversion.V21H1) +) diff --git a/internal/vm/vmutils/utils.go b/internal/vm/vmutils/utils.go new file mode 100644 index 0000000000..d0e7af16b2 --- /dev/null +++ b/internal/vm/vmutils/utils.go @@ -0,0 +1,19 @@ +//go:build windows + +package vmutils + +const ( + // GpuDeviceIDType is the assigned device ID type for GPU devices. + GpuDeviceIDType = "gpu" + // VPCIDeviceIDTypeLegacy is the legacy assigned device ID type for vPCI devices. + VPCIDeviceIDTypeLegacy = "vpci" + // VPCIDeviceIDType is the assigned device ID type for vPCI instance IDs. + VPCIDeviceIDType = "vpci-instance-id" +) + +// IsValidDeviceType returns true if the device type is valid i.e. supported by the runtime. +func IsValidDeviceType(deviceType string) bool { + return (deviceType == VPCIDeviceIDType) || + (deviceType == VPCIDeviceIDTypeLegacy) || + (deviceType == GpuDeviceIDType) +} diff --git a/test/functional/lcow_uvm_test.go b/test/functional/lcow_uvm_test.go index a703a4b969..900587303f 100644 --- a/test/functional/lcow_uvm_test.go +++ b/test/functional/lcow_uvm_test.go @@ -14,6 +14,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/Microsoft/hcsshim/osversion" testcmd "github.com/Microsoft/hcsshim/test/internal/cmd" @@ -58,12 +59,12 @@ func TestLCOW_UVM_KernelArgs(t *testing.T) { opts.VPMemDeviceCount = 0 opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd - opts.RootFSFile = uvm.InitrdFile + opts.RootFSFile = vmutils.InitrdFile opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile }, - wantArgs: []string{fmt.Sprintf(`initrd=/%s`, uvm.InitrdFile), + wantArgs: []string{fmt.Sprintf(`initrd=/%s`, vmutils.InitrdFile), `8250_core.nr_uarts=0`, fmt.Sprintf(`nr_cpus=%d`, numCPU), `panic=-1`, `quiet`, `pci=off`}, notWantArgs: []string{`root=`, `rootwait`, `init=`, `/dev/pmem`, `/dev/sda`, `console=`}, wantDmesg: []string{`initrd`, `initramfs`}, @@ -75,10 +76,10 @@ func TestLCOW_UVM_KernelArgs(t *testing.T) { opts.VPMemDeviceCount = 0 opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd - opts.RootFSFile = uvm.InitrdFile + opts.RootFSFile = vmutils.InitrdFile opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile }, wantArgs: []string{`8250_core.nr_uarts=0`, fmt.Sprintf(`nr_cpus=%d`, numCPU), `panic=-1`, `quiet`, `pci=off`}, notWantArgs: []string{`root=`, `rootwait`, `init=`, `/dev/pmem`, `/dev/sda`, `console=`}, @@ -96,10 +97,10 @@ func TestLCOW_UVM_KernelArgs(t *testing.T) { opts.VPMemDeviceCount = 1 opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile }, wantArgs: []string{`root=/dev/pmem0`, `rootwait`, `init=/init`, `8250_core.nr_uarts=0`, fmt.Sprintf(`nr_cpus=%d`, numCPU), `panic=-1`, `quiet`, `pci=off`}, @@ -113,10 +114,10 @@ func TestLCOW_UVM_KernelArgs(t *testing.T) { opts.VPMemDeviceCount = 0 opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile }, wantArgs: []string{`root=/dev/sda`, `rootwait`, `init=/init`, `8250_core.nr_uarts=0`, fmt.Sprintf(`nr_cpus=%d`, numCPU), `panic=-1`, `quiet`, `pci=off`}, @@ -130,10 +131,10 @@ func TestLCOW_UVM_KernelArgs(t *testing.T) { opts.VPMemDeviceCount = 1 opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile }, wantArgs: []string{`root=/dev/pmem0`, `rootwait`, `init=/init`, `8250_core.nr_uarts=0`, fmt.Sprintf(`nr_cpus=%d`, numCPU), `panic=-1`, `quiet`, `pci=off`}, @@ -147,10 +148,10 @@ func TestLCOW_UVM_KernelArgs(t *testing.T) { opts.VPMemDeviceCount = 0 opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile }, wantArgs: []string{`root=/dev/sda`, `rootwait`, `init=/init`, `8250_core.nr_uarts=0`, fmt.Sprintf(`nr_cpus=%d`, numCPU), `panic=-1`, `quiet`, `pci=off`}, @@ -210,9 +211,9 @@ func TestLCOW_UVM_Boot(t *testing.T) { name: "vPMEM no kernel direct initrd", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile - opts.RootFSFile = uvm.InitrdFile + opts.RootFSFile = vmutils.InitrdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd opts.VPMemDeviceCount = 32 @@ -222,36 +223,36 @@ func TestLCOW_UVM_Boot(t *testing.T) { name: "vPMEM kernel direct initrd", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile - opts.RootFSFile = uvm.InitrdFile + opts.RootFSFile = vmutils.InitrdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd - opts.VPMemDeviceCount = uvm.DefaultVPMEMCount + opts.VPMemDeviceCount = vmutils.DefaultVPMEMCount }, }, { name: "vPMEM no kernel direct VHD", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.VPMemDeviceCount = uvm.DefaultVPMEMCount + opts.VPMemDeviceCount = vmutils.DefaultVPMEMCount }, }, { name: "vPMEM kernel direct VHD", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile - opts.VPMemDeviceCount = uvm.DefaultVPMEMCount + opts.VPMemDeviceCount = vmutils.DefaultVPMEMCount }, }, } { @@ -299,9 +300,9 @@ func TestLCOW_UVM_WritableOverlay(t *testing.T) { name: "no kernel direct initrd", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile - opts.RootFSFile = uvm.InitrdFile + opts.RootFSFile = vmutils.InitrdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd }, }, @@ -309,9 +310,9 @@ func TestLCOW_UVM_WritableOverlay(t *testing.T) { name: "kernel direct initrd", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile - opts.RootFSFile = uvm.InitrdFile + opts.RootFSFile = vmutils.InitrdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd }, }, @@ -319,9 +320,9 @@ func TestLCOW_UVM_WritableOverlay(t *testing.T) { name: "no kernel direct VHD", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = false - opts.KernelFile = uvm.KernelFile + opts.KernelFile = vmutils.KernelFile - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD }, }, @@ -329,10 +330,10 @@ func TestLCOW_UVM_WritableOverlay(t *testing.T) { name: "kernel direct VHD", optsFn: func(opts *uvm.OptionsLCOW) { opts.KernelDirect = true - opts.KernelFile = uvm.UncompressedKernelFile + opts.KernelFile = vmutils.UncompressedKernelFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeVHD - opts.RootFSFile = uvm.VhdFile + opts.RootFSFile = vmutils.VhdFile }, }, } { diff --git a/test/functional/uvm_virtualdevice_test.go b/test/functional/uvm_virtualdevice_test.go index 624b6e49b1..0157ddd9f8 100644 --- a/test/functional/uvm_virtualdevice_test.go +++ b/test/functional/uvm_virtualdevice_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/Microsoft/hcsshim/osversion" "github.com/Microsoft/hcsshim/test/pkg/require" @@ -52,8 +53,8 @@ func TestVirtualDevice(t *testing.T) { opts.AllowOvercommit = false opts.KernelDirect = false opts.VPMemDeviceCount = 0 - opts.KernelFile = uvm.KernelFile - opts.RootFSFile = uvm.InitrdFile + opts.KernelFile = vmutils.KernelFile + opts.RootFSFile = vmutils.InitrdFile opts.PreferredRootFSType = uvm.PreferredRootFSTypeInitRd // create test uvm and ensure we can assign and remove the device diff --git a/test/functional/uvm_vpmem_test.go b/test/functional/uvm_vpmem_test.go index 2fee325f71..17f2ec87f6 100644 --- a/test/functional/uvm_vpmem_test.go +++ b/test/functional/uvm_vpmem_test.go @@ -9,7 +9,7 @@ import ( "testing" "github.com/Microsoft/hcsshim/internal/copyfile" - "github.com/Microsoft/hcsshim/internal/uvm" + "github.com/Microsoft/hcsshim/internal/vm/vmutils" "github.com/Microsoft/hcsshim/osversion" "github.com/Microsoft/hcsshim/test/internal/util" @@ -29,7 +29,7 @@ func TestVPMEM(t *testing.T) { u := testuvm.CreateAndStartLCOW(ctx, t, t.Name()) defer u.Close() - var iterations uint32 = uvm.MaxVPMEMCount + var iterations uint32 = vmutils.MaxVPMEMCount // Use layer.vhd from the alpine image as something to add tempDir := t.TempDir() From b4d8a922255cecd139076852e04aa22c63d59619 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 28 Feb 2026 02:12:57 +0530 Subject: [PATCH 3/7] capture stopped time for the UVM WaitSandbox API expects the exit code and exit timestamp in it's response. Therefore, adding and capturing the same to the actual compute system struct. Signed-off-by: Harsh Rawat --- internal/hcs/system.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/hcs/system.go b/internal/hcs/system.go index b1597466f6..f6400fd9c9 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -37,6 +37,7 @@ type System struct { exitError error os, typ, owner string startTime time.Time + stopTime time.Time } var _ cow.Container = &System{} @@ -292,6 +293,7 @@ func (computeSystem *System) waitBackground() { } computeSystem.closedWaitOnce.Do(func() { computeSystem.waitError = err + computeSystem.stopTime = time.Now() close(computeSystem.waitBlock) }) oc.SetSpanStatus(span, err) @@ -871,3 +873,7 @@ func (computeSystem *System) Modify(ctx context.Context, config interface{}) err return nil } + +func (computeSystem *System) StoppedTime() time.Time { + return computeSystem.stopTime +} From 902fff96e38cb4016e1fa98a67904ecf177aece0 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 28 Feb 2026 02:18:01 +0530 Subject: [PATCH 4/7] refactor guest connection management for reusability across shims There were few issues in the initial draft of guest manager code- - Guest connection listener was created during New method and then used in CreateConnection. This can be simplified by creating the listener prior to start accepting connections on the same. - Instead of hardcoding the GCS Service GUID, it can be made into a configurable parameter so that it can be used across shims. - Updation of HvSocket was needed only for WCOW and hence that responsibility can be deferred to the caller and hence hvSocket API was introduced. Signed-off-by: Harsh Rawat --- internal/vm/guestmanager/guest.go | 75 ++++++++++------------------ internal/vm/guestmanager/hvsocket.go | 12 ++++- internal/vm/guestmanager/manager.go | 4 +- 3 files changed, 39 insertions(+), 52 deletions(-) diff --git a/internal/vm/guestmanager/guest.go b/internal/vm/guestmanager/guest.go index 8fb52a042e..c1d9656e3b 100644 --- a/internal/vm/guestmanager/guest.go +++ b/internal/vm/guestmanager/guest.go @@ -5,16 +5,14 @@ package guestmanager import ( "context" "fmt" - "net" "github.com/Microsoft/hcsshim/internal/gcs" - "github.com/Microsoft/hcsshim/internal/gcs/prot" - hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/logfields" "github.com/Microsoft/hcsshim/internal/vm/vmmanager" "github.com/Microsoft/go-winio" + "github.com/Microsoft/go-winio/pkg/guid" "github.com/sirupsen/logrus" ) @@ -28,31 +26,20 @@ type Guest struct { vmmanager.LifetimeManager vmmanager.VMSocketManager } - - gcListener net.Listener // The listener for the GCS connection - gc *gcs.GuestConnection // The GCS connection + // gc is the active GCS connection to the guest. + // It will be nil if no connection is active. + gc *gcs.GuestConnection } // New creates a new Guest Manager. func New(ctx context.Context, uvm interface { vmmanager.LifetimeManager vmmanager.VMSocketManager -}) (*Guest, error) { - gm := &Guest{ +}) *Guest { + return &Guest{ log: log.G(ctx).WithField(logfields.UVMID, uvm.ID()), uvm: uvm, } - - conn, err := winio.ListenHvsock(&winio.HvsockAddr{ - VMID: uvm.RuntimeID(), - ServiceID: winio.VsockServiceID(prot.LinuxGcsVsockPort), - }) - if err != nil { - return nil, fmt.Errorf("failed to listen for guest connection: %w", err) - } - gm.gcListener = conn - - return gm, nil } // ConfigOption defines a function that modifies the GCS connection config. @@ -67,65 +54,55 @@ func WithInitializationState(state *gcs.InitialGuestState) ConfigOption { } // CreateConnection accepts the GCS connection and performs initial setup. -func (gm *Guest) CreateConnection(ctx context.Context, opts ...ConfigOption) error { - // 1. Accept the connection - conn, err := AcceptConnection(ctx, gm.uvm, gm.gcListener, true) +func (gm *Guest) CreateConnection(ctx context.Context, GCSServiceID guid.GUID, opts ...ConfigOption) error { + // The guest needs to connect to predefined GCS port. + // The host must already be listening on these port before the guest attempts to connect, + // otherwise the connection would fail. + vmConn, err := winio.ListenHvsock(&winio.HvsockAddr{ + VMID: gm.uvm.RuntimeID(), + ServiceID: GCSServiceID, + }) + if err != nil { + return fmt.Errorf("failed to listen for guest connection: %w", err) + } + + // Accept the connection + conn, err := vmmanager.AcceptConnection(ctx, gm.uvm, vmConn, true) if err != nil { return fmt.Errorf("failed to connect to GCS: %w", err) } - gm.gcListener = nil // Listener is closed/consumed by AcceptConnection on success. - // 2. Create the default base configuration + // Create the default base configuration gcc := &gcs.GuestConnectionConfig{ Conn: conn, Log: gm.log, // Ensure gm has a logger field IoListen: gcs.HvsockIoListen(gm.uvm.RuntimeID()), } - // 3. Apply all passed options. + // Apply all passed options. for _, opt := range opts { if err := opt(gcc); err != nil { return fmt.Errorf("failed to apply GCS config option: %w", err) } } - // 4. Start the GCS protocol + // Start the GCS protocol gm.gc, err = gcc.Connect(ctx, true) if err != nil { return fmt.Errorf("failed to connect to GCS: %w", err) } - // 5. Initial setup required for external GCS connection. - hvsocketAddress := &hcsschema.HvSocketAddress{ - LocalAddress: gm.uvm.RuntimeID().String(), - ParentAddress: prot.WindowsGcsHvHostID.String(), - } - - err = gm.updateHvSocketAddress(ctx, hvsocketAddress) - if err != nil { - return fmt.Errorf("failed to create GCS connection: %w", err) - } - return nil } // CloseConnection closes any active GCS connection and listener. func (gm *Guest) CloseConnection() error { - var firstErr error + var err error if gm.gc != nil { - if err := gm.gc.Close(); err != nil { - firstErr = err - } + err = gm.gc.Close() gm.gc = nil } - if gm.gcListener != nil { - if err := gm.gcListener.Close(); err != nil && firstErr == nil { - firstErr = err - } - gm.gcListener = nil - } - - return firstErr + return err } diff --git a/internal/vm/guestmanager/hvsocket.go b/internal/vm/guestmanager/hvsocket.go index 327dd515f9..1108ec1cd6 100644 --- a/internal/vm/guestmanager/hvsocket.go +++ b/internal/vm/guestmanager/hvsocket.go @@ -11,8 +11,16 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" ) -// updateHvSocketAddress configures the HvSocket address for GCS communication. -func (gm *Guest) updateHvSocketAddress(ctx context.Context, settings *hcsschema.HvSocketAddress) error { +// HVSocketManager exposes the hvSocket operations in the Guest. +type HVSocketManager interface { + UpdateHvSocketAddress(ctx context.Context, settings *hcsschema.HvSocketAddress) error +} + +var _ HVSocketManager = (*Guest)(nil) + +// UpdateHvSocketAddress updates the Hyper-V socket address settings for the VM. +// These address settings are applied by the GCS every time the VM starts or restores. +func (gm *Guest) UpdateHvSocketAddress(ctx context.Context, settings *hcsschema.HvSocketAddress) error { conSetupReq := &hcsschema.ModifySettingRequest{ GuestRequest: guestrequest.ModificationRequest{ RequestType: guestrequest.RequestTypeUpdate, diff --git a/internal/vm/guestmanager/manager.go b/internal/vm/guestmanager/manager.go index f347aa4532..137f8005df 100644 --- a/internal/vm/guestmanager/manager.go +++ b/internal/vm/guestmanager/manager.go @@ -8,13 +8,15 @@ import ( "github.com/Microsoft/hcsshim/internal/cow" "github.com/Microsoft/hcsshim/internal/gcs" + + "github.com/Microsoft/go-winio/pkg/guid" ) // Manager provides access to guest operations over the GCS connection. // Call CreateConnection before invoking other methods. type Manager interface { // CreateConnection accepts the GCS connection and performs initial setup. - CreateConnection(ctx context.Context, opts ...ConfigOption) error + CreateConnection(ctx context.Context, GCSServiceID guid.GUID, opts ...ConfigOption) error // CloseConnection closes the GCS connection and listener. CloseConnection() error // Capabilities returns the guest's declared capabilities. From 9b9d3a8f1606ac0a4ca40130f95227d0e701e024 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 28 Feb 2026 02:18:54 +0530 Subject: [PATCH 5/7] Added a reusable utility method to parse UVM Reference Info file Signed-off-by: Harsh Rawat --- internal/vm/vmutils/utils.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/internal/vm/vmutils/utils.go b/internal/vm/vmutils/utils.go index d0e7af16b2..4876271ec1 100644 --- a/internal/vm/vmutils/utils.go +++ b/internal/vm/vmutils/utils.go @@ -2,6 +2,16 @@ package vmutils +import ( + "context" + "encoding/base64" + "fmt" + "os" + "path/filepath" + + "github.com/Microsoft/hcsshim/internal/log" +) + const ( // GpuDeviceIDType is the assigned device ID type for GPU devices. GpuDeviceIDType = "gpu" @@ -17,3 +27,22 @@ func IsValidDeviceType(deviceType string) bool { (deviceType == VPCIDeviceIDTypeLegacy) || (deviceType == GpuDeviceIDType) } + +// ParseUVMReferenceInfo reads the UVM reference info file, and base64 encodes the content if it exists. +func ParseUVMReferenceInfo(ctx context.Context, referenceRoot, referenceName string) (string, error) { + if referenceName == "" { + return "", nil + } + + fullFilePath := filepath.Join(referenceRoot, referenceName) + content, err := os.ReadFile(fullFilePath) + if err != nil { + if os.IsNotExist(err) { + log.G(ctx).WithField("filePath", fullFilePath).Debug("UVM reference info file not found") + return "", nil + } + return "", fmt.Errorf("failed to read UVM reference info file: %w", err) + } + + return base64.StdEncoding.EncodeToString(content), nil +} From ec90c3d8dd80eb76d636c8e3b912e3e3ef5e2b92 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 28 Feb 2026 02:22:01 +0530 Subject: [PATCH 6/7] refactored utils.go to vmmanager and added StoppedTime API AcceptConnection method was incorrectly part of guestmanager package. Moreover, we needed stop time in SandboxWait API and hence the same is added to the manager. Signed-off-by: Harsh Rawat --- internal/vm/vmmanager/lifetime.go | 15 ++++++++++++++- internal/vm/{guestmanager => vmmanager}/utils.go | 15 +++------------ 2 files changed, 17 insertions(+), 13 deletions(-) rename internal/vm/{guestmanager => vmmanager}/utils.go (71%) diff --git a/internal/vm/vmmanager/lifetime.go b/internal/vm/vmmanager/lifetime.go index e81d332a2b..dea1203955 100644 --- a/internal/vm/vmmanager/lifetime.go +++ b/internal/vm/vmmanager/lifetime.go @@ -5,6 +5,7 @@ package vmmanager import ( "context" "fmt" + "time" "github.com/Microsoft/go-winio/pkg/guid" hcsschema "github.com/Microsoft/hcsshim/internal/hcs/schema2" @@ -24,6 +25,7 @@ type LifetimeManager interface { Start(ctx context.Context) error // Terminate will forcefully power off the Utility VM. + // It waits for the UVM to exit and returns any encountered errors. Terminate(ctx context.Context) error // Close terminates and releases resources associated with the utility VM. @@ -47,6 +49,9 @@ type LifetimeManager interface { // PropertiesV2 returns the properties of the Utility VM. PropertiesV2(ctx context.Context, types ...hcsschema.PropertyType) (*hcsschema.Properties, error) + // StoppedTime returns the time when the Utility VM entered the stopped state. + StoppedTime() time.Time + // ExitError will return any error if the Utility VM exited unexpectedly, or if the Utility VM experienced an // error after Wait returned, it will return the wait error. ExitError() error @@ -72,11 +77,14 @@ func (uvm *UtilityVM) Start(ctx context.Context) (err error) { return nil } -// Terminate terminates the utility VM. +// Terminate terminates the utility VM and waits for it to exit. func (uvm *UtilityVM) Terminate(ctx context.Context) error { if err := uvm.cs.Terminate(ctx); err != nil { return fmt.Errorf("failed to terminate utility VM: %w", err) } + if err := uvm.Wait(ctx); err != nil { + return fmt.Errorf("failed to wait for utility VM termination: %w", err) + } return nil } @@ -127,6 +135,11 @@ func (uvm *UtilityVM) PropertiesV2(ctx context.Context, types ...hcsschema.Prope return props, nil } +// StoppedTime returns the time when the utility VM entered the stopped state. +func (uvm *UtilityVM) StoppedTime() time.Time { + return uvm.cs.StoppedTime() +} + // ExitError returns the exit error of the utility VM, if it has exited. func (uvm *UtilityVM) ExitError() error { return uvm.cs.ExitError() diff --git a/internal/vm/guestmanager/utils.go b/internal/vm/vmmanager/utils.go similarity index 71% rename from internal/vm/guestmanager/utils.go rename to internal/vm/vmmanager/utils.go index 8c3cf03822..ad247352c8 100644 --- a/internal/vm/guestmanager/utils.go +++ b/internal/vm/vmmanager/utils.go @@ -1,17 +1,15 @@ //go:build windows -package guestmanager +package vmmanager import ( "context" "net" - - "github.com/Microsoft/hcsshim/internal/vm/vmmanager" ) // AcceptConnection accepts a connection and then closes a listener. // It monitors ctx.Done() and uvm.Wait() for early termination. -func AcceptConnection(ctx context.Context, uvm vmmanager.LifetimeManager, l net.Listener, closeConnection bool) (net.Conn, error) { +func AcceptConnection(ctx context.Context, uvm LifetimeManager, l net.Listener, closeConnection bool) (net.Conn, error) { // Channel to capture the accept result type acceptResult struct { conn net.Conn @@ -43,18 +41,11 @@ func AcceptConnection(ctx context.Context, uvm vmmanager.LifetimeManager, l net. } _ = l.Close() - res := <-resultCh - if res.err == nil { - return res.conn, res.err - } // Prefer context error to VM error to accept error in order to return the // most useful error. if ctx.Err() != nil { return nil, ctx.Err() } - if uvm.ExitError() != nil { - return nil, uvm.ExitError() - } - return nil, res.err + return nil, uvm.ExitError() } From 2e7b58a24971afb3709ea07a0aef8fe5b04fa238 Mon Sep 17 00:00:00 2001 From: Harsh Rawat Date: Sat, 28 Feb 2026 18:56:51 +0530 Subject: [PATCH 7/7] add started time API to hcs system and vmmanager Start time is needed to be returned in the StartSandbox API and SandboxStatus API. Therefore, we need the plumbing for the same. Signed-off-by: Harsh Rawat --- internal/hcs/system.go | 4 ++++ internal/vm/vmmanager/lifetime.go | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/internal/hcs/system.go b/internal/hcs/system.go index f6400fd9c9..823e27b0b7 100644 --- a/internal/hcs/system.go +++ b/internal/hcs/system.go @@ -877,3 +877,7 @@ func (computeSystem *System) Modify(ctx context.Context, config interface{}) err func (computeSystem *System) StoppedTime() time.Time { return computeSystem.stopTime } + +func (computeSystem *System) StartedTime() time.Time { + return computeSystem.startTime +} diff --git a/internal/vm/vmmanager/lifetime.go b/internal/vm/vmmanager/lifetime.go index dea1203955..b2cc737b53 100644 --- a/internal/vm/vmmanager/lifetime.go +++ b/internal/vm/vmmanager/lifetime.go @@ -49,6 +49,9 @@ type LifetimeManager interface { // PropertiesV2 returns the properties of the Utility VM. PropertiesV2(ctx context.Context, types ...hcsschema.PropertyType) (*hcsschema.Properties, error) + // StartedTime returns the time when the Utility VM entered the running state. + StartedTime() time.Time + // StoppedTime returns the time when the Utility VM entered the stopped state. StoppedTime() time.Time @@ -135,6 +138,11 @@ func (uvm *UtilityVM) PropertiesV2(ctx context.Context, types ...hcsschema.Prope return props, nil } +// StartedTime returns the time when the utility VM entered the running state. +func (uvm *UtilityVM) StartedTime() time.Time { + return uvm.cs.StartedTime() +} + // StoppedTime returns the time when the utility VM entered the stopped state. func (uvm *UtilityVM) StoppedTime() time.Time { return uvm.cs.StoppedTime()