From b20ba878e16b9d9f2cbd494adddc04213264afb0 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Mon, 13 Nov 2017 23:54:47 -0800 Subject: [PATCH] pod: Improve pod create performances In order to optimize pod creation, this patch does not wait for the VM after it has been started. Instead, it will wait for it during the "start" stage. This will allow our VM to be started while the caller could potentially do other things between create and start. Globally, this improves the boot time of our containers. Signed-off-by: Sebastien Boeuf --- hypervisor.go | 3 +- mock_hypervisor.go | 8 ++-- mock_hypervisor_test.go | 32 ++++++--------- pod.go | 46 +++++++++------------ qemu.go | 89 +++++++++++++++++++++++------------------ 5 files changed, 88 insertions(+), 90 deletions(-) diff --git a/hypervisor.go b/hypervisor.go index aa38c08d..9ebdc62c 100644 --- a/hypervisor.go +++ b/hypervisor.go @@ -446,7 +446,8 @@ func RunningOnVMM(cpuInfoPath string) (bool, error) { type hypervisor interface { init(pod *Pod) error createPod(podConfig PodConfig) error - startPod(startCh, stopCh chan struct{}) error + startPod() error + waitPod(timeout int) error stopPod() error pausePod() error resumePod() error diff --git a/mock_hypervisor.go b/mock_hypervisor.go index fc84851b..9ee6ac2e 100644 --- a/mock_hypervisor.go +++ b/mock_hypervisor.go @@ -36,9 +36,11 @@ func (m *mockHypervisor) createPod(podConfig PodConfig) error { return nil } -func (m *mockHypervisor) startPod(startCh, stopCh chan struct{}) error { - var msg struct{} - startCh <- msg +func (m *mockHypervisor) startPod() error { + return nil +} + +func (m *mockHypervisor) waitPod(timeout int) error { return nil } diff --git a/mock_hypervisor_test.go b/mock_hypervisor_test.go index 7d9c92b5..da52cf51 100644 --- a/mock_hypervisor_test.go +++ b/mock_hypervisor_test.go @@ -19,7 +19,6 @@ package virtcontainers import ( "fmt" "testing" - "time" ) func TestMockHypervisorInit(t *testing.T) { @@ -36,8 +35,7 @@ func TestMockHypervisorInit(t *testing.T) { } // wrong config - err := m.init(pod) - if err == nil { + if err := m.init(pod); err == nil { t.Fatal() } @@ -48,8 +46,7 @@ func TestMockHypervisorInit(t *testing.T) { } // right config - err = m.init(pod) - if err != nil { + if err := m.init(pod); err != nil { t.Fatal(err) } } @@ -59,8 +56,7 @@ func TestMockHypervisorCreatePod(t *testing.T) { config := PodConfig{} - err := m.createPod(config) - if err != nil { + if err := m.createPod(config); err != nil { t.Fatal(err) } } @@ -68,24 +64,23 @@ func TestMockHypervisorCreatePod(t *testing.T) { func TestMockHypervisorStartPod(t *testing.T) { var m *mockHypervisor - startCh := make(chan struct{}) - stopCh := make(chan struct{}) + if err := m.startPod(); err != nil { + t.Fatal(err) + } +} - go m.startPod(startCh, stopCh) +func TestMockHypervisorWaitPod(t *testing.T) { + var m *mockHypervisor - select { - case <-startCh: - break - case <-time.After(time.Second): - t.Fatal("Timeout waiting for start notification") + if err := m.waitPod(0); err != nil { + t.Fatal(err) } } func TestMockHypervisorStopPod(t *testing.T) { var m *mockHypervisor - err := m.stopPod() - if err != nil { + if err := m.stopPod(); err != nil { t.Fatal(err) } } @@ -93,8 +88,7 @@ func TestMockHypervisorStopPod(t *testing.T) { func TestMockHypervisorAddDevice(t *testing.T) { var m *mockHypervisor - err := m.addDevice(nil, imgDev) - if err != nil { + if err := m.addDevice(nil, imgDev); err != nil { t.Fatal(err) } } diff --git a/pod.go b/pod.go index 1cd09d5b..8a6ac516 100644 --- a/pod.go +++ b/pod.go @@ -23,7 +23,6 @@ import ( "strings" "sync" "syscall" - "time" "github.com/sirupsen/logrus" ) @@ -39,6 +38,10 @@ const controlSocket = "ctrl.sock" // to understand if the VM is still alive or not. const monitorSocket = "monitor.sock" +// vmStartTimeout represents the time in seconds a pod can wait before +// to consider the VM starting operation failed. +const vmStartTimeout = 10 + // stateString is a string representing a pod state. type stateString string @@ -748,34 +751,13 @@ func (p *Pod) startSetState() error { return nil } -// startVM starts the VM, ensuring it is started before it returns or issuing -// an error in case of timeout. Then it connects to the agent inside the VM. +// startVM starts the VM. func (p *Pod) startVM(netNsPath string) error { - vmStartedCh := make(chan struct{}) - vmStoppedCh := make(chan struct{}) - const timeout = time.Duration(10) * time.Second - - l := p.Logger() - l.Info("Starting VM") - - go func() { - p.network.run(netNsPath, func() error { - err := p.hypervisor.startPod(vmStartedCh, vmStoppedCh) - return err - }) - }() - - // Wait for the pod started notification - select { - case <-vmStartedCh: - break - case <-time.After(timeout): - return fmt.Errorf("Did not receive the pod started notification (timeout %ds)", timeout) - } - - l.Info("VM started") + p.Logger().Info("Starting VM") - return nil + return p.network.run(netNsPath, func() error { + return p.hypervisor.startPod() + }) } // startShims registers all containers to the proxy and starts one @@ -835,6 +817,14 @@ func (p *Pod) start() error { return err } + l := p.Logger() + + if err := p.hypervisor.waitPod(vmStartTimeout); err != nil { + return err + } + + l.Info("VM started") + if _, _, err := p.proxy.connect(*p, false); err != nil { return err } @@ -855,7 +845,7 @@ func (p *Pod) start() error { } } - p.Logger().Info("started") + l.Info("started") return nil } diff --git a/qemu.go b/qemu.go index f89d042e..40c3c03a 100644 --- a/qemu.go +++ b/qemu.go @@ -573,40 +573,6 @@ func (q *qemu) init(pod *Pod) error { return nil } -func (q *qemu) qmpMonitor(connectedCh chan struct{}) { - defer func(qemu *qemu) { - if q.qmpMonitorCh.qmp != nil { - q.qmpMonitorCh.qmp.Shutdown() - } - - q.qmpMonitorCh.wg.Done() - }(q) - - cfg := ciaoQemu.QMPConfig{Logger: newQMPLogger()} - qmp, ver, err := ciaoQemu.QMPStart(q.qmpMonitorCh.ctx, q.qmpMonitorCh.path, cfg, q.qmpMonitorCh.disconnectCh) - if err != nil { - q.Logger().WithError(err).Error("Failed to connect to QEMU instance") - return - } - - q.qmpMonitorCh.qmp = qmp - - q.Logger().WithFields(logrus.Fields{ - "qmp-major-version": ver.Major, - "qmp-minor-version": ver.Minor, - "qmp-micro-version": ver.Micro, - "qmp-capabilities": strings.Join(ver.Capabilities, ","), - }).Infof("QMP details") - - err = q.qmpMonitorCh.qmp.ExecuteQMPCapabilities(q.qmpMonitorCh.ctx) - if err != nil { - q.Logger().WithError(err).Error(qmpCapErrMsg) - return - } - - close(connectedCh) -} - func (q *qemu) setCPUResources(podConfig PodConfig) ciaoQemu.SMP { vcpus := q.config.DefaultVCPUs if podConfig.VMConfig.VCPUs > 0 { @@ -775,16 +741,61 @@ func (q *qemu) createPod(podConfig PodConfig) error { } // startPod will start the Pod's VM. -func (q *qemu) startPod(startCh, stopCh chan struct{}) error { +func (q *qemu) startPod() error { strErr, err := ciaoQemu.LaunchQemu(q.qemuConfig, newQMPLogger()) if err != nil { return fmt.Errorf("%s", strErr) } - // Start the QMP monitoring thread - q.qmpMonitorCh.disconnectCh = stopCh - q.qmpMonitorCh.wg.Add(1) - q.qmpMonitor(startCh) + return nil +} + +// waitPod will wait for the Pod's VM to be up and running. +func (q *qemu) waitPod(timeout int) error { + defer func(qemu *qemu) { + if q.qmpMonitorCh.qmp != nil { + q.qmpMonitorCh.qmp.Shutdown() + } + }(q) + + if timeout < 0 { + return fmt.Errorf("Invalid timeout %ds", timeout) + } + + disconnectCh := make(chan struct{}) + cfg := ciaoQemu.QMPConfig{Logger: newQMPLogger()} + + var qmp *ciaoQemu.QMP + var ver *ciaoQemu.QMPVersion + var err error + + timeStart := time.Now() + for { + qmp, ver, err = ciaoQemu.QMPStart(q.qmpMonitorCh.ctx, q.qmpMonitorCh.path, cfg, disconnectCh) + if err == nil { + break + } + + if int(time.Now().Sub(timeStart).Seconds()) > timeout { + return fmt.Errorf("Failed to connect to QEMU instance (timeout %ds): %v", timeout, err) + } + + time.Sleep(time.Duration(50) * time.Millisecond) + } + + q.qmpMonitorCh.qmp = qmp + + q.Logger().WithFields(logrus.Fields{ + "qmp-major-version": ver.Major, + "qmp-minor-version": ver.Minor, + "qmp-micro-version": ver.Micro, + "qmp-capabilities": strings.Join(ver.Capabilities, ","), + }).Infof("QMP details") + + if err = q.qmpMonitorCh.qmp.ExecuteQMPCapabilities(q.qmpMonitorCh.ctx); err != nil { + q.Logger().WithError(err).Error(qmpCapErrMsg) + return err + } return nil }