From 759a8c033778ee32fac9515b488f37ba312da9ef Mon Sep 17 00:00:00 2001 From: Ben LeMasurier Date: Tue, 19 Jul 2016 10:27:31 -0600 Subject: [PATCH] check for QEMU response errors When performing QEMU monitor commands, libvirt will return StatusOK even when the underlying QEMU process fails to perform the command. This modifies Run() to check for QEMU errors. I'm not entirely happy with the hacky modifications to the test library to handle this scenario. The test framework is in obvious need for a complete refactor. For now this will have to work. --- libvirt.go | 40 +++++++++++++++++++++++++++++++++++++++- libvirt_test.go | 11 +++++++++++ libvirttest/libvirt.go | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/libvirt.go b/libvirt.go index 34d9375..42d0d74 100644 --- a/libvirt.go +++ b/libvirt.go @@ -19,6 +19,7 @@ package libvirt import ( "bufio" "bytes" + "encoding/json" "errors" "fmt" "net" @@ -68,6 +69,14 @@ type DomainEvent struct { Details []byte } +// qemuError represents a QEMU process error. +type qemuError struct { + Error struct { + Class string `json:"class"` + Description string `json:"desc"` + } `json:"error"` +} + // Connect establishes communication with the libvirt server. // The underlying libvirt socket connection must be previously established. func (l *Libvirt) Connect() error { @@ -225,10 +234,16 @@ func (l *Libvirt) Run(dom string, cmd []byte) ([]byte, error) { } res := <-resp + // check for libvirt errors if res.Status != StatusOK { return nil, decodeError(res.Payload) } + // check for QEMU process errors + if err = getQEMUError(res); err != nil { + return nil, err + } + r := bytes.NewReader(res.Payload) dec := xdr.NewDecoder(r) data, _, err := dec.DecodeFixedOpaque(int32(r.Len())) @@ -238,7 +253,7 @@ func (l *Libvirt) Run(dom string, cmd []byte) ([]byte, error) { // drop QMP control characters from start of line, and drop // any trailing NULL characters from the end - return bytes.TrimRight(data[4:], "\x00"), err + return bytes.TrimRight(data[4:], "\x00"), nil } // Version returns the version of the libvirt daemon. @@ -308,6 +323,29 @@ func (l *Libvirt) lookup(name string) (*Domain, error) { return &d, nil } +// getQEMUError checks the provided response for QEMU process errors. +// If an error is found, it is extracted an returned, otherwise nil. +func getQEMUError(r response) error { + pl := bytes.NewReader(r.Payload) + dec := xdr.NewDecoder(pl) + + s, _, err := dec.DecodeString() + if err != nil { + return err + } + + var e qemuError + if err = json.Unmarshal([]byte(s), &e); err != nil { + return err + } + + if e.Error.Description != "" { + return errors.New(e.Error.Description) + } + + return nil +} + // New configures a new Libvirt RPC connection. func New(conn net.Conn) *Libvirt { l := &Libvirt{ diff --git a/libvirt_test.go b/libvirt_test.go index 8b987e0..c665433 100644 --- a/libvirt_test.go +++ b/libvirt_test.go @@ -148,6 +148,17 @@ func TestRun(t *testing.T) { } } +func TestRunFail(t *testing.T) { + conn := libvirttest.New() + conn.Fail = true + l := New(conn) + + _, err := l.Run("test", []byte(`{"drive-foo"}`)) + if err == nil { + t.Error("expected qemu error") + } +} + func TestVersion(t *testing.T) { conn := libvirttest.New() l := New(conn) diff --git a/libvirttest/libvirt.go b/libvirttest/libvirt.go index 77e097c..2a3ccdd 100644 --- a/libvirttest/libvirt.go +++ b/libvirttest/libvirt.go @@ -120,6 +120,32 @@ var testRunReply = []byte{ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, } +var testRunReplyFail = []byte{ + 0x00, 0x00, 0x00, 0x8c, // length + 0x20, 0x00, 0x80, 0x87, // program + 0x00, 0x00, 0x00, 0x01, // version + 0x00, 0x00, 0x00, 0x01, // procedure + 0x00, 0x00, 0x00, 0x01, // type + 0x00, 0x00, 0x00, 0x0a, // serial + 0x00, 0x00, 0x00, 0x00, // status + + // {"id":"libvirt-68","error":{"class":"CommandNotFound","desc":"The command drive-foo has not been found"}}` + 0x00, 0x00, 0x00, 0x69, 0x7b, 0x22, 0x69, 0x64, + 0x22, 0x3a, 0x22, 0x6c, 0x69, 0x62, 0x76, 0x69, + 0x72, 0x74, 0x2d, 0x36, 0x38, 0x22, 0x2c, 0x22, + 0x65, 0x72, 0x72, 0x6f, 0x72, 0x22, 0x3a, 0x7b, + 0x22, 0x63, 0x6c, 0x61, 0x73, 0x73, 0x22, 0x3a, + 0x22, 0x43, 0x6f, 0x6d, 0x6d, 0x61, 0x6e, 0x64, + 0x4e, 0x6f, 0x74, 0x46, 0x6f, 0x75, 0x6e, 0x64, + 0x22, 0x2c, 0x22, 0x64, 0x65, 0x73, 0x63, 0x22, + 0x3a, 0x22, 0x54, 0x68, 0x65, 0x20, 0x63, 0x6f, + 0x6d, 0x6d, 0x61, 0x6e, 0x64, 0x20, 0x64, 0x72, + 0x69, 0x76, 0x65, 0x2d, 0x66, 0x6f, 0x6f, 0x20, + 0x68, 0x61, 0x73, 0x20, 0x6e, 0x6f, 0x74, 0x20, + 0x62, 0x65, 0x65, 0x6e, 0x20, 0x66, 0x6f, 0x75, + 0x6e, 0x64, 0x22, 0x7d, 0x7d, 0x00, 0x00, 0x00, +} + var testDomainsReply = []byte{ 0x00, 0x00, 0x00, 0x6c, // length 0x20, 0x00, 0x80, 0x86, // program @@ -171,6 +197,7 @@ var testVersionReply = []byte{ type MockLibvirt struct { net.Conn Test net.Conn + Fail bool serial uint32 } @@ -233,7 +260,11 @@ func (m *MockLibvirt) handleQEMU(procedure uint32, conn net.Conn) { case constants.QEMUConnectDomainMonitorEventDeregister: conn.Write(m.reply(testDeregisterEvent)) case constants.QEMUDomainMonitor: - conn.Write(m.reply(testRunReply)) + if m.Fail { + conn.Write(m.reply(testRunReplyFail)) + } else { + conn.Write(m.reply(testRunReply)) + } } }