On 10/08/2012 07:52 PM, Peter Krempa wrote: > On 10/08/12 19:32, Martin Kletzander wrote: >> When both kvmclock and kvm_pv_eoi are configured (either disabled or >> enabled) libvirt will generate invalid CPU specification due to the >> fact that even though kvmclock causes the CPU to be specified, it >> doesn't set have_cpu flag to true (and the new kvm_pv_eoi as well). >> This patch fixes the issue and adds a test exactly for that to show >> that it is fixed correctly (and also to keep it that way in the future >> of course). >> --- >> src/qemu/qemu_command.c | 2 ++ >> .../qemuxml2argv-kvmclock+eoi-disabled.args | 4 ++++ >> .../qemuxml2argv-kvmclock+eoi-disabled.xml | 27 >> ++++++++++++++++++++++ >> tests/qemuxml2argvtest.c | 1 + >> 4 files changed, 34 insertions(+) >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-kvmclock+eoi-disabled.args >> create mode 100644 >> tests/qemuxml2argvdata/qemuxml2argv-kvmclock+eoi-disabled.xml >> >> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c >> index 20730a9..09f412e 100644 >> --- a/src/qemu/qemu_command.c >> +++ b/src/qemu/qemu_command.c >> @@ -4210,6 +4210,7 @@ qemuBuildCpuArgStr(const struct qemud_driver >> *driver, >> virBufferAsprintf(&buf, "%s,%ckvmclock", >> have_cpu ? "" : default_model, >> sign); >> + have_cpu = true; >> break; >> } >> } >> @@ -4224,6 +4225,7 @@ qemuBuildCpuArgStr(const struct qemud_driver >> *driver, >> virBufferAsprintf(&buf, "%s,%ckvm_pv_eoi", >> have_cpu ? "" : default_model, >> sign); >> + have_cpu = true; > > This assignment has no effect, the value isn't used elsewhere. Although > it probably would make sense to keep it if somebody else tries to > improve this function (probably this was the source of the bug fixed by > this patch). > That's exactly why I've put it there. The last CPU-adding statement missed this and I've forgot to put it there. This way it'll work next time somebody adds something new :) >> } >> >> if (virBufferError(&buf)) > > ACK regardless of the fate of the last assignment. > > Peter > Thanks, pushed. Martin -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list