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 <pmo...@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
>
