[libvirt] [PATCH 0/3] qemu: fix pit timer tick policy
Maxim Nestratov (3): qemu: fix pit timer tick policy=delay qemu: allow to specify pit timer tick policy=discard qemu: tests: add "no-kvm-pit-device" testcase src/qemu/qemu_command.c | 10 ++++++-- .../qemuxml2argv-kvm-pit-delay.args | 2 +- .../qemuxml2argv-kvm-pit-device.xml | 2 +- .../qemuxml2argv-no-kvm-pit-device.args | 23 +++++++++++++++++ .../qemuxml2argv-no-kvm-pit-device.xml | 29 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 5 ++-- 6 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml -- 2.4.11
By a mistake, 'delay' libvirt xml parameter was converted to 'discard' QEMU command line string one. Test "kvm-pit-delay" is fixed accordinly, so that redundant test cases removed as there is no need to specify both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY simultaneusly in tests as they are mutually exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay". Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args | 2 +- tests/qemuxml2argvtest.c | 5 +---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 815abff..cd243e4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6139,7 +6139,7 @@ qemuBuildClockCommandLine(virCommandPtr cmd, (-no-kvm-pit), otherwise, the default is catchup. */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) virCommandAddArgList(cmd, "-global", - "kvm-pit.lost_tick_policy=discard", NULL); + "kvm-pit.lost_tick_policy=delay", NULL); else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args index 1d69797..7a02d36 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args @@ -14,7 +14,7 @@ QEMU_AUDIO_DRV=none \ -nographic \ -nodefaults \ -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ --no-kvm-pit-reinjection \ +-global kvm-pit.lost_tick_policy=delay \ -no-acpi \ -boot c \ -usb \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 90d6aaf..b8619dd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2059,10 +2059,7 @@ mymain(void) QEMU_CAPS_KVM); qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); - DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY); - DO_TEST("kvm-pit-delay", QEMU_CAPS_NO_KVM_PIT); - DO_TEST("kvm-pit-device", QEMU_CAPS_NO_KVM_PIT, - QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG); -- 2.4.11
Since this has been sitting unreviewed for a while... On 12/09/2016 09:28 AM, Maxim Nestratov wrote:
By a mistake, 'delay' libvirt xml parameter was converted to 'discard' QEMU command line string one.
Test "kvm-pit-delay" is fixed accordinly, so that redundant test cases removed as there is no need to specify both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY simultaneusly in tests as they are mutually exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay".
I'd like to alter the commit to be: qemu: Fix pit timer tick policy=delay By a mistake, for the VIR_DOMAIN_TIMER_TICKPOLICY_DELAY qemu command line creation, 'discard' was used instead of 'delay' in commit id '1569fa14'. Test "kvm-pit-delay" is fixed accordingly to show the correct option being generated. Remove the (now) redundant kvm-pit-device tests. As it turns out there is no need to specify both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY since they are mutually exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay". ... ACK - (let me know if you agree with the adjust commit - I can push if you would like, although I know you have that access.) John
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/qemu/qemu_command.c | 2 +- tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args | 2 +- tests/qemuxml2argvtest.c | 5 +---- 3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 815abff..cd243e4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6139,7 +6139,7 @@ qemuBuildClockCommandLine(virCommandPtr cmd, (-no-kvm-pit), otherwise, the default is catchup. */ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) virCommandAddArgList(cmd, "-global", - "kvm-pit.lost_tick_policy=discard", NULL); + "kvm-pit.lost_tick_policy=delay", NULL); else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); break; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args index 1d69797..7a02d36 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-delay.args @@ -14,7 +14,7 @@ QEMU_AUDIO_DRV=none \ -nographic \ -nodefaults \ -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ --no-kvm-pit-reinjection \ +-global kvm-pit.lost_tick_policy=delay \ -no-acpi \ -boot c \ -usb \ diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 90d6aaf..b8619dd 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2059,10 +2059,7 @@ mymain(void) QEMU_CAPS_KVM); qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
- DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY); - DO_TEST("kvm-pit-delay", QEMU_CAPS_NO_KVM_PIT); - DO_TEST("kvm-pit-device", QEMU_CAPS_NO_KVM_PIT, - QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY);
DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG);
05-Jan-17 19:52, John Ferlan пишет:
Since this has been sitting unreviewed for a while...
On 12/09/2016 09:28 AM, Maxim Nestratov wrote:
By a mistake, 'delay' libvirt xml parameter was converted to 'discard' QEMU command line string one.
Test "kvm-pit-delay" is fixed accordinly, so that redundant test cases removed as there is no need to specify both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY simultaneusly in tests as they are mutually exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay". I'd like to alter the commit to be:
qemu: Fix pit timer tick policy=delay
By a mistake, for the VIR_DOMAIN_TIMER_TICKPOLICY_DELAY qemu command line creation, 'discard' was used instead of 'delay' in commit id '1569fa14'.
Test "kvm-pit-delay" is fixed accordingly to show the correct option being generated.
Remove the (now) redundant kvm-pit-device tests. As it turns out there is no need to specify both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY since they are mutually exclusive and "kvm-pit-device" becomes just the same as "kvm-pit-delay".
...
ACK - (let me know if you agree with the adjust commit - I can push if you would like, although I know you have that access.)
John
Agree with the your changes and don't mind if you push it. Maxim
Reuse "kvm-pit-device" test case for testing it. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/qemu/qemu_command.c | 8 +++++++- tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml | 2 +- tests/qemuxml2argvtest.c | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cd243e4..7f10d75 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6158,8 +6158,14 @@ qemuBuildClockCommandLine(virCommandPtr cmd, return -1; } break; - case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) + virCommandAddArgList(cmd, "-global", + "kvm-pit.lost_tick_policy=discard", NULL); + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) + virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: /* no way to support these modes for pit in qemu */ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported pit tickpolicy '%s'"), diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml index 7835a1b..d8ddcba 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml @@ -9,7 +9,7 @@ <boot dev='hd'/> </os> <clock offset='utc'> - <timer name='pit' tickpolicy='delay'/> + <timer name='pit' tickpolicy='discard'/> </clock> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b8619dd..713a8fe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2060,6 +2060,7 @@ mymain(void) qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG); -- 2.4.11
On 12/09/2016 09:28 AM, Maxim Nestratov wrote:
Reuse "kvm-pit-device" test case for testing it.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/qemu/qemu_command.c | 8 +++++++- tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml | 2 +- tests/qemuxml2argvtest.c | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)
My suggestion here - let's rename *pit-device.xml to *pit-discard.xml and do the same for the .args file. That'll make it clearer. Of course that means modifying argvtest.c as well. This would alter the commit message to: qemu: Allow to specify pit timer tick policy=discard Separate out the "policy=discard" into it's own specific qemu command line. We'll rename "kvm-pit-device" test case to be "kvm-pit-discard" since it has the syntax we'd be using.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cd243e4..7f10d75 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6158,8 +6158,14 @@ qemuBuildClockCommandLine(virCommandPtr cmd, return -1; } break; - case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) + virCommandAddArgList(cmd, "-global", + "kvm-pit.lost_tick_policy=discard", NULL); + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) + virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: /* no way to support these modes for pit in qemu */
s/these modes/this mode/ ACK w/ these adjustments (I can do this as well if you want) John
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported pit tickpolicy '%s'"), diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml index 7835a1b..d8ddcba 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml @@ -9,7 +9,7 @@ <boot dev='hd'/> </os> <clock offset='utc'> - <timer name='pit' tickpolicy='delay'/> + <timer name='pit' tickpolicy='discard'/> </clock> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b8619dd..713a8fe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2060,6 +2060,7 @@ mymain(void) qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY);
DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG);
05-Jan-17 19:52, John Ferlan пишет:
On 12/09/2016 09:28 AM, Maxim Nestratov wrote:
Reuse "kvm-pit-device" test case for testing it.
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- src/qemu/qemu_command.c | 8 +++++++- tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml | 2 +- tests/qemuxml2argvtest.c | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)
My suggestion here - let's rename *pit-device.xml to *pit-discard.xml and do the same for the .args file. That'll make it clearer. Of course that means modifying argvtest.c as well.
I thought about it too but decided to leave it as is to reduce changes, thus I don't mind.
This would alter the commit message to:
qemu: Allow to specify pit timer tick policy=discard
Separate out the "policy=discard" into it's own specific qemu command line.
We'll rename "kvm-pit-device" test case to be "kvm-pit-discard" since it has the syntax we'd be using.
Agree.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cd243e4..7f10d75 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6158,8 +6158,14 @@ qemuBuildClockCommandLine(virCommandPtr cmd, return -1; } break; - case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: case VIR_DOMAIN_TIMER_TICKPOLICY_DISCARD: + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_KVM_PIT_TICK_POLICY)) + virCommandAddArgList(cmd, "-global", + "kvm-pit.lost_tick_policy=discard", NULL); + else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_KVM_PIT)) + virCommandAddArg(cmd, "-no-kvm-pit-reinjection"); + break; + case VIR_DOMAIN_TIMER_TICKPOLICY_MERGE: /* no way to support these modes for pit in qemu */ s/these modes/this mode/
ACK w/ these adjustments (I can do this as well if you want)
John
This would be very kind of you as I'll able to fix it on monday only as we have holidays currently. Maxim
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported pit tickpolicy '%s'"), diff --git a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml index 7835a1b..d8ddcba 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-kvm-pit-device.xml @@ -9,7 +9,7 @@ <boot dev='hd'/> </os> <clock offset='utc'> - <timer name='pit' tickpolicy='delay'/> + <timer name='pit' tickpolicy='discard'/> </clock> <on_poweroff>destroy</on_poweroff> <on_reboot>restart</on_reboot> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b8619dd..713a8fe 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2060,6 +2060,7 @@ mymain(void) qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY);
DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG);
As specifying both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY capabilities has no practical sense in tests, and we already have tests for QEMU_CAPS_KVM_PIT_TICK_POLICY, it's better to add a separate one with QEMU_CAPS_NO_KVM_PIT set. Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- .../qemuxml2argv-no-kvm-pit-device.args | 23 +++++++++++++++++ .../qemuxml2argv-no-kvm-pit-device.xml | 29 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 53 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args new file mode 100644 index 0000000..1d69797 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.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 \ +-m 214 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-kvm-pit-reinjection \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml new file mode 100644 index 0000000..d8ddcba --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'> + <timer name='pit' tickpolicy='discard'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 713a8fe..77af591 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2061,6 +2061,7 @@ mymain(void) DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("no-kvm-pit-device", QEMU_CAPS_NO_KVM_PIT); DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG); -- 2.4.11
On 12/09/2016 09:28 AM, Maxim Nestratov wrote:
As specifying both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY capabilities has no practical sense in tests, and we already have tests for QEMU_CAPS_KVM_PIT_TICK_POLICY, it's better to add a separate one with QEMU_CAPS_NO_KVM_PIT set.
My suggested alteration: tests: Add "no-kvm-pit-device" testcase Add a test case for when the QEMU_CAPS_NO_KVM_PIT capability is set. This capability is mutually exclusive to QEMU_CAPS_KVM_PIT_TICK_POLICY and results in the same output regardless of whether "discard" or "delay" was specified in the guest XML for 'tickpolicy'. ACK - John
Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com> --- .../qemuxml2argv-no-kvm-pit-device.args | 23 +++++++++++++++++ .../qemuxml2argv-no-kvm-pit-device.xml | 29 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 53 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.args new file mode 100644 index 0000000..1d69797 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.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 \ +-m 214 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ +-no-kvm-pit-reinjection \ +-no-acpi \ +-boot c \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml new file mode 100644 index 0000000..d8ddcba --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-no-kvm-pit-device.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'> + <timer name='pit' tickpolicy='discard'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 713a8fe..77af591 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2061,6 +2061,7 @@ mymain(void)
DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); DO_TEST("kvm-pit-device", QEMU_CAPS_KVM_PIT_TICK_POLICY); + DO_TEST("no-kvm-pit-device", QEMU_CAPS_NO_KVM_PIT);
DO_TEST("panic", QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_NODEFCONFIG);
05-Jan-17 19:53, John Ferlan пишет:
On 12/09/2016 09:28 AM, Maxim Nestratov wrote:
As specifying both QEMU_CAPS_NO_KVM_PIT and QEMU_CAPS_KVM_PIT_TICK_POLICY capabilities has no practical sense in tests, and we already have tests for QEMU_CAPS_KVM_PIT_TICK_POLICY, it's better to add a separate one with QEMU_CAPS_NO_KVM_PIT set. My suggested alteration:
tests: Add "no-kvm-pit-device" testcase
Add a test case for when the QEMU_CAPS_NO_KVM_PIT capability is set. This capability is mutually exclusive to QEMU_CAPS_KVM_PIT_TICK_POLICY and results in the same output regardless of whether "discard" or "delay" was specified in the guest XML for 'tickpolicy'.
ACK -
John
Your wording is better. Thanks for the review. Maxim
participants (2)
-
John Ferlan -
Maxim Nestratov