From c8b4fabc37588d4ad6b341649f93b8e31d354876 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Tue, 31 Jul 2018 11:39:19 +0800 Subject: [PATCH 1/2] qemu: clear qmp state before wait for qemu process So that if there is any remaining state, we do not let it interfere with the new one. This should fix the occasional vm factory hang. Fixes: #535 Signed-off-by: Peng Tao --- virtcontainers/qemu.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 1d84fa88d7..8002110fd4 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -517,6 +517,8 @@ func (q *qemu) waitSandbox(timeout int) error { var ver *govmmQemu.QMPVersion var err error + // clear any possible old state before trying to connect again. + q.qmpShutdown() timeStart := time.Now() for { disconnectCh := make(chan struct{}) From 44a3a441aa92f1596389288c8db4daea3011e419 Mon Sep 17 00:00:00 2001 From: Peng Tao Date: Tue, 31 Jul 2018 18:18:26 +0800 Subject: [PATCH 2/2] qemu: wait on disconnected channel in qmp shutdown That is how govmm ensures us that the qmp channel has been cleaned up entirely. Signed-off-by: Peng Tao --- virtcontainers/qemu.go | 16 ++++++++++++---- virtcontainers/qemu_test.go | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index 8002110fd4..ca3d63ee84 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -25,9 +25,10 @@ import ( ) type qmpChannel struct { - ctx context.Context - path string - qmp *govmmQemu.QMP + ctx context.Context + path string + qmp *govmmQemu.QMP + disconn chan struct{} } // CPUDevice represents a CPU device which was hot-added in a running VM @@ -514,6 +515,7 @@ func (q *qemu) waitSandbox(timeout int) error { cfg := govmmQemu.QMPConfig{Logger: newQMPLogger()} var qmp *govmmQemu.QMP + var disconnectCh chan struct{} var ver *govmmQemu.QMPVersion var err error @@ -521,7 +523,7 @@ func (q *qemu) waitSandbox(timeout int) error { q.qmpShutdown() timeStart := time.Now() for { - disconnectCh := make(chan struct{}) + disconnectCh = make(chan struct{}) qmp, ver, err = govmmQemu.QMPStart(q.qmpMonitorCh.ctx, q.qmpMonitorCh.path, cfg, disconnectCh) if err == nil { break @@ -534,6 +536,7 @@ func (q *qemu) waitSandbox(timeout int) error { time.Sleep(time.Duration(50) * time.Millisecond) } q.qmpMonitorCh.qmp = qmp + q.qmpMonitorCh.disconn = disconnectCh defer q.qmpShutdown() qemuMajorVersion = ver.Major @@ -619,6 +622,7 @@ func (q *qemu) qmpSetup() error { return err } q.qmpMonitorCh.qmp = qmp + q.qmpMonitorCh.disconn = disconnectCh return nil } @@ -626,7 +630,11 @@ func (q *qemu) qmpSetup() error { func (q *qemu) qmpShutdown() { if q.qmpMonitorCh.qmp != nil { q.qmpMonitorCh.qmp.Shutdown() + // wait on disconnected channel to be sure that the qmp channel has + // been closed cleanly. + <-q.qmpMonitorCh.disconn q.qmpMonitorCh.qmp = nil + q.qmpMonitorCh.disconn = nil } } diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index 55b443ea75..6b069f12ad 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -345,3 +345,18 @@ func TestHotplugUnsupportedDeviceType(t *testing.T) { _, err = q.hotplugRemoveDevice(&memoryDevice{0, 128}, fsDev) assert.Error(err) } + +func TestQMPSetupShutdown(t *testing.T) { + assert := assert.New(t) + + qemuConfig := newQemuConfig() + q := &qemu{ + config: qemuConfig, + } + + q.qmpShutdown() + + q.qmpMonitorCh.qmp = &govmmQemu.QMP{} + err := q.qmpSetup() + assert.Nil(err) +}