On 21.10.19 11:52, Thomas Huth wrote:
On 21/10/2019 11.34, David Hildenbrand wrote:We have to set the default model of all machine classes, not just for the active one. Otherwise, "query-machines" will indicate the wrong CPU model ("qemu-s390x-cpu" instead of "host-s390x-cpu") as "default-cpu-type".Doing a {"execute":"query-machines"} under KVM now results in {"return": [ { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-4.0", "numa-mem-supported": false, "default-cpu-type": "host-s390x-cpu", "cpu-max": 248, "deprecated": false}, { "hotpluggable-cpus": true, "name": "s390-ccw-virtio-2.7", "numa-mem-supported": false, "default-cpu-type": "host-s390x-cpu", "cpu-max": 248, "deprecated": false } ... Reported-by: Jiri Denemark <[email protected]> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing") Cc: Igor Mammedov <[email protected]> Signed-off-by: David Hildenbrand <[email protected]> --- target/s390x/kvm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index c24c869e77..5966ab0d37 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -320,11 +320,17 @@ void kvm_s390_set_max_pagesize(uint64_t pagesize, Error **errp) cap_hpage_1m = 1; }-int kvm_arch_init(MachineState *ms, KVMState *s)+static void ccw_machine_class_foreach(ObjectClass *klass, void *opaque) { - MachineClass *mc = MACHINE_GET_CLASS(ms); + MachineClass *mc = MACHINE_CLASS(klass);I think we rather wanted to avoid using "klass" in new code... maybe use "oc" instead of "klass" ?
Can do, this was a copy and paste :)
mc->default_cpu_type = S390_CPU_TYPE_NAME("host"); +} + +int kvm_arch_init(MachineState *ms, KVMState *s) +{ + object_class_foreach(ccw_machine_class_foreach, TYPE_S390_CCW_MACHINE, + false, NULL);if (!kvm_check_extension(kvm_state, KVM_CAP_DEVICE_CTRL)) {error_report("KVM is missing capability KVM_CAP_DEVICE_CTRL - "Weird, if you start an older machine, you still get the "host" CPU without your patch, too: echo -e "info qom-tree \n quit" | \ qemu-system-s390x -display none -monitor stdio -no-shutdown \ -accel kvm -M s390-ccw-virtio-3.0 | grep s390x-cpu Results in: /device[0] (host-s390x-cpu) ... so I wonder why that differs from the "query-machines" command?
query-machines probes with the "none" machine all other machines. Current code only fixes up the active machine.
(that's why you won't notice when starting a machine - you will always get "host" for the active one)
Anyway, your patch sounds fine, so (with "klass" replaced by "oc"): Reviewed-by: Thomas Huth <[email protected]>
-- Thanks, David / dhildenb
