From d8cdf9aa2a1002c471230a5df6f056b268e4a9cd Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 3 Aug 2021 13:03:32 +1000 Subject: [PATCH 1/2] qemu: Drop support for versions older than 5.0 Kata requires version 5.2 (or 5.1 on ARM) anyway. Simplify code by dropping support for older versions. In any case explicit checks against version number aren't necessarily reliable for patched qemu versions. Signed-off-by: David Gibson --- README.md | 5 ++-- qemu/qmp.go | 61 +++++++++++++++--------------------------------- qemu/qmp_test.go | 9 ------- 3 files changed, 22 insertions(+), 53 deletions(-) diff --git a/README.md b/README.md index e23bab54b8..d96c4d9deb 100644 --- a/README.md +++ b/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 -is provided by the github.com/kata-containers/govmm/qemu package. +currently support for only one hypervisor, qemu/kvm (version 5.0 and +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 diff --git a/qemu/qmp.go b/qemu/qmp.go index 8527ff015e..9ebe905b04 100644 --- a/qemu/qmp.go +++ b/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, - } - } + blockdevArgs["node-name"] = blockdevID + args = 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 -// for qemu versions < 2.9. It sends the updated blockdev-del command for qemu>=2.9. -// blockdevID is the id of the block device to be deleted. Typically, this will -// match the id passed to ExecuteBlockdevAdd. It must be a valid QMP id. +// ExecuteBlockdevDel deletes a block device by sending blockdev-del +// command. blockdevID is the id of the block device to be deleted. +// Typically, this will match the id passed to ExecuteBlockdevAdd. It +// 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) + args["node-name"] = blockdevID + return q.executeCommand(ctx, "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 true - } - return false + return driver == "host-x86_64-cpu" } // ExecuteCPUDeviceAdd adds a CPU to a QEMU instance using the device_add command. @@ -1454,11 +1433,9 @@ func (q *QMP) ExecuteNVDIMMDeviceAdd(ctx context.Context, id, mempath string, si }, } - if q.version.Major > 4 || (q.version.Major == 4 && q.version.Minor >= 1) { - if pmem != nil { - props := args["props"].(map[string]interface{}) - props["pmem"] = *pmem - } + if pmem != nil { + props := args["props"].(map[string]interface{}) + props["pmem"] = *pmem } err := q.executeCommand(ctx, "object-add", args, nil) diff --git a/qemu/qmp_test.go b/qemu/qmp_test.go index 47fe11ecdd..38153f914e 100644 --- a/qemu/qmp_test.go +++ b/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 { From d27256f8635d3fa382d6cbd9f3a60f601773c4dc Mon Sep 17 00:00:00 2001 From: David Gibson Date: Tue, 3 Aug 2021 12:52:26 +1000 Subject: [PATCH 2/2] qmp: Don't use deprecated 'props' field for object-add Use of the 'props' argument to 'object-add' has been deprecated since QEMU 5.0 (commit 5f07c4d60d09) in favor of flattening the properties directly into the 'object-add' arguments. Support for 'props' is removed entirely in qemu 6.0 (commit 50243407457a). fixes #193 Signed-off-by: David Gibson --- qemu/qmp.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/qemu/qmp.go b/qemu/qmp.go index 9ebe905b04..229a2e206b 100644 --- a/qemu/qmp.go +++ b/qemu/qmp.go @@ -1366,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 { @@ -1426,16 +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, - }, + "mem-path": mempath, + "size": size, + "share": true, } if pmem != nil { - props := args["props"].(map[string]interface{}) - props["pmem"] = *pmem + args["pmem"] = *pmem } err := q.executeCommand(ctx, "object-add", args, nil)