1. Patchew
  2. QEMU
  3. [PATCH v9 00/10] s390x: CPU Topology
  4. View patch

[PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear

Pierre Morel posted 10 patches 3 years, 5 months ago
There is a newer version of this series
[PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 5 months ago 
S390x do not support multithreading in the guest.
Do not let admin falsely specify multithreading on QEMU
smp commandline.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 hw/s390x/s390-virtio-ccw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 70229b102b..b5ca154e2f 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     int i;
 
+    /* Explicitely do not support threads */
+    assert(machine->smp.threads == 1);
+
     /* initialize possible_cpus */
     mc->possible_cpu_arch_ids(machine);
 
-- 
2.31.1
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Daniel P. Berrangé 3 years, 4 months ago 
On Fri, Sep 02, 2022 at 09:55:22AM +0200, Pierre Morel wrote:
> S390x do not support multithreading in the guest.
> Do not let admin falsely specify multithreading on QEMU
> smp commandline.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 70229b102b..b5ca154e2f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      int i;
>  
> +    /* Explicitely do not support threads */
> +    assert(machine->smp.threads == 1);

What is the functional effect for currently released QEMU versions
if a user has set threads == 2  for an s390 machine ?  Is the
threads setting simply ignored ?

If we want to eliminate this mistake, then there's two possible
options

  * If it had no effect, treat this like a deprecation process
    where we print a warning for 2 releases, and then turn the
    warning into an error. Gives a little grace to fix the config
    mistakes some users might have made, at a time convenient to
    them.

Or

  * If it had effect and we need migration compatibility then forbid
    threads > 1 only for new machine type versions, so existing
    deployed guests are not changed.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 4 months ago 

On 9/28/22 20:11, Daniel P. Berrangé wrote:
> On Fri, Sep 02, 2022 at 09:55:22AM +0200, Pierre Morel wrote:
>> S390x do not support multithreading in the guest.
>> Do not let admin falsely specify multithreading on QEMU
>> smp commandline.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   hw/s390x/s390-virtio-ccw.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 70229b102b..b5ca154e2f 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>       int i;
>>   
>> +    /* Explicitely do not support threads */
>> +    assert(machine->smp.threads == 1);
> 
> What is the functional effect for currently released QEMU versions
> if a user has set threads == 2  for an s390 machine ?  Is the
> threads setting simply ignored ?

It is not ignored, the number of CPUs per sockets seen by the guest is 
cores*threads

> 
> If we want to eliminate this mistake, then there's two possible
> options
> 
>    * If it had no effect, treat this like a deprecation process
>      where we print a warning for 2 releases, and then turn the
>      warning into an error. Gives a little grace to fix the config
>      mistakes some users might have made, at a time convenient to
>      them.
> 
> Or
> 
>    * If it had effect and we need migration compatibility then forbid
>      threads > 1 only for new machine type versions, so existing
>      deployed guests are not changed.
> 
> With regards,
> Daniel

Thanks for your comments Daniel.
I will need to forbid threads > 1 for new machine.

regards,
Pierre




-- 
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Nico Boehr 3 years, 5 months ago 
Quoting Pierre Morel (2022-09-02 09:55:22)
> S390x do not support multithreading in the guest.
> Do not let admin falsely specify multithreading on QEMU
> smp commandline.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  hw/s390x/s390-virtio-ccw.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 70229b102b..b5ca154e2f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      int i;
>  
> +    /* Explicitely do not support threads */
          ^
          Explicitly

> +    assert(machine->smp.threads == 1);

It might be nicer to give a better error message to the user.
What do you think about something like (broken whitespace ahead):

    if (machine->smp.threads != 1) {
        error_setg(&error_fatal, "More than one thread specified, but multithreading unsupported");
        return;
    }
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 5 months ago 

On 9/5/22 13:32, Nico Boehr wrote:
> Quoting Pierre Morel (2022-09-02 09:55:22)
>> S390x do not support multithreading in the guest.
>> Do not let admin falsely specify multithreading on QEMU
>> smp commandline.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   hw/s390x/s390-virtio-ccw.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 70229b102b..b5ca154e2f 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>       int i;
>>   
>> +    /* Explicitely do not support threads */
>            ^
>            Explicitly
> 
>> +    assert(machine->smp.threads == 1);
> 
> It might be nicer to give a better error message to the user.
> What do you think about something like (broken whitespace ahead):
> 
>      if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
>          error_setg(&error_fatal, "More than one thread specified, but multithreading unsupported");
>          return;
>      }
> 


OK, I think I wanted to do this and I changed my mind, obviously, I do 
not recall why.
I will do almost the same but after a look at error.h I will use 
error_report()/exit() instead of error_setg()/return as in:


+    /* Explicitly do not support threads */
+    if (machine->smp.threads != 1) {
+        error_report("More than one thread specified, but 
multithreading unsupported");
+        exit(1);
+    }


Thanks,

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Cédric Le Goater 3 years, 4 months ago 
On 9/5/22 17:10, Pierre Morel wrote:
> 
> 
> On 9/5/22 13:32, Nico Boehr wrote:
>> Quoting Pierre Morel (2022-09-02 09:55:22)
>>> S390x do not support multithreading in the guest.
>>> Do not let admin falsely specify multithreading on QEMU
>>> smp commandline.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>   hw/s390x/s390-virtio-ccw.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 70229b102b..b5ca154e2f 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>       int i;
>>> +    /* Explicitely do not support threads */
>>            ^
>>            Explicitly
>>
>>> +    assert(machine->smp.threads == 1);
>>
>> It might be nicer to give a better error message to the user.
>> What do you think about something like (broken whitespace ahead):
>>
>>      if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
>>          error_setg(&error_fatal, "More than one thread specified, but multithreading unsupported");
>>          return;
>>      }
>>
> 
> 
> OK, I think I wanted to do this and I changed my mind, obviously, I do not recall why.
> I will do almost the same but after a look at error.h I will use error_report()/exit() instead of error_setg()/return as in:
> 
> 
> +    /* Explicitly do not support threads */
> +    if (machine->smp.threads != 1) {
> +        error_report("More than one thread specified, but multithreading unsupported");
> +        exit(1);
> +    }


or add an 'Error **errp' parameter to s390_init_cpus() and use error_setg()
as initially proposed. s390x_new_cpu() would benefit from it also.

Thanks,

C.


> 
> 
> Thanks,
> 
> Regards,
> Pierre
>
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 4 months ago 

On 9/27/22 11:44, Cédric Le Goater wrote:
> On 9/5/22 17:10, Pierre Morel wrote:
>>
>>
>> On 9/5/22 13:32, Nico Boehr wrote:
>>> Quoting Pierre Morel (2022-09-02 09:55:22)
>>>> S390x do not support multithreading in the guest.
>>>> Do not let admin falsely specify multithreading on QEMU
>>>> smp commandline.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>   hw/s390x/s390-virtio-ccw.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 70229b102b..b5ca154e2f 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>>       int i;
>>>> +    /* Explicitely do not support threads */
>>>            ^
>>>            Explicitly
>>>
>>>> +    assert(machine->smp.threads == 1);
>>>
>>> It might be nicer to give a better error message to the user.
>>> What do you think about something like (broken whitespace ahead):
>>>
>>>      if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
>>>          error_setg(&error_fatal, "More than one thread specified, 
>>> but multithreading unsupported");
>>>          return;
>>>      }
>>>
>>
>>
>> OK, I think I wanted to do this and I changed my mind, obviously, I do 
>> not recall why.
>> I will do almost the same but after a look at error.h I will use 
>> error_report()/exit() instead of error_setg()/return as in:
>>
>>
>> +    /* Explicitly do not support threads */
>> +    if (machine->smp.threads != 1) {
>> +        error_report("More than one thread specified, but 
>> multithreading unsupported");
>> +        exit(1);
>> +    }
> 
> 
> or add an 'Error **errp' parameter to s390_init_cpus() and use error_setg()
> as initially proposed. s390x_new_cpu() would benefit from it also.
> 
OK, Thanks,

Pierre

-- 
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 4 months ago 
More thinking about this I will drop this patch for backward 
compatibility and in topology masks treat CPUs as being cores*threads



On 9/28/22 15:21, Pierre Morel wrote:
> 
> 
> On 9/27/22 11:44, Cédric Le Goater wrote:
>> On 9/5/22 17:10, Pierre Morel wrote:
>>>
>>>
>>> On 9/5/22 13:32, Nico Boehr wrote:
>>>> Quoting Pierre Morel (2022-09-02 09:55:22)
>>>>> S390x do not support multithreading in the guest.
>>>>> Do not let admin falsely specify multithreading on QEMU
>>>>> smp commandline.
>>>>>
>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>> ---
>>>>>   hw/s390x/s390-virtio-ccw.c | 3 +++
>>>>>   1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>>> index 70229b102b..b5ca154e2f 100644
>>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>>>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>>>       int i;
>>>>> +    /* Explicitely do not support threads */
>>>>            ^
>>>>            Explicitly
>>>>
>>>>> +    assert(machine->smp.threads == 1);
>>>>
>>>> It might be nicer to give a better error message to the user.
>>>> What do you think about something like (broken whitespace ahead):
>>>>
>>>>      if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
>>>>          error_setg(&error_fatal, "More than one thread specified, 
>>>> but multithreading unsupported");
>>>>          return;
>>>>      }
>>>>
>>>
>>>
>>> OK, I think I wanted to do this and I changed my mind, obviously, I 
>>> do not recall why.
>>> I will do almost the same but after a look at error.h I will use 
>>> error_report()/exit() instead of error_setg()/return as in:
>>>
>>>
>>> +    /* Explicitly do not support threads */
>>> +    if (machine->smp.threads != 1) {
>>> +        error_report("More than one thread specified, but 
>>> multithreading unsupported");
>>> +        exit(1);
>>> +    }
>>
>>
>> or add an 'Error **errp' parameter to s390_init_cpus() and use 
>> error_setg()
>> as initially proposed. s390x_new_cpu() would benefit from it also.
>>
> OK, Thanks,
> 
> Pierre
> 

-- 
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Cédric Le Goater 3 years, 4 months ago 
On 9/28/22 18:16, Pierre Morel wrote:
> More thinking about this I will drop this patch for backward compatibility and in topology masks treat CPUs as being cores*threads

yes. You never know, people might have set threads=2 in their
domain file (like me). You could give the user a warning though,
with warn_report().

Thanks,

C.

  
> 
> 
> 
> On 9/28/22 15:21, Pierre Morel wrote:
>>
>>
>> On 9/27/22 11:44, Cédric Le Goater wrote:
>>> On 9/5/22 17:10, Pierre Morel wrote:
>>>>
>>>>
>>>> On 9/5/22 13:32, Nico Boehr wrote:
>>>>> Quoting Pierre Morel (2022-09-02 09:55:22)
>>>>>> S390x do not support multithreading in the guest.
>>>>>> Do not let admin falsely specify multithreading on QEMU
>>>>>> smp commandline.
>>>>>>
>>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>>> ---
>>>>>>   hw/s390x/s390-virtio-ccw.c | 3 +++
>>>>>>   1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>>>> index 70229b102b..b5ca154e2f 100644
>>>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>>>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>>>>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>>>>       int i;
>>>>>> +    /* Explicitely do not support threads */
>>>>>            ^
>>>>>            Explicitly
>>>>>
>>>>>> +    assert(machine->smp.threads == 1);
>>>>>
>>>>> It might be nicer to give a better error message to the user.
>>>>> What do you think about something like (broken whitespace ahead):
>>>>>
>>>>>      if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
>>>>>          error_setg(&error_fatal, "More than one thread specified, but multithreading unsupported");
>>>>>          return;
>>>>>      }
>>>>>
>>>>
>>>>
>>>> OK, I think I wanted to do this and I changed my mind, obviously, I do not recall why.
>>>> I will do almost the same but after a look at error.h I will use error_report()/exit() instead of error_setg()/return as in:
>>>>
>>>>
>>>> +    /* Explicitly do not support threads */
>>>> +    if (machine->smp.threads != 1) {
>>>> +        error_report("More than one thread specified, but multithreading unsupported");
>>>> +        exit(1);
>>>> +    }
>>>
>>>
>>> or add an 'Error **errp' parameter to s390_init_cpus() and use error_setg()
>>> as initially proposed. s390x_new_cpu() would benefit from it also.
>>>
>> OK, Thanks,
>>
>> Pierre
>>
>
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 4 months ago 

On 9/28/22 18:28, Cédric Le Goater wrote:
> On 9/28/22 18:16, Pierre Morel wrote:
>> More thinking about this I will drop this patch for backward 
>> compatibility and in topology masks treat CPUs as being cores*threads
> 
> yes. You never know, people might have set threads=2 in their
> domain file (like me). You could give the user a warning though,
> with warn_report().

More thinking, I come back to the first idea after Daniel comment and 
protect the change with a new machine type version.


> 
> Thanks,
> 
> C.
> 
> 
>>

-- 
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Cédric Le Goater 3 years, 4 months ago 
On 10/11/22 09:21, Pierre Morel wrote:
> 
> 
> On 9/28/22 18:28, Cédric Le Goater wrote:
>> On 9/28/22 18:16, Pierre Morel wrote:
>>> More thinking about this I will drop this patch for backward compatibility and in topology masks treat CPUs as being cores*threads
>>
>> yes. You never know, people might have set threads=2 in their
>> domain file (like me). You could give the user a warning though,
>> with warn_report().
> 
> More thinking, I come back to the first idea after Daniel comment and protect the change with a new machine type version.

yes. That would be another machine class attribute to set in the new machine,
may be 'max_threads' to compare with the user provided value.

C.

> 
> 
>>
>> Thanks,
>>
>> C.
>>
>>
>>>
>
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Janis Schoetterl-Glausch 3 years, 5 months ago 
On Mon, 2022-09-05 at 17:10 +0200, Pierre Morel wrote:
> 
> On 9/5/22 13:32, Nico Boehr wrote:
> > Quoting Pierre Morel (2022-09-02 09:55:22)
> > > S390x do not support multithreading in the guest.
> > > Do not let admin falsely specify multithreading on QEMU
> > > smp commandline.
> > > 
> > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > > ---
> > >   hw/s390x/s390-virtio-ccw.c | 3 +++
> > >   1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > > index 70229b102b..b5ca154e2f 100644
> > > --- a/hw/s390x/s390-virtio-ccw.c
> > > +++ b/hw/s390x/s390-virtio-ccw.c
> > > @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
> > >       MachineClass *mc = MACHINE_GET_CLASS(machine);
> > >       int i;
> > >   
> > > +    /* Explicitely do not support threads */
> >            ^
> >            Explicitly
> > 
> > > +    assert(machine->smp.threads == 1);
> > 
> > It might be nicer to give a better error message to the user.
> > What do you think about something like (broken whitespace ahead):
> > 
> >      if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
> >          error_setg(&error_fatal, "More than one thread specified, but multithreading unsupported");
> >          return;
> >      }
> > 
> 
> 
> OK, I think I wanted to do this and I changed my mind, obviously, I do 
> not recall why.
> I will do almost the same but after a look at error.h I will use 
> error_report()/exit() instead of error_setg()/return as in:
> 
> 
> +    /* Explicitly do not support threads */
> +    if (machine->smp.threads != 1) {
> +        error_report("More than one thread specified, but 
> multithreading unsupported");
> +        exit(1);
> +    }

I agree that an assert is not a good solution, and I'm not sure
aborting is a good idea either.
I'm assuming that currently if you specify threads > 0 qemu will run
with the number of CPUs multiplied by threads (compared to threads=1).
If that is true, then a new qemu version will break existing
invocations.

An alternative would be to print a warning and do:
cores *= threads
threads = 1

The questions would be what the best place to do that is.
I guess we'd need a new compat variable if that's done in machine-smp.c
> 
> 
> Thanks,
> 
> Regards,
> Pierre
>
Re: [PATCH v9 01/10] s390x/cpus: Make absence of multithreading clear
Posted by Pierre Morel 3 years, 5 months ago 

On 9/5/22 17:23, Janis Schoetterl-Glausch wrote:
> On Mon, 2022-09-05 at 17:10 +0200, Pierre Morel wrote:
>>
>> On 9/5/22 13:32, Nico Boehr wrote:
>>> Quoting Pierre Morel (2022-09-02 09:55:22)
>>>> S390x do not support multithreading in the guest.
>>>> Do not let admin falsely specify multithreading on QEMU
>>>> smp commandline.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    hw/s390x/s390-virtio-ccw.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 70229b102b..b5ca154e2f 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -86,6 +86,9 @@ static void s390_init_cpus(MachineState *machine)
>>>>        MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>>        int i;
>>>>    
>>>> +    /* Explicitely do not support threads */
>>>             ^
>>>             Explicitly
>>>
>>>> +    assert(machine->smp.threads == 1);
>>>
>>> It might be nicer to give a better error message to the user.
>>> What do you think about something like (broken whitespace ahead):
>>>
>>>       if (machine->smp.threads != 1) {if (machine->smp.threads != 1) {
>>>           error_setg(&error_fatal, "More than one thread specified, but multithreading unsupported");
>>>           return;
>>>       }
>>>
>>
>>
>> OK, I think I wanted to do this and I changed my mind, obviously, I do
>> not recall why.
>> I will do almost the same but after a look at error.h I will use
>> error_report()/exit() instead of error_setg()/return as in:
>>
>>
>> +    /* Explicitly do not support threads */
>> +    if (machine->smp.threads != 1) {
>> +        error_report("More than one thread specified, but
>> multithreading unsupported");
>> +        exit(1);
>> +    }
> 
> I agree that an assert is not a good solution, and I'm not sure
> aborting is a good idea either.
> I'm assuming that currently if you specify threads > 0 qemu will run
> with the number of CPUs multiplied by threads (compared to threads=1).
> If that is true, then a new qemu version will break existing
> invocations.
> 
> An alternative would be to print a warning and do:
> cores *= threads
> threads = 1
> 
> The questions would be what the best place to do that is.
> I guess we'd need a new compat variable if that's done in machine-smp.c

Right, I think we can use the new "topology_disable" machine property.


>>
>>
>> Thanks,
>>
>> Regards,
>> Pierre
>>
> 

-- 
Pierre Morel
IBM Lab Boeblingen

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.