[libvirt] [PATCH 0/2] Prefer -machine accel={tcg, kvm} over -{dis, en}able-kvm

Jiri Denemark (2): qemu: Move -enable-kvm and friends earlier in the command line qemu: Use -machine accel=tcg|kvm when available src/qemu/qemu_command.c | 141 ++++++++++++--------- .../qemuxml2argv-cpu-host-kvmclock.args | 2 +- .../qemuxml2argv-cpu-kvmclock.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-kvm.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvm.xml | 22 ++++ .../qemuxml2argv-machine-core-off.args | 2 +- .../qemuxml2argv-machine-core-on.args | 2 +- .../qemuxml2argv-machine-usb-opt.args | 2 +- tests/qemuxml2argvtest.c | 1 + 9 files changed, 113 insertions(+), 65 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.xml -- 1.8.2.1

--- src/qemu/qemu_command.c | 18 +++++++++--------- .../qemuxml2argv-cpu-host-kvmclock.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05c12b2..dc611aa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5885,6 +5885,15 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0) goto error; + if (disableKQEMU) + virCommandAddArg(cmd, "-no-kqemu"); + else if (enableKQEMU) + virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL); + if (disableKVM) + virCommandAddArg(cmd, "-no-kvm"); + if (enableKVM) + virCommandAddArg(cmd, "-enable-kvm"); + if (qemuBuildCpuArgStr(driver, def, emulator, qemuCaps, hostarch, &cpu, &hasHwVirt, !!migrateFrom) < 0) goto error; @@ -5898,15 +5907,6 @@ qemuBuildCommandLine(virConnectPtr conn, virCommandAddArg(cmd, "-enable-nesting"); } - if (disableKQEMU) - virCommandAddArg(cmd, "-no-kqemu"); - else if (enableKQEMU) - virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL); - if (disableKVM) - virCommandAddArg(cmd, "-no-kvm"); - if (enableKVM) - virCommandAddArg(cmd, "-enable-kvm"); - if (def->os.loader) { virCommandAddArg(cmd, "-bios"); virCommandAddArg(cmd, def->os.loader); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args index 78cf408..817f844 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-host-kvmclock.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ --cpu host,-kvmclock -enable-kvm -m 214 -smp 6 \ +-enable-kvm -cpu host,-kvmclock -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -usb -net \ none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args index 9de6f09..a76f2c2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ --cpu core2duo,-kvmclock -enable-kvm -m 214 -smp 6 \ +-enable-kvm -cpu core2duo,-kvmclock -m 214 -smp 6 \ -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot n -usb -net \ none -serial none -parallel none -- 1.8.2.1

On Tue, Apr 23, 2013 at 05:11:54PM +0200, Jiri Denemark wrote:
--- src/qemu/qemu_command.c | 18 +++++++++--------- .../qemuxml2argv-cpu-host-kvmclock.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.args | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-)
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This is a better interface to choose accelerator than guessing whether we should enable or disable kvm to get the right one. --- src/qemu/qemu_command.c | 141 ++++++++++++--------- tests/qemuxml2argvdata/qemuxml2argv-kvm.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvm.xml | 22 ++++ .../qemuxml2argv-machine-core-off.args | 2 +- .../qemuxml2argv-machine-core-on.args | 2 +- .../qemuxml2argv-machine-usb-opt.args | 2 +- tests/qemuxml2argvtest.c | 1 + 7 files changed, 111 insertions(+), 63 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dc611aa..890c76b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5275,10 +5275,79 @@ no_memory: } static int +qemuBuildObsoleteAccelArg(virCommandPtr cmd, + const virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + int disableKQEMU = 0; + int enableKQEMU = 0; + int disableKVM = 0; + int enableKVM = 0; + + switch (def->virtType) { + case VIR_DOMAIN_VIRT_QEMU: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KQEMU)) + disableKQEMU = 1; + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) + disableKVM = 1; + break; + + case VIR_DOMAIN_VIRT_KQEMU: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) + disableKVM = 1; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KQEMU)) { + enableKQEMU = 1; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KQEMU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support kqemu")); + return -1; + } + break; + + case VIR_DOMAIN_VIRT_KVM: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KQEMU)) + disableKQEMU = 1; + + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM)) { + enableKVM = 1; + } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the QEMU binary does not support kvm")); + return -1; + } + break; + + case VIR_DOMAIN_VIRT_XEN: + /* XXX better check for xenner */ + break; + + default: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("the QEMU binary does not support %s"), + virDomainVirtTypeToString(def->virtType)); + return -1; + } + + if (disableKQEMU) + virCommandAddArg(cmd, "-no-kqemu"); + else if (enableKQEMU) + virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL); + if (disableKVM) + virCommandAddArg(cmd, "-no-kvm"); + if (enableKVM) + virCommandAddArg(cmd, "-enable-kvm"); + + return 0; +} + +static int qemuBuildMachineArgStr(virCommandPtr cmd, const virDomainDefPtr def, virQEMUCapsPtr qemuCaps) { + bool obsoleteAccel = false; + /* This should *never* be NULL, since we always provide * a machine in the capabilities data for QEMU. So this * check is just here as a safety in case the unexpected @@ -5297,12 +5366,20 @@ qemuBuildMachineArgStr(virCommandPtr cmd, "with this QEMU binary")); return -1; } + obsoleteAccel = true; } else { virBuffer buf = VIR_BUFFER_INITIALIZER; virCommandAddArg(cmd, "-machine"); virBufferAdd(&buf, def->os.machine, -1); + if (def->virtType == VIR_DOMAIN_VIRT_QEMU) + virBufferAddLit(&buf, ",accel=tcg"); + else if (def->virtType == VIR_DOMAIN_VIRT_KVM) + virBufferAddLit(&buf, ",accel=kvm"); + else + obsoleteAccel = true; + /* To avoid the collision of creating USB controllers when calling * machine->init in QEMU, it needs to set usb=off */ @@ -5325,6 +5402,10 @@ qemuBuildMachineArgStr(virCommandPtr cmd, virCommandAddArgBuffer(cmd, &buf); } + if (obsoleteAccel && + qemuBuildObsoleteAccelArg(cmd, def, qemuCaps) < 0) + return -1; + return 0; } @@ -5771,10 +5852,6 @@ qemuBuildCommandLine(virConnectPtr conn, enum virNetDevVPortProfileOp vmop) { int i, j; - int disableKQEMU = 0; - int enableKQEMU = 0; - int disableKVM = 0; - int enableKVM = 0; const char *emulator; char uuid[VIR_UUID_STRING_BUFLEN]; char *cpu; @@ -5819,53 +5896,6 @@ qemuBuildCommandLine(virConnectPtr conn, (def->virtType == VIR_DOMAIN_VIRT_QEMU)) virQEMUCapsClear(qemuCaps, QEMU_CAPS_DRIVE_BOOT); - switch (def->virtType) { - case VIR_DOMAIN_VIRT_QEMU: - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KQEMU)) - disableKQEMU = 1; - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) - disableKVM = 1; - break; - - case VIR_DOMAIN_VIRT_KQEMU: - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) - disableKVM = 1; - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KQEMU)) { - enableKQEMU = 1; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KQEMU)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support kqemu"), - emulator); - goto error; - } - break; - - case VIR_DOMAIN_VIRT_KVM: - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KQEMU)) - disableKQEMU = 1; - - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ENABLE_KVM)) { - enableKVM = 1; - } else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support kvm"), - emulator); - goto error; - } - break; - - case VIR_DOMAIN_VIRT_XEN: - /* XXX better check for xenner */ - break; - - default: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("the QEMU binary %s does not support %s"), - emulator, virDomainVirtTypeToString(def->virtType)); - break; - } - cmd = virCommandNew(emulator); virCommandAddEnvPassCommon(cmd); @@ -5885,15 +5915,6 @@ qemuBuildCommandLine(virConnectPtr conn, if (qemuBuildMachineArgStr(cmd, def, qemuCaps) < 0) goto error; - if (disableKQEMU) - virCommandAddArg(cmd, "-no-kqemu"); - else if (enableKQEMU) - virCommandAddArgList(cmd, "-enable-kqemu", "-kernel-kqemu", NULL); - if (disableKVM) - virCommandAddArg(cmd, "-no-kvm"); - if (enableKVM) - virCommandAddArg(cmd, "-enable-kvm"); - if (qemuBuildCpuArgStr(driver, def, emulator, qemuCaps, hostarch, &cpu, &hasHwVirt, !!migrateFrom) < 0) goto error; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm.args b/tests/qemuxml2argvdata/qemuxml2argv-kvm.args new file mode 100644 index 0000000..ac4ed6e --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \ +/usr/libexec/qemu-kvm -S -machine pc-1.0,accel=kvm -m 4096 \ +-smp 4 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ +-no-acpi -boot c -usb -net none -serial none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm.xml b/tests/qemuxml2argvdata/qemuxml2argv-kvm.xml new file mode 100644 index 0000000..1bd367c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm.xml @@ -0,0 +1,22 @@ +<domain type='kvm'> + <name>kvm</name> + <uuid>d091ea82-29e6-2e34-3005-f02617b36e87</uuid> + <memory unit='KiB'>4194304</memory> + <currentMemory unit='KiB'>4194304</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='x86_64' machine='pc-1.0'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.args index 57c1aa3..67de73a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \ --S -machine pc,dump-guest-core=off -m 214 -smp 1 -nographic \ +-S -machine pc,accel=tcg,dump-guest-core=off -m 214 -smp 1 -nographic \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial \ none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.args index 4e57b21..42d0fdd 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \ --S -machine pc,dump-guest-core=on -m 214 -smp 1 -nographic \ +-S -machine pc,accel=tcg,dump-guest-core=on -m 214 -smp 1 -nographic \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial \ none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-usb-opt.args b/tests/qemuxml2argvdata/qemuxml2argv-machine-usb-opt.args index ed88850..2a51b27 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-machine-usb-opt.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-usb-opt.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu \ --S -machine pc,usb=off -m 214 -smp 1 -nographic \ +-S -machine pc,accel=tcg,usb=off -m 214 -smp 1 -nographic \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ -hda /dev/HostVG/QEMUGuest1 -net none -serial \ none -parallel none diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ae73509..e6e1c31 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -372,6 +372,7 @@ mymain(void) DO_TEST_FAILURE("machine-core-on", QEMU_CAPS_MACHINE_OPT); DO_TEST("machine-usb-opt", QEMU_CAPS_MACHINE_OPT, QEMU_CAPS_MACHINE_USB_OPT); + DO_TEST("kvm", QEMU_CAPS_MACHINE_OPT); DO_TEST("boot-cdrom", NONE); DO_TEST("boot-network", NONE); DO_TEST("boot-floppy", NONE); -- 1.8.2.1

On Tue, Apr 23, 2013 at 05:11:55PM +0200, Jiri Denemark wrote:
This is a better interface to choose accelerator than guessing whether we should enable or disable kvm to get the right one. --- src/qemu/qemu_command.c | 141 ++++++++++++--------- tests/qemuxml2argvdata/qemuxml2argv-kvm.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvm.xml | 22 ++++ .../qemuxml2argv-machine-core-off.args | 2 +- .../qemuxml2argv-machine-core-on.args | 2 +- .../qemuxml2argv-machine-usb-opt.args | 2 +- tests/qemuxml2argvtest.c | 1 + 7 files changed, 111 insertions(+), 63 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.xml
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Tue, Apr 23, 2013 at 17:11:53 +0200, Jiri Denemark wrote:
Jiri Denemark (2): qemu: Move -enable-kvm and friends earlier in the command line qemu: Use -machine accel=tcg|kvm when available
src/qemu/qemu_command.c | 141 ++++++++++++--------- .../qemuxml2argv-cpu-host-kvmclock.args | 2 +- .../qemuxml2argv-cpu-kvmclock.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-kvm.args | 4 + tests/qemuxml2argvdata/qemuxml2argv-kvm.xml | 22 ++++ .../qemuxml2argv-machine-core-off.args | 2 +- .../qemuxml2argv-machine-core-on.args | 2 +- .../qemuxml2argv-machine-usb-opt.args | 2 +- tests/qemuxml2argvtest.c | 1 + 9 files changed, 113 insertions(+), 65 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-kvm.xml
Pushed, thanks for the review. Jirka
participants (2)
-
Daniel P. Berrange
-
Jiri Denemark