From a27a3e70494a7ed6857ff2c3e6acbb0f96b64703 Mon Sep 17 00:00:00 2001 From: Marco Vedovati Date: Thu, 16 May 2019 11:35:22 +0200 Subject: [PATCH 1/2] virtcontainers: kill hypervisor if startSandbox fails Make sure the hypervisor is stopped if startSandbox does not succeed, by calling stopSandbox. Fixes: #1636 Signed-off-by: Marco Vedovati --- virtcontainers/fc.go | 97 +++++++++++++++++++++++++------------------- 1 file changed, 55 insertions(+), 42 deletions(-) diff --git a/virtcontainers/fc.go b/virtcontainers/fc.go index 2f58dccf24..4fde3f6def 100644 --- a/virtcontainers/fc.go +++ b/virtcontainers/fc.go @@ -265,6 +265,54 @@ func (fc *firecracker) fcInit(timeout int) error { return fc.store.Store(store.Hypervisor, fc.info) } +func (fc *firecracker) fcEnd() (err error) { + span, _ := fc.trace("fcEnd") + defer span.Finish() + + fc.Logger().Info("Stopping firecracker VM") + + defer func() { + if err != nil { + fc.Logger().Info("fcEnd failed") + } else { + fc.Logger().Info("Firecracker VM stopped") + } + }() + + pid := fc.info.PID + + // Check if VM process is running, in case it is not, let's + // return from here. + if err = syscall.Kill(pid, syscall.Signal(0)); err != nil { + return nil + } + + // Send a SIGTERM to the VM process to try to stop it properly + if err = syscall.Kill(pid, syscall.SIGTERM); err != nil { + return err + } + + // Wait for the VM process to terminate + tInit := time.Now() + for { + if err = syscall.Kill(pid, syscall.Signal(0)); err != nil { + return nil + } + + if time.Since(tInit).Seconds() >= fcStopSandboxTimeout { + fc.Logger().Warnf("VM still running after waiting %ds", fcStopSandboxTimeout) + break + } + + // Let's avoid to run a too busy loop + time.Sleep(time.Duration(50) * time.Millisecond) + } + + // Let's try with a hammer now, a SIGKILL should get rid of the + // VM process. + return syscall.Kill(pid, syscall.SIGKILL) +} + func (fc *firecracker) client() *client.Firecracker { span, _ := fc.trace("client") defer span.Finish() @@ -370,6 +418,12 @@ func (fc *firecracker) startSandbox(timeout int) error { return err } + defer func() { + if err != nil { + fc.fcEnd() + } + }() + if err := fc.fcSetVMBaseConfig(int64(fc.config.MemorySize), int64(fc.config.NumVCPUs), false); err != nil { @@ -463,48 +517,7 @@ func (fc *firecracker) stopSandbox() (err error) { span, _ := fc.trace("stopSandbox") defer span.Finish() - fc.Logger().Info("Stopping firecracker VM") - - defer func() { - if err != nil { - fc.Logger().Info("stopSandbox failed") - } else { - fc.Logger().Info("Firecracker VM stopped") - } - }() - - pid := fc.info.PID - - // Check if VM process is running, in case it is not, let's - // return from here. - if err = syscall.Kill(pid, syscall.Signal(0)); err != nil { - return nil - } - - // Send a SIGTERM to the VM process to try to stop it properly - if err = syscall.Kill(pid, syscall.SIGTERM); err != nil { - return err - } - - // Wait for the VM process to terminate - tInit := time.Now() - for { - if err = syscall.Kill(pid, syscall.Signal(0)); err != nil { - return nil - } - - if time.Since(tInit).Seconds() >= fcStopSandboxTimeout { - fc.Logger().Warnf("VM still running after waiting %ds", fcStopSandboxTimeout) - break - } - - // Let's avoid to run a too busy loop - time.Sleep(time.Duration(50) * time.Millisecond) - } - - // Let's try with a hammer now, a SIGKILL should get rid of the - // VM process. - return syscall.Kill(pid, syscall.SIGKILL) + return fc.fcEnd() } func (fc *firecracker) pauseSandbox() error { From f89834a276286ed1a3c36e4ee4db57ee90a9925f Mon Sep 17 00:00:00 2001 From: Marco Vedovati Date: Wed, 8 May 2019 10:17:02 +0200 Subject: [PATCH 2/2] virtcontainers: avoid unnecessary error checking in startVM Remove redundant error checking in startVM. Signed-off-by: Marco Vedovati --- virtcontainers/sandbox.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index f02ebbbdce..a9cd507a7d 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -967,11 +967,8 @@ func (s *Sandbox) startVM() (err error) { if err != nil { return err } - err = vm.assignSandbox(s) - if err != nil { - return err - } - return nil + + return vm.assignSandbox(s) } return s.hypervisor.startSandbox(vmStartTimeout)