From f30fd1354a01c45a538541db091934d6e687ebcb Mon Sep 17 00:00:00 2001 From: NingBo Date: Thu, 29 Nov 2018 16:20:03 +0800 Subject: [PATCH 1/2] qmp: Output error detail when execute QMP command failed Only get 'QMP command failed' error message now when execute QMP command by 'executeCommandWithResponse' failed. This patch will output more error detail. Signed-off-by: NingBo --- qemu/qmp.go | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/qemu/qmp.go b/qemu/qmp.go index 43daed369d..a5da684ef7 100644 --- a/qemu/qmp.go +++ b/qemu/qmp.go @@ -313,7 +313,7 @@ func (q *QMP) finaliseCommandWithResponse(cmdEl *list.Element, cmdQueue *list.Li if succeeded { cmd.res <- qmpResult{response: response} } else { - cmd.res <- qmpResult{err: fmt.Errorf("QMP command failed")} + cmd.res <- qmpResult{err: fmt.Errorf("QMP command failed: %v", response)} } } if cmdQueue.Len() > 0 { @@ -325,6 +325,23 @@ func (q *QMP) finaliseCommand(cmdEl *list.Element, cmdQueue *list.List, succeede q.finaliseCommandWithResponse(cmdEl, cmdQueue, succeeded, nil) } +func (q *QMP) errorDesc(errorData interface{}) (string, error) { + // convert error to json + data, err := json.Marshal(errorData) + if err != nil { + return "", fmt.Errorf("Unable to extract error information: %v", err) + } + + // see: https://github.com/qemu/qemu/blob/stable-2.12/qapi/qmp-dispatch.c#L125 + var qmpErr map[string]string + // convert json to qmpError + if err = json.Unmarshal(data, &qmpErr); err != nil { + return "", fmt.Errorf("Unable to convert json to qmpError: %v", err) + } + + return qmpErr["desc"], nil +} + func (q *QMP) processQMPInput(line []byte, cmdQueue *list.List) { var vmData map[string]interface{} err := json.Unmarshal(line, &vmData) @@ -339,7 +356,7 @@ func (q *QMP) processQMPInput(line []byte, cmdQueue *list.List) { } response, succeeded := vmData["return"] - _, failed := vmData["error"] + errData, failed := vmData["error"] if !succeeded && !failed { return @@ -353,6 +370,14 @@ func (q *QMP) processQMPInput(line []byte, cmdQueue *list.List) { } cmd := cmdEl.Value.(*qmpCommand) if failed || cmd.filter == nil { + if errData != nil { + desc, err := q.errorDesc(errData) + if err != nil { + q.cfg.Logger.Infof("Get error description failed: %v", err) + } else { + response = desc + } + } q.finaliseCommandWithResponse(cmdEl, cmdQueue, succeeded, response) } else { cmd.resultReceived = true From dab4cf1d70801c4c78ce47dbbd2d2c1300a399f4 Mon Sep 17 00:00:00 2001 From: NingBo Date: Mon, 3 Dec 2018 14:40:26 +0800 Subject: [PATCH 2/2] qmp: Add tests Test execute QMP command with error response. Signed-off-by: NingBo --- qemu/qmp_test.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/qemu/qmp_test.go b/qemu/qmp_test.go index a1a784dd6e..0d9dd04628 100644 --- a/qemu/qmp_test.go +++ b/qemu/qmp_test.go @@ -1363,3 +1363,86 @@ func TestExecuteBalloon(t *testing.T) { q.Shutdown() <-disconnectedCh } + +func TestErrorDesc(t *testing.T) { + errDesc := "Somthing err messages" + errData := map[string]string{ + "class": "GenericError", + "desc": errDesc, + } + + connectedCh := make(chan *QMPVersion) + disconnectedCh := make(chan struct{}) + buf := newQMPTestCommandBuffer(t) + cfg := QMPConfig{Logger: qmpTestLogger{}} + q := startQMPLoop(buf, cfg, connectedCh, disconnectedCh) + checkVersion(t, connectedCh) + + desc, err := q.errorDesc(errData) + if err != nil { + t.Fatalf("Unexpected error '%v'", err) + } + if desc != errDesc { + t.Fatalf("expected '%v'\n got '%v'", errDesc, desc) + } + + q.Shutdown() + <-disconnectedCh +} + +func TestExecCommandFailed(t *testing.T) { + errDesc := "unable to map backing store for guest RAM: Cannot allocate memory" + errData := map[string]string{ + "class": "GenericError", + "desc": errDesc, + } + + connectedCh := make(chan *QMPVersion) + disconnectedCh := make(chan struct{}) + buf := newQMPTestCommandBuffer(t) + buf.AddCommand("object-add", nil, "error", errData) + cfg := QMPConfig{Logger: qmpTestLogger{}} + q := startQMPLoop(buf, cfg, connectedCh, disconnectedCh) + checkVersion(t, connectedCh) + + _, err := q.executeCommandWithResponse(context.Background(), "object-add", nil, nil, nil) + if err == nil { + t.Fatalf("expected error but got nil") + } + + expectedString := "QMP command failed: " + errDesc + if err.Error() != expectedString { + t.Fatalf("expected '%v' but got '%v'", expectedString, err) + } + + q.Shutdown() + <-disconnectedCh +} + +func TestExecCommandFailedWithInnerError(t *testing.T) { + errData := map[string]string{ + "class": "GenericError", + "descFieldInvalid": "Invalid", + } + + connectedCh := make(chan *QMPVersion) + disconnectedCh := make(chan struct{}) + buf := newQMPTestCommandBuffer(t) + buf.AddCommand("object-add", nil, "error", errData) + cfg := QMPConfig{Logger: qmpTestLogger{}} + q := startQMPLoop(buf, cfg, connectedCh, disconnectedCh) + checkVersion(t, connectedCh) + + _, err := q.executeCommandWithResponse(context.Background(), "object-add", nil, nil, nil) + if err == nil { + t.Fatalf("expected error but got nil") + } + + expectedString := "QMP command failed: " + if err.Error() != expectedString { + t.Fatalf("expected '%v' but got '%v'", expectedString, err) + } + + q.Shutdown() + <-disconnectedCh +}