Merge pull request #194 from dgibson/object-add-props

Don't use deprecated 'props' argument to QMP 'object-add'

README.md

@@ -7,8 +7,9 @@
 
 Virtual Machine Manager for Go (govmm) is a suite of packages that
 provide Go APIs for creating and managing virtual machines.  There's
-currently support for only one hypervisor, qemu/kvm, support for which
+currently support for only one hypervisor, qemu/kvm (version 5.0 and
-is provided by the github.com/kata-containers/govmm/qemu package.
+later), support for which is provided by the
+github.com/kata-containers/govmm/qemu package.
 
 The qemu package provides APIs for launching qemu instances and for
 managing those instances via QMP, once launched.  VM instances can

qemu/qmp.go

@@ -719,6 +719,10 @@ func QMPStart(ctx context.Context, socket string, cfg QMPConfig, disconnectedCh
 		}
 	}
 
+	if q.version.Major < 5 {
+		return nil, nil, fmt.Errorf("govmm requires qemu version 5.0 or later, this is qemu (%d.%d)", q.version.Major, q.version.Minor)
+	}
+
 	return q, q.version, nil
 }
 
@@ -780,15 +784,8 @@ func (q *QMP) blockdevAddBaseArgs(device, blockdevID string, ro bool) (map[strin
 		},
 	}
 
-	if q.version.Major > 2 || (q.version.Major == 2 && q.version.Minor >= 8) {
 	blockdevArgs["node-name"] = blockdevID
 	args = blockdevArgs
-	} else {
-		blockdevArgs["id"] = blockdevID
-		args = map[string]interface{}{
-			"options": blockdevArgs,
-		}
-	}
 
 	return args, blockdevArgs
 }
@@ -813,11 +810,6 @@ func (q *QMP) ExecuteBlockdevAdd(ctx context.Context, device, blockdevID string,
 func (q *QMP) ExecuteBlockdevAddWithCache(ctx context.Context, device, blockdevID string, direct, noFlush, ro bool) error {
 	args, blockdevArgs := q.blockdevAddBaseArgs(device, blockdevID, ro)
 
-	if q.version.Major < 2 || (q.version.Major == 2 && q.version.Minor < 9) {
-		return fmt.Errorf("versions of qemu (%d.%d) older than 2.9 do not support set cache-related options for block devices",
-			q.version.Major, q.version.Minor)
-	}
-
 	blockdevArgs["cache"] = map[string]interface{}{
 		"direct":   direct,
 		"no-flush": noFlush,
@@ -850,7 +842,7 @@ func (q *QMP) ExecuteDeviceAdd(ctx context.Context, blockdevID, devID, driver, b
 		args["bus"] = bus
 	}
 
-	if shared && (q.version.Major > 2 || (q.version.Major == 2 && q.version.Minor >= 10)) {
+	if shared {
 		args["share-rw"] = "on"
 	}
 	if transport.isVirtioPCI(nil) {
@@ -904,32 +896,22 @@ func (q *QMP) ExecuteSCSIDeviceAdd(ctx context.Context, blockdevID, devID, drive
 	if lun >= 0 {
 		args["lun"] = lun
 	}
-	if shared && (q.version.Major > 2 || (q.version.Major == 2 && q.version.Minor >= 10)) {
+	if shared {
 		args["share-rw"] = "on"
 	}
 
 	return q.executeCommand(ctx, "device_add", args, nil)
 }
 
-// ExecuteBlockdevDel deletes a block device by sending a x-blockdev-del command
+// ExecuteBlockdevDel deletes a block device by sending blockdev-del
-// for qemu versions < 2.9. It sends the updated blockdev-del command for qemu>=2.9.
+// command.  blockdevID is the id of the block device to be deleted.
-// blockdevID is the id of the block device to be deleted.  Typically, this will
+// Typically, this will match the id passed to ExecuteBlockdevAdd.  It
-// match the id passed to ExecuteBlockdevAdd.  It must be a valid QMP id.
+// must be a valid QMP id.
 func (q *QMP) ExecuteBlockdevDel(ctx context.Context, blockdevID string) error {
 	args := map[string]interface{}{}
 
-	if q.version.Major > 2 || (q.version.Major == 2 && q.version.Minor >= 9) {
 	args["node-name"] = blockdevID
 	return q.executeCommand(ctx, "blockdev-del", args, nil)
-	}
-
-	if q.version.Major == 2 && q.version.Minor == 8 {
-		args["node-name"] = blockdevID
-	} else {
-		args["id"] = blockdevID
-	}
-
-	return q.executeCommand(ctx, "x-blockdev-del", args, nil)
 }
 
 // ExecuteChardevDel deletes a char device by sending a chardev-remove command.
@@ -1104,7 +1086,7 @@ func (q *QMP) ExecutePCIDeviceAdd(ctx context.Context, blockdevID, devID, driver
 	if bus != "" {
 		args["bus"] = bus
 	}
-	if shared && (q.version.Major > 2 || (q.version.Major == 2 && q.version.Minor >= 10)) {
+	if shared {
 		args["share-rw"] = "on"
 	}
 	if queues > 0 {
@@ -1240,10 +1222,7 @@ func isThreadIDSupported(driver string) bool {
 
 // isDieIDSupported returns if the cpu driver and the qemu version support the die id option
 func (q *QMP) isDieIDSupported(driver string) bool {
-	if (q.version.Major > 4 || (q.version.Major == 4 && q.version.Minor >= 1)) && driver == "host-x86_64-cpu" {
+	return driver == "host-x86_64-cpu"
-		return true
-	}
-	return false
 }
 
 // ExecuteCPUDeviceAdd adds a CPU to a QEMU instance using the device_add command.
@@ -1387,17 +1366,16 @@ func (q *QMP) ExecQueryCpusFast(ctx context.Context) ([]CPUInfoFast, error) {
 
 // ExecMemdevAdd adds size of MiB memory device to the guest
 func (q *QMP) ExecMemdevAdd(ctx context.Context, qomtype, id, mempath string, size int, share bool, driver, driverID, addr, bus string) error {
-	props := map[string]interface{}{"size": uint64(size) << 20}
 	args := map[string]interface{}{
 		"qom-type": qomtype,
 		"id":       id,
-		"props":    props,
+		"size":     uint64(size) << 20,
 	}
 	if mempath != "" {
-		props["mem-path"] = mempath
+		args["mem-path"] = mempath
 	}
 	if share {
-		props["share"] = true
+		args["share"] = true
 	}
 	err := q.executeCommand(ctx, "object-add", args, nil)
 	if err != nil {
@@ -1447,18 +1425,13 @@ func (q *QMP) ExecuteNVDIMMDeviceAdd(ctx context.Context, id, mempath string, si
 	args := map[string]interface{}{
 		"qom-type": "memory-backend-file",
 		"id":       "nvdimmbackmem" + id,
-		"props": map[string]interface{}{
 		"mem-path": mempath,
 		"size":     size,
 		"share":    true,
-		},
 	}
 
-	if q.version.Major > 4 || (q.version.Major == 4 && q.version.Minor >= 1) {
 	if pmem != nil {
-			props := args["props"].(map[string]interface{})
+		args["pmem"] = *pmem
-			props["pmem"] = *pmem
-		}
 	}
 
 	err := q.executeCommand(ctx, "object-add", args, nil)

qemu/qmp_test.go

@@ -1125,10 +1125,6 @@ func TestQMPCPUDeviceAdd(t *testing.T) {
 	dieID := "0"
 	coreID := "1"
 	threadID := "0"
-	version := &QMPVersion{
-		Major: 4,
-		Minor: 1,
-	}
 	for _, d := range drivers {
 		connectedCh := make(chan *QMPVersion)
 		disconnectedCh := make(chan struct{})
@@ -1137,7 +1133,6 @@ func TestQMPCPUDeviceAdd(t *testing.T) {
 		cfg := QMPConfig{Logger: qmpTestLogger{}}
 		q := startQMPLoop(buf, cfg, connectedCh, disconnectedCh)
 		checkVersion(t, connectedCh)
-		q.version = version
 		err := q.ExecuteCPUDeviceAdd(context.Background(), d, cpuID, socketID, dieID, coreID, threadID, "")
 		if err != nil {
 			t.Fatalf("Unexpected error %v", err)
@@ -1634,10 +1629,6 @@ func TestExecuteNVDIMMDeviceAdd(t *testing.T) {
 	cfg := QMPConfig{Logger: qmpTestLogger{}}
 	q := startQMPLoop(buf, cfg, connectedCh, disconnectedCh)
 	checkVersion(t, connectedCh)
-	q.version = &QMPVersion{
-		Major: 4,
-		Minor: 1,
-	}
 	pmem := true
 	err := q.ExecuteNVDIMMDeviceAdd(context.Background(), "nvdimm0", "/dev/rbd0", 1024, &pmem)
 	if err != nil {