pod: Improve pod create performances

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

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
 }
 

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,42 +56,39 @@ func TestMockHypervisorCreatePod(t *testing.T) {
 
 	config := PodConfig{}
 
-	err := m.createPod(config)
-	if err != nil {
+	if err := m.createPod(config); err != nil {
 		t.Fatal(err)
 	}
 }
 
 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)
 	}
 }
 
 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)
 	}
 }

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
 }

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
 }