target/s390x/kvm.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
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 <jdenemar@redhat.com>
Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
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);
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 - "
--
2.21.0
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 <jdenemar@redhat.com>
> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> 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" ?
> 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?
Anyway, your patch sounds fine, so (with "klass" replaced by "oc"):
Reviewed-by: Thomas Huth <thuth@redhat.com>
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 <jdenemar@redhat.com>
>> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> 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 <thuth@redhat.com>
>
--
Thanks,
David / dhildenb
On 21/10/2019 11.54, David Hildenbrand wrote:
> 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 <jdenemar@redhat.com>
>>> Fixes: b6805e127c6b ("s390x: use generic cpu_model parsing")
>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> ---
>>> 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)
Ah, right, thanks for the explanation. I somehow read the patch
description that it is only fixed for the latest one (I should really
read more closely) ... but now it makes perfect sense.
Thomas
> query-machines probes with the "none" machine all other machines. > Current code only fixes up the active machine. > To be more precise, libvirt probes with "-machine none,accel=kvm:tcg" all other machines. > (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 <thuth@redhat.com> >> > > -- Thanks, David / dhildenb
On Mon, 21 Oct 2019 11:58:35 +0200 David Hildenbrand <david@redhat.com> wrote: > > query-machines probes with the "none" machine all other machines. > > Current code only fixes up the active machine. > > > > To be more precise, libvirt probes with "-machine none,accel=kvm:tcg" > all other machines. Add that to the patch description? > > > (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"): Respin with that? Makes it easier to pick up :) > >> > >> Reviewed-by: Thomas Huth <thuth@redhat.com> > >> > > > > > >
© 2016 - 2025 Red Hat, Inc.