Skip to content

Commit dafcc65

Browse files
author
Manish Ranjan Mahanta
committed
Address review comments
1 parent 2364b8c commit dafcc65

File tree

8 files changed

+80
-61
lines changed

8 files changed

+80
-61
lines changed

‎internal/gcs/bridge.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ func (brdg *bridge) recvLoop() error {
332332
}
333333

334334
case prot.MsgTypeNotify:
335-
if typ != prot.NotifyContainer|prot.MsgTypeNotify {
335+
if typ != prot.NotifyContainer|prot.ComputeSystem|prot.MsgTypeNotify {
336336
return fmt.Errorf("bridge received unknown unknown notification message %s", typ)
337337
}
338338
var ntf prot.ContainerNotification

‎internal/gcs/bridge_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func notifyThroughBridge(t *testing.T, typ prot.MsgType, msg interface{}, fn not
179179
func TestBridgeNotify(t *testing.T) {
180180
ntf := &prot.ContainerNotification{Operation: "testing"}
181181
recvd := false
182-
err := notifyThroughBridge(t, prot.MsgTypeNotify|prot.NotifyContainer, ntf, func(nntf *prot.ContainerNotification) error {
182+
err := notifyThroughBridge(t, prot.MsgTypeNotify|prot.ComputeSystem|prot.NotifyContainer, ntf, func(nntf *prot.ContainerNotification) error {
183183
if !reflect.DeepEqual(ntf, nntf) {
184184
t.Errorf("%+v != %+v", ntf, nntf)
185185
}
@@ -197,7 +197,7 @@ func TestBridgeNotify(t *testing.T) {
197197
func TestBridgeNotifyFailure(t *testing.T) {
198198
ntf := &prot.ContainerNotification{Operation: "testing"}
199199
errMsg := "notify should have failed"
200-
err := notifyThroughBridge(t, prot.MsgTypeNotify|prot.NotifyContainer, ntf, func(nntf *prot.ContainerNotification) error {
200+
err := notifyThroughBridge(t, prot.MsgTypeNotify|prot.ComputeSystem|prot.NotifyContainer, ntf, func(nntf *prot.ContainerNotification) error {
201201
return errors.New(errMsg)
202202
})
203203
if err == nil || !strings.Contains(err.Error(), errMsg) {

‎internal/gcs/guestconnection.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func (gc *GuestConnection) ModifyServiceSettings(ctx context.Context, serviceTyp
191191

192192
req := prot.ServiceModificationRequest{
193193
RequestBase: makeRequest(ctx, nullContainerID),
194-
PropertyType: serviceType.String(),
194+
PropertyType: string(serviceType),
195195
Settings: settings,
196196
}
197197
var resp prot.ResponseBase

‎internal/gcs/guestconnection_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func simpleGcsLoop(t *testing.T, rw io.ReadWriter) error {
131131
return err
132132
}
133133
time.Sleep(50 * time.Millisecond)
134-
err = sendJSON(t, rw, prot.MsgType(prot.MsgTypeNotify|prot.NotifyContainer), 0, &prot.ContainerNotification{
134+
err = sendJSON(t, rw, prot.MsgType(prot.MsgTypeNotify|prot.ComputeSystem|prot.NotifyContainer), 0, &prot.ContainerNotification{
135135
RequestBase: prot.RequestBase{
136136
ContainerID: req.ContainerID,
137137
},

‎internal/gcs/prot/protocol.go‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,12 @@ const (
124124
RPCModifyServiceSettings RPCProc = ComputeService | (iota+1)<<8 | 1
125125
)
126126

127-
type ServiceModifyPropertyType uint32
127+
type ServiceModifyPropertyType string
128128

129129
const (
130-
LogForwardService ServiceModifyPropertyType = iota
130+
LogForwardService = ServiceModifyPropertyType("LogForwardService")
131131
)
132132

133-
func (m ServiceModifyPropertyType) String() string {
134-
switch m {
135-
case LogForwardService:
136-
return "LogForwardService"
137-
default:
138-
return fmt.Sprintf("UnknownModifyServiceType(%d)", m)
139-
}
140-
}
141-
142133
func (rpc RPCProc) String() string {
143134
switch rpc {
144135
case RPCCreate:

‎internal/uvm/log_wcow.go‎

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,57 +12,63 @@ import (
1212
)
1313

1414
func (uvm *UtilityVM) StartLogForwarding(ctx context.Context) error {
15-
// Implementation for stopping the log forwarder
16-
if uvm.OS() == "windows" && uvm.gc != nil {
17-
wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
18-
if wcaps != nil && wcaps.IsLogForwardingSupported() {
19-
req := guestrequest.LogForwardServiceRPCRequest{
20-
RPCType: guestrequest.RPCStartLogForwarding,
21-
Settings: "",
22-
}
23-
err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
24-
if err != nil {
25-
return err
26-
}
27-
} else {
28-
log.G(ctx).WithField("os", uvm.operatingSystem).Error("Log forwarding not supported for this OS")
15+
// Implementation for starting the log forwarding service
16+
if uvm.OS() != "windows" || uvm.gc == nil {
17+
return errNotSupported
18+
}
19+
20+
wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
21+
if wcaps != nil && wcaps.IsLogForwardingSupported() {
22+
req := guestrequest.LogForwardServiceRPCRequest{
23+
RPCType: guestrequest.RPCStartLogForwarding,
24+
Settings: "",
25+
}
26+
err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
27+
if err != nil {
28+
return err
2929
}
30+
} else {
31+
log.G(ctx).WithField("os", uvm.operatingSystem).Error("Log forwarding not supported for this OS")
3032
}
3133
return nil
3234
}
3335

3436
func (uvm *UtilityVM) StopLogForwarding(ctx context.Context) error {
35-
// Implementation for stopping the log forwarder
36-
if uvm.OS() == "windows" && uvm.gc != nil {
37-
wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
38-
if wcaps != nil && wcaps.IsLogForwardingSupported() {
39-
req := guestrequest.LogForwardServiceRPCRequest{
40-
RPCType: guestrequest.RPCStopLogForwarding,
41-
Settings: "",
42-
}
43-
err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
44-
if err != nil {
45-
return err
46-
}
37+
// Implementation for stopping the log forwarding service
38+
if uvm.OS() != "windows" || uvm.gc == nil {
39+
return errNotSupported
40+
}
41+
42+
wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
43+
if wcaps != nil && wcaps.IsLogForwardingSupported() {
44+
req := guestrequest.LogForwardServiceRPCRequest{
45+
RPCType: guestrequest.RPCStopLogForwarding,
46+
Settings: "",
47+
}
48+
err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
49+
if err != nil {
50+
return err
4751
}
4852
}
4953
return nil
5054
}
5155

5256
func (uvm *UtilityVM) SetLogSources(ctx context.Context) error {
5357
// Implementation for setting the log sources
54-
if uvm.OS() == "windows" && uvm.logSources != "" && uvm.gc != nil {
55-
wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
56-
if wcaps != nil && wcaps.IsLogForwardingSupported() {
57-
// Make a call to the GCS to set the ETW providers
58-
req := guestrequest.LogForwardServiceRPCRequest{
59-
RPCType: guestrequest.RPCModifyServiceSettings,
60-
Settings: uvm.logSources,
61-
}
62-
err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
63-
if err != nil {
64-
return err
65-
}
58+
if uvm.OS() != "windows" || uvm.gc == nil {
59+
return errNotSupported
60+
}
61+
62+
wcaps := gcs.GetWCOWCapabilities(uvm.gc.Capabilities())
63+
if wcaps != nil && wcaps.IsLogForwardingSupported() {
64+
// Make a call to the GCS to set the ETW providers
65+
req := guestrequest.LogForwardServiceRPCRequest{
66+
RPCType: guestrequest.RPCModifyServiceSettings,
67+
Settings: uvm.logSources,
68+
}
69+
err := uvm.gc.ModifyServiceSettings(ctx, prot.LogForwardService, req)
70+
if err != nil {
71+
return err
6672
}
6773
}
6874
return nil

‎internal/uvm/start.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,11 @@ func (e *gcsLogEntry) UnmarshalJSON(b []byte) error {
7070
if e.Fields["Source"] == "ETW" {
7171
// Windows ETW log entry
7272
// Original ETW Event Data may have "message" or "Message" field instead of "msg"
73-
74-
if e.Message == "" && e.Fields["message"] != nil {
75-
e.Message = e.Fields["message"].(string)
73+
if msg, ok := e.Fields["message"].(string); ok {
74+
e.Message = msg
7675
delete(e.Fields, "message")
77-
}
78-
if e.Message == "" && e.Fields["Message"] != nil {
79-
e.Message = e.Fields["Message"].(string)
76+
} else if msg, ok := e.Fields["Message"].(string); ok {
77+
e.Message = msg
8078
delete(e.Fields, "Message")
8179
}
8280
}

‎pkg/annotations/annotations.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,31 @@ const (
401401
// VSMBNoDirectMap specifies that no direct mapping should be used for any VSMBs added to the UVM.
402402
VSMBNoDirectMap = "io.microsoft.virtualmachine.wcow.virtualSMB.nodirectmap"
403403

404-
// LogSources specifies the ETW providers to be set for the logging service.
404+
// LogSources specifies the ETW providers to be set for the logging service as a base64-encoded JSON string.
405+
//
406+
// For example:
407+
//
408+
// {
409+
// "logConfig": {
410+
// "sources": [
411+
// {
412+
// "type": "ETW",
413+
// "providers": [
414+
// {
415+
// "providerGuid": "80CE50DE-D264-4581-950D-ABADEEE0D340",
416+
// "providerName": "Microsoft.Windows.HyperV.Compute",
417+
// "level": "Information"
418+
// }
419+
// ]
420+
// }
421+
// ]
422+
// }
423+
// }
424+
//
425+
// Would be encoded as:
426+
//
427+
// "io.microsoft.virtualmachine.wcow.logsources" =
428+
// "eyJsb2dDb25maWciOnsic291cmNlcyI6W3sidHlwZSI6IkVUVyIsInByb3ZpZGVycyI6W3sicHJvdmlkZXJHdWlkIjoiODBDRTUwREUtRDI2NC00NTgxLTk1MEQtQUJBREVFRTBEMzQwIiwicHJvdmlkZXJOYW1lIjoiTWljcm9zb2Z0LldpbmRvd3MuSHlwZXJWLkNvbXB1dGUiLCJsZXZlbCI6IkluZm9ybWF0aW9uIn1dfV19fQ=="
405429
LogSources = "io.microsoft.virtualmachine.wcow.logsources"
406430

407431
// ForwardLogs specifies whether to forward logs to the host or not.

0 commit comments

Comments
 (0)