Clh driver: removed hard-coded vsock contextid (cid) #2217
Conversation
sync fork after merge clh driver #1
virtcontainers/clh.go
Outdated
| @@ -436,7 +429,13 @@ func (clh *cloudHypervisor) addDevice(devInfo interface{}, devType deviceType) e | |||
| device: v.Name(), | |||
| mac: v.HardwareAddr(), | |||
| }) | |||
|
|
|||
| case types.HybridVSock: | |||
| clh.Logger().WithField("function", "addDevice").Infof("Adding HybridVSock: path=%s, cid=%d, port=%d", v.UdsPath, v.ContextID, v.Port) | |||
There was a problem hiding this comment.
Could you change the log calls to be "structured" by pulling out the relevant info into a set of fields:
.WithFields(logrus.Fields{
"function" : "addDevice",
"path" : v.UdsPath,
"cid" : v.ContextID,
"port" : v.Port,
}).Info("Adding HybridVSock")
virtcontainers/clh.go
Outdated
| clh.Logger().WithField("function", "generateSocket").Infof("Using hybrid vsock %s:%d", udsPath, vSockPort) | ||
| _, cid, err := utils.FindContextID() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("Can't generate hybrid vsocket for cloud-hypervisor: error finding available context id") |
There was a problem hiding this comment.
Please include err in the returned error.
There was a problem hiding this comment.
Thanks @ericooper.
lgtm but please reformat the file using gofmt -s as shown in the Travis log:
virtcontainers/types/sandbox.go:190: File is not `gofmt`-ed with `-s` (gofmt)
UdsPath string
ContextID uint64
Port uint32
The command ".ci/static-checks.sh" failed and exited with 1 during .
Note that you can run the static-checks.sh script locally to avoid CI surprises once you push 😄
update after review 2. applied ci static checks Fixes: #2206 Signed-off-by: Johan Kuijpers <[email protected]>
Sure. I didn't know, thanks for the tip. I've applied the ci script and did an update so it should be good to go now. Cheers. |
|
/test-ubuntu |
Codecov Report
@@ Coverage Diff @@
## master #2217 +/- ##
=========================================
Coverage ? 48.95%
=========================================
Files ? 111
Lines ? 16049
Branches ? 0
=========================================
Hits ? 7857
Misses ? 7217
Partials ? 975 |
Removed hardcoded cid in the driver and replaced it by a call to utils find context id. To support this approach an addition of a contextid field in the hybrid vsock type was necessary in sandbox