[libvirt] [PATCH 0/4] qemu: Add support for setting TSC frequency to enable migration with invtsc

QEMU allows for TSC frequency to be explicitly set to enable migration with invtsc (migration fails if the destination QEMU cannot set the exact same frequency used when starting the domain on the source host). Jiri Denemark (4): conf: Fix XML parser for timer frequency qemu: Add support for setting TSC frequency qemu: Use virCPUCheckFeature in qemuMigrationIsAllowed qemu: Allow migration with invtsc if tsc frequency is set src/conf/domain_conf.c | 2 +- src/qemu/qemu_command.c | 16 ++++++---- src/qemu/qemu_migration.c | 28 +++++++++++------ .../qemuxml2argv-cpu-tsc-frequency.args | 23 ++++++++++++++ .../qemuxml2argv-cpu-tsc-frequency.xml | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 6 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml -- 2.12.1

The frequency is documented and formatted as an attribute of the <timer> element rather than a nested <frequency> element expected by the parser. Luckily enough, timer frequency has not been used by any driver so far. And users were not able to set it in the XML either. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6bbc6a2a7..0b39f025c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11350,7 +11350,7 @@ virDomainTimerDefParseXML(xmlNodePtr node, } } - ret = virXPathULong("string(./frequency)", ctxt, &def->frequency); + ret = virXPathULong("string(./@frequency)", ctxt, &def->frequency); if (ret == -1) { def->frequency = 0; } else if (ret < 0) { -- 2.12.1

QEMU allows for TSC frequency to be explicitly set to enable migration with invtsc (migration fails if the destination QEMU cannot set the exact same frequency used when starting the domain on the source host). Libvirt already supports setting the TSC frequency in the XML using <clock> <timer name='tsc' frequency='1234567890'/> </clock> which will be transformed into -cpu Model,tsc-frequency=1234567890 QEMU command line. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_command.c | 16 ++++++---- .../qemuxml2argv-cpu-tsc-frequency.args | 23 ++++++++++++++ .../qemuxml2argv-cpu-tsc-frequency.xml | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 4 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2045c2e7c..8c2e43e60 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6284,12 +6284,12 @@ qemuBuildClockCommandLine(virCommandPtr cmd, for (i = 0; i < def->clock.ntimers; i++) { switch ((virDomainTimerNameType) def->clock.timers[i]->name) { case VIR_DOMAIN_TIMER_NAME_PLATFORM: - case VIR_DOMAIN_TIMER_NAME_TSC: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported timer type (name) '%s'"), virDomainTimerNameTypeToString(def->clock.timers[i]->name)); return -1; + case VIR_DOMAIN_TIMER_NAME_TSC: case VIR_DOMAIN_TIMER_NAME_KVMCLOCK: case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK: /* Timers above are handled when building -cpu. */ @@ -6876,19 +6876,23 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, for (i = 0; i < def->clock.ntimers; i++) { virDomainTimerDefPtr timer = def->clock.timers[i]; - if (timer->present == -1) - continue; - - if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK) { + if (timer->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK && + timer->present != -1) { virBufferAsprintf(&buf, "%s,%ckvmclock", have_cpu ? "" : default_model, timer->present ? '+' : '-'); have_cpu = true; } else if (timer->name == VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK && - timer->present) { + timer->present == 1) { virBufferAsprintf(&buf, "%s,hv_time", have_cpu ? "" : default_model); have_cpu = true; + } else if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC && + timer->frequency > 0) { + virBufferAsprintf(&buf, "%s,tsc-frequency=%lu", + have_cpu ? "" : default_model, + timer->frequency); + have_cpu = true; } } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args b/tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args new file mode 100644 index 000000000..50223fab2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args @@ -0,0 +1,23 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-cpu Haswell,+vme,+ds,+acpi,+ss,+ht,+tm,+pbe,+dtes64,+monitor,+ds_cpl,+vmx,\ ++smx,+est,+tm2,+xtpr,+pdcm,+osxsave,+f16c,+rdrand,+pdpe1gb,+abm,+lahf_lm,\ ++invtsc,tsc-frequency=3504000000 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml b/tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml new file mode 100644 index 000000000..72afe6cd2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml @@ -0,0 +1,35 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <title>A description of the test machine.</title> + <description> + A test of qemu's minimal configuration. + This test also tests the description and title elements. + </description> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <cpu mode='host-model'> + <model fallback='allow'/> + <feature policy='require' name='invtsc'/> + </cpu> + <clock offset='utc'> + <timer name='tsc' frequency='3504000000'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ec73ac7e3..339048d8e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1543,6 +1543,7 @@ mymain(void) DO_TEST("cpu-Haswell3", QEMU_CAPS_KVM); DO_TEST("cpu-Haswell-noTSX", QEMU_CAPS_KVM); DO_TEST("cpu-host-model-cmt", NONE); + DO_TEST("cpu-tsc-frequency", QEMU_CAPS_KVM); qemuTestSetHostCPU(driver.caps, NULL); DO_TEST("encrypted-disk", NONE); -- 2.12.1

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f5711bcf7..1e052a197 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2324,19 +2324,12 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, return false; if (vm->def->cpu) { - for (i = 0; i < vm->def->cpu->nfeatures; i++) { - virCPUFeatureDefPtr feature = &vm->def->cpu->features[i]; - - if (feature->policy != VIR_CPU_FEATURE_REQUIRE) - continue; - - /* QEMU blocks migration and save with invariant TSC enabled */ - if (STREQ(feature->name, "invtsc")) { - virReportError(VIR_ERR_OPERATION_INVALID, - _("domain has CPU feature: %s"), - feature->name); - return false; - } + /* QEMU blocks migration and save with invariant TSC enabled */ + if (virCPUCheckFeature(vm->def->os.arch, vm->def->cpu, + "invtsc") == 1) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain has 'invtsc' CPU feature")); + return false; } } -- 2.12.1

Migration with invtsc is allowed by QEMU as long as TSC frequency is explicitly specified. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_migration.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 1e052a197..924929632 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2324,12 +2324,29 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, return false; if (vm->def->cpu) { - /* QEMU blocks migration and save with invariant TSC enabled */ + /* QEMU blocks migration and save with invariant TSC enabled + * unless TSC frequency is explicitly set. + */ if (virCPUCheckFeature(vm->def->os.arch, vm->def->cpu, "invtsc") == 1) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain has 'invtsc' CPU feature")); - return false; + bool block = true; + + for (i = 0; i < vm->def->clock.ntimers; i++) { + virDomainTimerDefPtr timer = vm->def->clock.timers[i]; + + if (timer->name == VIR_DOMAIN_TIMER_NAME_TSC && + timer->frequency > 0) { + block = false; + break; + } + } + + if (block) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain has 'invtsc' CPU feature but " + "TSC frequency is not specified")); + return false; + } } } -- 2.12.1

On 03/24/2017 09:25 AM, Jiri Denemark wrote:
QEMU allows for TSC frequency to be explicitly set to enable migration with invtsc (migration fails if the destination QEMU cannot set the exact same frequency used when starting the domain on the source host).
Jiri Denemark (4): conf: Fix XML parser for timer frequency qemu: Add support for setting TSC frequency qemu: Use virCPUCheckFeature in qemuMigrationIsAllowed qemu: Allow migration with invtsc if tsc frequency is set
src/conf/domain_conf.c | 2 +- src/qemu/qemu_command.c | 16 ++++++---- src/qemu/qemu_migration.c | 28 +++++++++++------ .../qemuxml2argv-cpu-tsc-frequency.args | 23 ++++++++++++++ .../qemuxml2argv-cpu-tsc-frequency.xml | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 6 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml
Interesting we we didn't require a test back when first committed in '92a9e5df63' and since that was 0.8.0, we won't need some sort of capability check either. ACK series, but don't forget to add a news article before pushing... John

On Mon, Mar 27, 2017 at 14:11:24 -0400, John Ferlan wrote:
On 03/24/2017 09:25 AM, Jiri Denemark wrote:
QEMU allows for TSC frequency to be explicitly set to enable migration with invtsc (migration fails if the destination QEMU cannot set the exact same frequency used when starting the domain on the source host).
Jiri Denemark (4): conf: Fix XML parser for timer frequency qemu: Add support for setting TSC frequency qemu: Use virCPUCheckFeature in qemuMigrationIsAllowed qemu: Allow migration with invtsc if tsc frequency is set
src/conf/domain_conf.c | 2 +- src/qemu/qemu_command.c | 16 ++++++---- src/qemu/qemu_migration.c | 28 +++++++++++------ .../qemuxml2argv-cpu-tsc-frequency.args | 23 ++++++++++++++ .../qemuxml2argv-cpu-tsc-frequency.xml | 35 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 6 files changed, 89 insertions(+), 16 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-tsc-frequency.xml
Interesting we we didn't require a test back when first committed in '92a9e5df63' and since that was 0.8.0, we won't need some sort of capability check either.
ACK series, but don't forget to add a news article before pushing...
Oh yes, I always forget to do that... patch coming soon. Thanks and pushed. Jirka
participants (2)
-
Jiri Denemark
-
John Ferlan