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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxxxx>
---
   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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux