From 62142696d216f254c6b11c991c113fbc24d1eb97 Mon Sep 17 00:00:00 2001 From: Archana Shinde Date: Mon, 1 Apr 2019 22:02:09 -0700 Subject: [PATCH] channel: Check for channel type in kernel cmdline options With a recent kernel change, the agent can no longer rely on /dev/vsock and AF_VSOCK in socket(), to detect if vhost-vsock channel has been passed by the runtime. These are not created when the vhost-vsock driver is initialised. The runtime should now pass this information explicilty. Based on the channel type passed, the agent now checks for that partcular channel type. In case it is not passed, check for both serial and vsock channel. This also introduces a change in the way vsock channel is detected by checking for devices under the vhost-vsock driver path. Fixes #506 Changes did not apply directly due to additions related to tracing in master, manually resolved conflicts, by getting rid of any tracing related changes. Signed-off-by: Archana Shinde (cherry picked from commit 2af3599360a9bdac04c7c11ce9a340633b85cd39) --- agent.go | 16 ++++++++ channel.go | 102 +++++++++++++++++++++++++++++++++++++++---------- config.go | 14 +++++++ config_test.go | 43 +++++++++++++++++++++ 4 files changed, 155 insertions(+), 20 deletions(-) diff --git a/agent.go b/agent.go index ecc0e3a57b..5159d91bdb 100644 --- a/agent.go +++ b/agent.go @@ -138,6 +138,22 @@ var debug = false // if true, coredump when an internal error occurs or a fatal signal is received var crashOnError = false +// commType is used to denote the communication channel type used. +type commType int + +const ( + // virtio-serial channel + serialCh commType = iota + + // vsock channel + vsockCh + + // channel type not passed explicitly + unknownCh +) + +var commCh = unknownCh + // This is the list of file descriptors we can properly close after the process // has been started. When the new process is exec(), those file descriptors are // duplicated and it is our responsibility to close them since we have opened diff --git a/channel.go b/channel.go index 12966fbaea..6592963d3f 100644 --- a/channel.go +++ b/channel.go @@ -46,21 +46,29 @@ type channel interface { // If there are neither vsocks nor serial ports, an error is returned. func newChannel() (channel, error) { var serialErr error - var serialPath string var vsockErr error - var vSockSupported bool + var ch channel for i := 0; i < channelExistMaxTries; i++ { - // check vsock path - if _, err := os.Stat(vSockDevPath); err == nil { - if vSockSupported, vsockErr = isAFVSockSupportedFunc(); vSockSupported && vsockErr == nil { - return &vSockChannel{}, nil + switch commCh { + case serialCh: + if ch, serialErr = checkForSerialChannel(); serialErr == nil && ch.(*serialChannel) != nil { + return ch, nil + } + case vsockCh: + if ch, vsockErr = checkForVsockChannel(); vsockErr == nil && ch.(*vSockChannel) != nil { + return ch, nil } - } - // Check serial port path - if serialPath, serialErr = findVirtualSerialPath(serialChannelName); serialErr == nil { - return &serialChannel{serialPath: serialPath}, nil + case unknownCh: + // If we have not been explicitly passed if vsock is used or not, maybe due to + // an older runtime, try to check for vsock support. + if ch, vsockErr = checkForVsockChannel(); vsockErr == nil && ch.(*vSockChannel) != nil { + return ch, nil + } + if ch, serialErr = checkForSerialChannel(); serialErr == nil && ch.(*serialChannel) != nil { + return ch, nil + } } time.Sleep(channelExistWaitTime) @@ -77,6 +85,32 @@ func newChannel() (channel, error) { return nil, fmt.Errorf("Neither vsocks nor serial ports were found") } +func checkForSerialChannel() (*serialChannel, error) { + // Check serial port path + serialPath, serialErr := findVirtualSerialPath(serialChannelName) + if serialErr == nil { + agentLog.Debug("Serial channel type detected") + return &serialChannel{serialPath: serialPath}, nil + } + + return nil, serialErr +} + +func checkForVsockChannel() (*vSockChannel, error) { + // check vsock path + if _, err := os.Stat(vSockDevPath); err != nil { + return nil, err + } + + vSockSupported, vsockErr := isAFVSockSupportedFunc() + if vSockSupported && vsockErr == nil { + agentLog.Debug("Vsock channel type detected") + return &vSockChannel{}, nil + } + + return nil, fmt.Errorf("Vsock not found : %s", vsockErr) +} + type vSockChannel struct { } @@ -221,23 +255,51 @@ func (c *serialChannel) teardown() error { return c.serialConn.Close() } +// isAFVSockSupported checks if vsock channel is used by the runtime +// by checking for devices under the vhost-vsock driver path. +// It returns true if a device is found for the vhost-vsock driver. func isAFVSockSupported() (bool, error) { - fd, err := unix.Socket(unix.AF_VSOCK, unix.SOCK_STREAM, 0) - if err != nil { - // This case is valid. It means AF_VSOCK is not a supported - // domain on this system. - if err == unix.EAFNOSUPPORT { - return false, nil - } + // Driver path for virtio-vsock + sysVsockPath := "/sys/bus/virtio/drivers/vmw_vsock_virtio_transport/" + files, err := ioutil.ReadDir(sysVsockPath) + + // This should not happen for a hypervisor with vsock driver + if err != nil { return false, err } - if err := unix.Close(fd); err != nil { - return true, err + // standard driver files that should be ignored + driverFiles := []string{"bind", "uevent", "unbind"} + + for _, file := range files { + for _, f := range driverFiles { + if file.Name() == f { + continue + } + } + + fPath := filepath.Join(sysVsockPath, file.Name()) + fInfo, err := os.Lstat(fPath) + if err != nil { + return false, err + } + + if fInfo.Mode()&os.ModeSymlink == 0 { + continue + } + + link, err := os.Readlink(fPath) + if err != nil { + return false, err + } + + if strings.Contains(link, "devices") { + return true, nil + } } - return true, nil + return false, nil } func findVirtualSerialPath(serialName string) (string, error) { diff --git a/config.go b/config.go index c5e842d6f7..73ca41be8c 100644 --- a/config.go +++ b/config.go @@ -8,6 +8,7 @@ package main import ( "io/ioutil" + "strconv" "strings" "github.com/sirupsen/logrus" @@ -20,6 +21,7 @@ const ( logLevelFlag = optionPrefix + "log" devModeFlag = optionPrefix + "devmode" kernelCmdlineFile = "/proc/cmdline" + useVsockFlag = optionPrefix + "use_vsock" ) type agentConfig struct { @@ -94,6 +96,18 @@ func (c *agentConfig) parseCmdlineOption(option string) error { if level == logrus.DebugLevel { debug = true } + case useVsockFlag: + flag, err := strconv.ParseBool(split[valuePosition]) + if err != nil { + return err + } + if flag { + agentLog.Debug("Param passed to use vsock channel") + commCh = vsockCh + } else { + agentLog.Debug("Param passed to NOT use vsock channel") + commCh = serialCh + } default: if strings.HasPrefix(split[optionPosition], optionPrefix) { return grpcStatus.Errorf(codes.NotFound, "Unknown option %s", split[optionPosition]) diff --git a/config_test.go b/config_test.go index ad68a09b5f..cd716d7c51 100644 --- a/config_test.go +++ b/config_test.go @@ -208,3 +208,46 @@ func TestSetGrpcTrace(t *testing.T) { assert.True(s.enableGrpcTrace, "grpc trace should be enabled") } + +func TestParseCmdlineOptionWrongOptionVsock(t *testing.T) { + t.Skip() + assert := assert.New(t) + + a := &agentConfig{} + + wrongOption := "use_vsockkk=true" + + err := a.parseCmdlineOption(wrongOption) + assert.Errorf(err, "Parsing should fail because wrong option %q", wrongOption) +} + +func TestParseCmdlineOptionsVsock(t *testing.T) { + assert := assert.New(t) + + a := &agentConfig{} + + type testData struct { + val string + shouldErr bool + expectedCommCh commType + } + + data := []testData{ + {"true", false, vsockCh}, + {"false", false, serialCh}, + {"blah", true, unknownCh}, + } + + for _, d := range data { + commCh = unknownCh + option := useVsockFlag + "=" + d.val + + err := a.parseCmdlineOption(option) + if d.shouldErr { + assert.Error(err) + } else { + assert.NoError(err) + } + assert.Equal(commCh, d.expectedCommCh) + } +}