From 4beea5133ed152abb43c2734dbe0815eb1f7f53e Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 15:32:07 +0100 Subject: [PATCH 1/7] Fix staticcheck (ST1005) errors staticcheck was complaining as some of the error messages returned by govmm began with a capital letter. This commit fixes the issue. Signed-off-by: Mark Ryan --- qemu/image.go | 8 ++++---- qemu/qmp.go | 30 +++++++++++++++--------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/qemu/image.go b/qemu/image.go index de12333c4..ddee0670e 100644 --- a/qemu/image.go +++ b/qemu/image.go @@ -51,18 +51,18 @@ func CreateCloudInitISO(ctx context.Context, scratchDir, isoPath string, err := os.MkdirAll(dataDirPath, 0750) if err != nil { - return fmt.Errorf("Unable to create config drive directory %s : %v", + return fmt.Errorf("unable to create config drive directory %s : %v", dataDirPath, err) } err = ioutil.WriteFile(metaDataPath, metaData, 0644) if err != nil { - return fmt.Errorf("Unable to create %s : %v", metaDataPath, err) + return fmt.Errorf("unable to create %s : %v", metaDataPath, err) } err = ioutil.WriteFile(userDataPath, userData, 0644) if err != nil { - return fmt.Errorf("Unable to create %s : %v", userDataPath, err) + return fmt.Errorf("unable to create %s : %v", userDataPath, err) } cmd := exec.CommandContext(ctx, "xorriso", "-as", "mkisofs", "-R", "-V", "config-2", @@ -70,7 +70,7 @@ func CreateCloudInitISO(ctx context.Context, scratchDir, isoPath string, cmd.SysProcAttr = attr err = cmd.Run() if err != nil { - return fmt.Errorf("Unable to create cloudinit iso image %v", err) + return fmt.Errorf("unable to create cloudinit iso image %v", err) } return nil diff --git a/qemu/qmp.go b/qemu/qmp.go index 6e4d4f355..3811e052d 100644 --- a/qemu/qmp.go +++ b/qemu/qmp.go @@ -329,14 +329,14 @@ 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) + 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 "", fmt.Errorf("unable to convert json to qmpError: %v", err) } return qmpErr["desc"], nil @@ -404,7 +404,7 @@ func (q *QMP) writeNextQMPCommand(cmdQueue *list.List) { encodedCmd, err := json.Marshal(&cmdData) if err != nil { cmd.res <- qmpResult{ - err: fmt.Errorf("Unable to marhsall command %s: %v", + err: fmt.Errorf("unable to marhsall command %s: %v", cmd.name, err), } cmdQueue.Remove(cmdEl) @@ -419,7 +419,7 @@ func (q *QMP) writeNextQMPCommand(cmdQueue *list.List) { if err != nil { cmd.res <- qmpResult{ - err: fmt.Errorf("Unable to write command to qmp socket %v", err), + err: fmt.Errorf("unable to write command to qmp socket %v", err), } cmdQueue.Remove(cmdEl) } @@ -689,12 +689,12 @@ func QMPStart(ctx context.Context, socket string, cfg QMPConfig, disconnectedCh case <-ctx.Done(): q.Shutdown() <-disconnectedCh - return nil, nil, fmt.Errorf("Canceled by caller") + return nil, nil, fmt.Errorf("canceled by caller") case <-disconnectedCh: - return nil, nil, fmt.Errorf("Lost connection to VM") + return nil, nil, fmt.Errorf("lost connection to VM") case q.version = <-connectedCh: if q.version == nil { - return nil, nil, fmt.Errorf("Failed to find QMP version information") + return nil, nil, fmt.Errorf("failed to find QMP version information") } } @@ -860,7 +860,7 @@ func (q *QMP) ExecuteSCSIDeviceAdd(ctx context.Context, blockdevID, devID, drive } if !isSCSIDriver { - return fmt.Errorf("Invalid SCSI driver provided %s", driver) + return fmt.Errorf("invalid SCSI driver provided %s", driver) } args := map[string]interface{}{ @@ -1171,13 +1171,13 @@ func (q *QMP) ExecuteQueryHotpluggableCPUs(ctx context.Context) ([]HotpluggableC // convert response to json data, err := json.Marshal(response) if err != nil { - return nil, fmt.Errorf("Unable to extract CPU information: %v", err) + return nil, fmt.Errorf("unable to extract CPU information: %v", err) } var cpus []HotpluggableCPU // convert json to []HotpluggableCPU if err = json.Unmarshal(data, &cpus); err != nil { - return nil, fmt.Errorf("Unable to convert json to hotpluggable CPU: %v", err) + return nil, fmt.Errorf("unable to convert json to hotpluggable CPU: %v", err) } return cpus, nil @@ -1211,7 +1211,7 @@ func (q *QMP) ExecQueryMemoryDevices(ctx context.Context) ([]MemoryDevices, erro // convert response to json data, err := json.Marshal(response) if err != nil { - return nil, fmt.Errorf("Unable to extract memory devices information: %v", err) + return nil, fmt.Errorf("unable to extract memory devices information: %v", err) } var memoryDevices []MemoryDevices @@ -1235,7 +1235,7 @@ func (q *QMP) ExecQueryCpus(ctx context.Context) ([]CPUInfo, error) { // convert response to json data, err := json.Marshal(response) if err != nil { - return nil, fmt.Errorf("Unable to extract memory devices information: %v", err) + return nil, fmt.Errorf("unable to extract memory devices information: %v", err) } var cpuInfo []CPUInfo @@ -1259,7 +1259,7 @@ func (q *QMP) ExecQueryCpusFast(ctx context.Context) ([]CPUInfoFast, error) { // convert response to json data, err := json.Marshal(response) if err != nil { - return nil, fmt.Errorf("Unable to extract memory devices information: %v", err) + return nil, fmt.Errorf("unable to extract memory devices information: %v", err) } var cpuInfoFast []CPUInfoFast @@ -1434,12 +1434,12 @@ func (q *QMP) ExecuteQueryMigration(ctx context.Context) (MigrationStatus, error data, err := json.Marshal(response) if err != nil { - return MigrationStatus{}, fmt.Errorf("Unable to extract migrate status information: %v", err) + return MigrationStatus{}, fmt.Errorf("unable to extract migrate status information: %v", err) } var status MigrationStatus if err = json.Unmarshal(data, &status); err != nil { - return MigrationStatus{}, fmt.Errorf("Unable to convert migrate status information: %v", err) + return MigrationStatus{}, fmt.Errorf("unable to convert migrate status information: %v", err) } return status, nil From 5f2e630bdad07960f2ebd1a721c21ab8a44b9c87 Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 16:06:49 +0100 Subject: [PATCH 2/7] Fix staticcheck (S1025) staticcheck was complaining as there were quite a lot of fmt.Sprintf("%s",d) in the code where d was either a string or had string as its underlying type. Signed-off-by: Mark Ryan --- qemu/qemu.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/qemu/qemu.go b/qemu/qemu.go index 8bd6afbef..f209cae1c 100644 --- a/qemu/qemu.go +++ b/qemu/qemu.go @@ -260,7 +260,7 @@ func (fsdev FSDevice) QemuParams(config *Config) []string { var deviceParams []string var qemuParams []string - deviceParams = append(deviceParams, fmt.Sprintf("%s", fsdev.Driver)) + deviceParams = append(deviceParams, string(fsdev.Driver)) if s := fsdev.Driver.disableModern(fsdev.DisableModern); s != "" { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } @@ -346,7 +346,7 @@ func (cdev CharDevice) QemuParams(config *Config) []string { var deviceParams []string var qemuParams []string - deviceParams = append(deviceParams, fmt.Sprintf("%s", cdev.Driver)) + deviceParams = append(deviceParams, string(cdev.Driver)) if s := cdev.Driver.disableModern(cdev.DisableModern); s != "" { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } @@ -627,7 +627,7 @@ func (dev SerialDevice) QemuParams(config *Config) []string { var deviceParams []string var qemuParams []string - deviceParams = append(deviceParams, fmt.Sprintf("%s", dev.Driver)) + deviceParams = append(deviceParams, string(dev.Driver)) if s := dev.Driver.disableModern(dev.DisableModern); s != "" { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } @@ -705,7 +705,7 @@ func (blkdev BlockDevice) QemuParams(config *Config) []string { var deviceParams []string var qemuParams []string - deviceParams = append(deviceParams, fmt.Sprintf("%s", blkdev.Driver)) + deviceParams = append(deviceParams, string(blkdev.Driver)) if s := blkdev.Driver.disableModern(blkdev.DisableModern); s != "" { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } @@ -910,7 +910,7 @@ func (scsiCon SCSIController) QemuParams(config *Config) []string { devParams = append(devParams, fmt.Sprintf("addr=%s", scsiCon.Addr)) } if s := driver.disableModern(scsiCon.DisableModern); s != "" { - devParams = append(devParams, fmt.Sprintf("%s", s)) + devParams = append(devParams, s) } if scsiCon.IOThread != "" { devParams = append(devParams, fmt.Sprintf("iothread=%s", scsiCon.IOThread)) @@ -1057,7 +1057,7 @@ func (vsock VSOCKDevice) QemuParams(config *Config) []string { var qemuParams []string driver := VHostVSock - deviceParams = append(deviceParams, fmt.Sprintf("%s", driver)) + deviceParams = append(deviceParams, string(driver)) if s := driver.disableModern(vsock.DisableModern); s != "" { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } @@ -1174,7 +1174,7 @@ func (b BalloonDevice) QemuParams(_ *Config) []string { deviceParams = append(deviceParams, "deflate-on-oom=off") } if s := driver.disableModern(b.DisableModern); s != "" { - deviceParams = append(deviceParams, fmt.Sprintf("%s", s)) + deviceParams = append(deviceParams, string(s)) } qemuParams = append(qemuParams, "-device") qemuParams = append(qemuParams, strings.Join(deviceParams, ",")) @@ -1528,7 +1528,7 @@ func (config *Config) appendQMPSockets() { } qmpParams := append([]string{}, fmt.Sprintf("%s:", q.Type)) - qmpParams = append(qmpParams, fmt.Sprintf("%s", q.Name)) + qmpParams = append(qmpParams, q.Name) if q.Server == true { qmpParams = append(qmpParams, ",server") if q.NoWait == true { From f0172cd2a6a0e2701f89883df654cd318c832297 Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 16:13:48 +0100 Subject: [PATCH 3/7] Fix staticcheck (S1002) staticcheck was complaining about code that looked like if x == true { } rather than if x { } This commit fixes the issue. Signed-off-by: Mark Ryan --- qemu/qemu.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/qemu/qemu.go b/qemu/qemu.go index f209cae1c..47fc3fadf 100644 --- a/qemu/qemu.go +++ b/qemu/qemu.go @@ -534,7 +534,7 @@ func (netdev NetDevice) QemuNetdevParams(config *Config) []string { netdevParams = append(netdevParams, netdev.Type.QemuNetdevParam()) netdevParams = append(netdevParams, fmt.Sprintf(",id=%s", netdev.ID)) - if netdev.VHost == true { + if netdev.VHost { netdevParams = append(netdevParams, ",vhost=on") if len(netdev.VhostFDs) > 0 { var fdParams []string @@ -710,11 +710,11 @@ func (blkdev BlockDevice) QemuParams(config *Config) []string { deviceParams = append(deviceParams, fmt.Sprintf(",%s", s)) } deviceParams = append(deviceParams, fmt.Sprintf(",drive=%s", blkdev.ID)) - if blkdev.SCSI == false { + if !blkdev.SCSI { deviceParams = append(deviceParams, ",scsi=off") } - if blkdev.WCE == false { + if !blkdev.WCE { deviceParams = append(deviceParams, ",config-wce=off") } @@ -1523,15 +1523,15 @@ func (config *Config) appendCPUModel() { func (config *Config) appendQMPSockets() { for _, q := range config.QMPSockets { - if q.Valid() == false { + if !q.Valid() { continue } qmpParams := append([]string{}, fmt.Sprintf("%s:", q.Type)) qmpParams = append(qmpParams, q.Name) - if q.Server == true { + if q.Server { qmpParams = append(qmpParams, ",server") - if q.NoWait == true { + if q.NoWait { qmpParams = append(qmpParams, ",nowait") } } @@ -1543,7 +1543,7 @@ func (config *Config) appendQMPSockets() { func (config *Config) appendDevices() { for _, d := range config.Devices { - if d.Valid() == false { + if !d.Valid() { continue } @@ -1611,7 +1611,7 @@ func (config *Config) appendCPUs() error { } func (config *Config) appendRTC() { - if config.RTC.Valid() == false { + if !config.RTC.Valid() { return } @@ -1663,7 +1663,7 @@ func (config *Config) appendKernel() { } func (config *Config) appendMemoryKnobs() { - if config.Knobs.HugePages == true { + if config.Knobs.HugePages { if config.Memory.Size != "" { dimmName := "dimm1" objMemParam := "memory-backend-file,id=" + dimmName + ",size=" + config.Memory.Size + ",mem-path=/dev/hugepages,share=on,prealloc=on" @@ -1675,7 +1675,7 @@ func (config *Config) appendMemoryKnobs() { config.qemuParams = append(config.qemuParams, "-numa") config.qemuParams = append(config.qemuParams, numaMemParam) } - } else if config.Knobs.MemPrealloc == true { + } else if config.Knobs.MemPrealloc { if config.Memory.Size != "" { dimmName := "dimm1" objMemParam := "memory-backend-ram,id=" + dimmName + ",size=" + config.Memory.Size + ",prealloc=on" @@ -1687,11 +1687,11 @@ func (config *Config) appendMemoryKnobs() { config.qemuParams = append(config.qemuParams, "-numa") config.qemuParams = append(config.qemuParams, numaMemParam) } - } else if config.Knobs.FileBackedMem == true { + } else if config.Knobs.FileBackedMem { if config.Memory.Size != "" && config.Memory.Path != "" { dimmName := "dimm1" objMemParam := "memory-backend-file,id=" + dimmName + ",size=" + config.Memory.Size + ",mem-path=" + config.Memory.Path - if config.Knobs.FileBackedMemShared == true { + if config.Knobs.FileBackedMemShared { objMemParam += ",share=on" } numaMemParam := "node,memdev=" + dimmName @@ -1706,45 +1706,45 @@ func (config *Config) appendMemoryKnobs() { } func (config *Config) appendKnobs() { - if config.Knobs.NoUserConfig == true { + if config.Knobs.NoUserConfig { config.qemuParams = append(config.qemuParams, "-no-user-config") } - if config.Knobs.NoDefaults == true { + if config.Knobs.NoDefaults { config.qemuParams = append(config.qemuParams, "-nodefaults") } - if config.Knobs.NoGraphic == true { + if config.Knobs.NoGraphic { config.qemuParams = append(config.qemuParams, "-nographic") } - if config.Knobs.Daemonize == true { + if config.Knobs.Daemonize { config.qemuParams = append(config.qemuParams, "-daemonize") } config.appendMemoryKnobs() - if config.Knobs.Realtime == true { + if config.Knobs.Realtime { config.qemuParams = append(config.qemuParams, "-realtime") // This path is redundant as the default behaviour is locked memory // Realtime today does not control any other feature even though // other features may be added in the future // https://lists.gnu.org/archive/html/qemu-devel/2012-12/msg03330.html - if config.Knobs.Mlock == true { + if config.Knobs.Mlock { config.qemuParams = append(config.qemuParams, "mlock=on") } else { config.qemuParams = append(config.qemuParams, "mlock=off") } } else { // In order to turn mlock off we need the -realtime option as well - if config.Knobs.Mlock == false { + if !config.Knobs.Mlock { //Enable realtime anyway just to get the right swapping behaviour config.qemuParams = append(config.qemuParams, "-realtime") config.qemuParams = append(config.qemuParams, "mlock=off") } } - if config.Knobs.Stopped == true { + if config.Knobs.Stopped { config.qemuParams = append(config.qemuParams, "-S") } } From cb2ce9339c38b3d145660475b859406fb0148a01 Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 16:17:24 +0100 Subject: [PATCH 4/7] Fix staticcheck S1008 static check was complaining about code that looked like if x == "" { return false } return true when what it wants to see is return x != "". This commit fixes the issue. Signed-off-by: Mark Ryan --- qemu/qemu.go | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/qemu/qemu.go b/qemu/qemu.go index 47fc3fadf..9703018d5 100644 --- a/qemu/qemu.go +++ b/qemu/qemu.go @@ -842,11 +842,7 @@ type VFIODevice struct { // Valid returns true if the VFIODevice structure is valid and complete. func (vfioDev VFIODevice) Valid() bool { - if vfioDev.BDF == "" { - return false - } - - return true + return vfioDev.BDF != "" } // QemuParams returns the qemu parameters built out of this vfio device. @@ -889,11 +885,7 @@ type SCSIController struct { // Valid returns true if the SCSIController structure is valid and complete. func (scsiCon SCSIController) Valid() bool { - if scsiCon.ID == "" { - return false - } - - return true + return scsiCon.ID != "" } // QemuParams returns the qemu parameters built out of this SCSIController device. @@ -1094,11 +1086,7 @@ type RngDevice struct { // Valid returns true if the RngDevice structure is valid and complete. func (v RngDevice) Valid() bool { - if v.ID == "" { - return false - } - - return true + return v.ID != "" } // QemuParams returns the qemu parameters built out of the RngDevice. @@ -1184,11 +1172,7 @@ func (b BalloonDevice) QemuParams(_ *Config) []string { // Valid returns true if the balloonDevice structure is valid and complete. func (b BalloonDevice) Valid() bool { - if b.ID == "" { - return false - } - - return true + return b.ID != "" } // RTCBaseType is the qemu RTC base time type. From 932fdc7f501525156fef7fbba38e4dd5ca46f9ee Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 16:19:20 +0100 Subject: [PATCH 5/7] Fix staticcheck S1023 By removing a redundant return statement. Signed-off-by: Mark Ryan --- qemu/qemu_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/qemu/qemu_test.go b/qemu/qemu_test.go index 622f47ea7..2af0ded79 100644 --- a/qemu/qemu_test.go +++ b/qemu/qemu_test.go @@ -30,8 +30,6 @@ const volumeUUID = "67d86208-b46c-4465-9018-e14187d4010" func testAppend(structure interface{}, expected string, t *testing.T) { var config Config testConfigAppend(&config, structure, expected, t) - - return } func testConfigAppend(config *Config, structure interface{}, expected string, t *testing.T) { From ad310f9fde4fffaafdea0138fc8773b8481cc7bb Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 16:20:23 +0100 Subject: [PATCH 6/7] Fix staticcheck S1023 Static check was complaining about code that looked like _ = <-ch when it wants to see simply <-ch There was only one instance of this in govmm and this commit fixes that instance. Signed-off-by: Mark Ryan --- qemu/qmp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu/qmp.go b/qemu/qmp.go index 3811e052d..74431c12d 100644 --- a/qemu/qmp.go +++ b/qemu/qmp.go @@ -525,7 +525,7 @@ func (q *QMP) mainLoop() { } /* #nosec */ _ = q.conn.Close() - _ = <-fromVMCh + <-fromVMCh failOutstandingCommands(cmdQueue) close(q.disconnectedCh) }() From 1f51b4386be382f21810cdc01f3cd4df0a946f45 Mon Sep 17 00:00:00 2001 From: Mark Ryan Date: Mon, 28 Jan 2019 16:31:51 +0100 Subject: [PATCH 7/7] Update the versions of Go used to build GoVMM The .travis file was building GoVMM with some old of date versions of Go that seem to be incompatible with the latest versions of gometalinter. This commit updates the .travis file so that we build against 1.10 and 1.11. Signed-off-by: Mark Ryan --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index b8eaf3eb6..41aec9bb0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,8 @@ language: go go: - - 1.8 - - 1.9 + - "1.10" + - "1.11" - tip go_import_path: github.com/intel/govmm