From 8e3bd358e59915c65b426d960488fe410e6c5121 Mon Sep 17 00:00:00 2001 From: Ted Yu Date: Wed, 3 Jun 2020 06:47:08 -0700 Subject: [PATCH 1/4] shimv2: check correct error variable for deferred func in service#StartShim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In service#StartShim, there is no applicable error variable which is checked by deferred func because the err variable is redefined. This PR fixes the error variable. Fixes #2727 Signed-off-by: Ted Yu Signed-off-by: Fabiano Fidêncio --- src/runtime/containerd-shim-v2/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/containerd-shim-v2/service.go b/src/runtime/containerd-shim-v2/service.go index 9d06b3ec3c..6f049bb216 100644 --- a/src/runtime/containerd-shim-v2/service.go +++ b/src/runtime/containerd-shim-v2/service.go @@ -211,10 +211,10 @@ func (s *service) StartShim(ctx context.Context, id, containerdBinary, container // make sure to wait after start go cmd.Wait() - if err := cdshim.WritePidFile("shim.pid", cmd.Process.Pid); err != nil { + if err = cdshim.WritePidFile("shim.pid", cmd.Process.Pid); err != nil { return "", err } - if err := cdshim.WriteAddress("address", address); err != nil { + if err = cdshim.WriteAddress("address", address); err != nil { return "", err } return address, nil From 342bf3e94912c856db706ff8e8afe9f00a1dbf9d Mon Sep 17 00:00:00 2001 From: Ted Yu Date: Fri, 5 Jun 2020 02:54:59 -0700 Subject: [PATCH 2/4] virtcontainers: drop deferred func for GetAndSetSandboxBlockIndex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #2726 Signed-off-by: Ted Yu Signed-off-by: Fabiano Fidêncio --- src/runtime/virtcontainers/acrn.go | 4 +++- src/runtime/virtcontainers/sandbox.go | 6 ------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/runtime/virtcontainers/acrn.go b/src/runtime/virtcontainers/acrn.go index e186089629..b7582a1b05 100644 --- a/src/runtime/virtcontainers/acrn.go +++ b/src/runtime/virtcontainers/acrn.go @@ -235,7 +235,9 @@ func (a *Acrn) appendImage(devices []Device, imagePath string) ([]Device, error) if sandbox == nil && err != nil { return nil, err } - sandbox.GetAndSetSandboxBlockIndex() + if _, err = sandbox.GetAndSetSandboxBlockIndex(); err != nil { + return nil, err + } devices, err = a.arch.appendImage(devices, imagePath) if err != nil { diff --git a/src/runtime/virtcontainers/sandbox.go b/src/runtime/virtcontainers/sandbox.go index a01a360d39..0a78a11188 100644 --- a/src/runtime/virtcontainers/sandbox.go +++ b/src/runtime/virtcontainers/sandbox.go @@ -1606,7 +1606,6 @@ const maxBlockIndex = 65535 // the BlockIndexMap and marks it as used. This index is used to maintain the // index at which a block device is assigned to a container in the sandbox. func (s *Sandbox) getAndSetSandboxBlockIndex() (int, error) { - var err error currentIndex := -1 for i := 0; i < maxBlockIndex; i++ { if _, ok := s.state.BlockIndexMap[i]; !ok { @@ -1618,11 +1617,6 @@ func (s *Sandbox) getAndSetSandboxBlockIndex() (int, error) { return -1, errors.New("no available block index") } s.state.BlockIndexMap[currentIndex] = struct{}{} - defer func() { - if err != nil { - delete(s.state.BlockIndexMap, currentIndex) - } - }() return currentIndex, nil } From 042426d73a8ed5b64cef51005c7d00dc980fbff7 Mon Sep 17 00:00:00 2001 From: Christophe de Dinechin Date: Wed, 13 May 2020 16:37:16 +0200 Subject: [PATCH 3/4] katatestutils: Use the configured virtiofs daemon path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current path is hardcoded as follows: virtio_fs_daemon = "/path/to/virtiofsd" Switch to using the value of config.VirtioFSDaemon instead. Fixes: #2686 Signed-off-by: Christophe de Dinechin Signed-off-by: Fabiano Fidêncio --- src/runtime/cli/kata-env_test.go | 2 ++ src/runtime/containerd-shim-v2/create_test.go | 2 ++ src/runtime/pkg/katatestutils/utils.go | 3 ++- src/runtime/pkg/katautils/config_test.go | 4 +++- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/runtime/cli/kata-env_test.go b/src/runtime/cli/kata-env_test.go index 4e2daf9b18..46da0ff3c9 100644 --- a/src/runtime/cli/kata-env_test.go +++ b/src/runtime/cli/kata-env_test.go @@ -96,6 +96,7 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC pcieRootPort := uint32(2) disableNewNetNs := false sharedFS := "virtio-9p" + virtioFSdaemon := filepath.Join(prefixDir, "virtiofsd") filesToCreate := []string{ hypervisorPath, @@ -168,6 +169,7 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC AgentDebug: agentDebug, AgentTrace: agentTrace, SharedFS: sharedFS, + VirtioFSDaemon: virtioFSdaemon, } runtimeConfig := katatestutils.MakeRuntimeConfigFileData(configFileOptions) diff --git a/src/runtime/containerd-shim-v2/create_test.go b/src/runtime/containerd-shim-v2/create_test.go index 58daece498..91b80055ad 100644 --- a/src/runtime/containerd-shim-v2/create_test.go +++ b/src/runtime/containerd-shim-v2/create_test.go @@ -401,6 +401,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config string, err err pcieRootPort := uint32(2) disableNewNetNs := false sharedFS := "virtio-9p" + virtioFSdaemon := path.Join(dir, "virtiofsd") configFileOptions := ktu.RuntimeConfigOptions{ Hypervisor: "qemu", @@ -420,6 +421,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config string, err err PCIeRootPort: pcieRootPort, DisableNewNetNs: disableNewNetNs, SharedFS: sharedFS, + VirtioFSDaemon: virtioFSdaemon, } runtimeConfigFileData := ktu.MakeRuntimeConfigFileData(configFileOptions) diff --git a/src/runtime/pkg/katatestutils/utils.go b/src/runtime/pkg/katatestutils/utils.go index bb71d616fb..999ab64b2f 100644 --- a/src/runtime/pkg/katatestutils/utils.go +++ b/src/runtime/pkg/katatestutils/utils.go @@ -28,6 +28,7 @@ type RuntimeConfigOptions struct { AgentTraceMode string AgentTraceType string SharedFS string + VirtioFSDaemon string PCIeRootPort uint32 DisableBlock bool EnableIOThreads bool @@ -65,7 +66,7 @@ func MakeRuntimeConfigFileData(config RuntimeConfigOptions) string { enable_debug = ` + strconv.FormatBool(config.HypervisorDebug) + ` guest_hook_path = "` + config.DefaultGuestHookPath + `" shared_fs = "` + config.SharedFS + `" - virtio_fs_daemon = "/path/to/virtiofsd" + virtio_fs_daemon = "` + config.VirtioFSDaemon + `" [proxy.kata] enable_debug = ` + strconv.FormatBool(config.ProxyDebug) + ` diff --git a/src/runtime/pkg/katautils/config_test.go b/src/runtime/pkg/katautils/config_test.go index 4298f0b843..0e48b186e3 100644 --- a/src/runtime/pkg/katautils/config_test.go +++ b/src/runtime/pkg/katautils/config_test.go @@ -85,6 +85,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf pcieRootPort := uint32(2) disableNewNetNs := false sharedFS := "virtio-9p" + virtioFSdaemon := path.Join(dir, "virtiofsd") configFileOptions := ktu.RuntimeConfigOptions{ Hypervisor: "qemu", @@ -117,6 +118,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf AgentDebug: agentDebug, AgentTrace: agentTrace, SharedFS: sharedFS, + VirtioFSDaemon: virtioFSdaemon, } runtimeConfigFileData := ktu.MakeRuntimeConfigFileData(configFileOptions) @@ -167,7 +169,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf GuestHookPath: defaultGuestHookPath, VhostUserStorePath: defaultVhostUserStorePath, SharedFS: sharedFS, - VirtioFSDaemon: "/path/to/virtiofsd", + VirtioFSDaemon: virtioFSdaemon, VirtioFSCache: defaultVirtioFSCacheMode, } From 487520ff741cdfb8dc1c2c58d5d407811ef43b72 Mon Sep 17 00:00:00 2001 From: Christophe de Dinechin Date: Wed, 13 May 2020 16:04:19 +0200 Subject: [PATCH 4/4] qemu: Report all errors on virtiofsd execution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The virtiofs daemon may run into errors other than the file not existing, e.g. the file may not be executable. Fixes: #2682 Message is now: virtiofs daemon /usr/local/bin/hello returned with error: fork/exec /usr/local/bin/virtiofsd: permission denied instead of panic: runtime error: invalid memory address or nil Fixes: #2582 Message is now: virtiofs daemon /usr/local/bin/hello-not-found returned with error: fork/exec /usr/local/bin/hello-not-found: no such file or directory instead of: virtiofsd path (/usr/local/bin/hello-no-found) does not exist Signed-off-by: Christophe de Dinechin Signed-off-by: Fabiano Fidêncio --- src/runtime/virtcontainers/qemu.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index fca0e57dee..2d04123442 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -646,10 +646,6 @@ func (q *qemu) setupVirtiofsd() (err error) { var listener *net.UnixListener var fd *os.File - if _, err = os.Stat(q.config.VirtioFSDaemon); os.IsNotExist(err) { - return fmt.Errorf("virtiofsd path (%s) does not exist", q.config.VirtioFSDaemon) - } - sockPath, err := q.vhostFSSocketPath(q.id) if err != nil { return err @@ -680,9 +676,10 @@ func (q *qemu) setupVirtiofsd() (err error) { } err = cmd.Start() - if err == nil { - q.state.VirtiofsdPid = cmd.Process.Pid + if err != nil { + return fmt.Errorf("virtiofs daemon %v returned with error: %v", q.config.VirtioFSDaemon, err) } + q.state.VirtiofsdPid = cmd.Process.Pid fd.Close() // Monitor virtiofsd's stderr and stop sandbox if virtiofsd quits