search

sandia-minimega / minimega

minimega
GNU General Public License v3.0
148 stars 67 forks source link

Fix for the CPU core count issue #1528

Closed Narratot closed 4 months ago

Narratot commented 5 months ago

This fix will enable the use of vm.VCPUs as a single argument to create VMs with the right core count.

activeshadow commented 5 months ago

See #1527 for additional details.

jacdavi commented 5 months ago

I might be understanding incorrectly, but it seems the current code should be working correctly? vm.VCPUS defines the number of CPUs the VM should have, not the core count. So typically you'd leave vcpus at the default of 1 then just config vm.Cores to your desired value.

@activeshadow I saw your PR in phenix and I think that addresses the issue. One other thing is maybe the dropdown in StoppedExperiment should control core count and not vcpus? I'm guessing users would more often want to change the core count.

Narratot commented 5 months ago

@jacdavi From my understanding that option should be obsolete in QEMU.

I am with you, that it should work correctly. However it does not (both within Minimega and using Qemu directly). My fix is intended to match my expectations, where I would expect to end up with the number of specified cores within my guest. And from my knowledge it used to work that way. But with a change within QEMU my archivable maximum without my change was to get 2 Cores both within Linux and Windows guest environments. From my understanding there was a change in the smp configuration with QEMU 5.2, so that QEMU does automatically compute something with the - smp vm.VCPUs but at least in my tests that did not lead to the expected results. So my change should keep the indetended configuration if specified, if only the VCPUs or cores is specified it adds a single socket to present the expected result to the user.

jacdavi commented 4 months ago

Ok I took a closer look at the QEMU source. It looks like starting in 6.2 the algorithm changed to prefer using cores over sockets when calculating the remaining values. This aligns with a test where I used vm config vcpus 5 with two versions of QEMU then ran lscpu in the vm.

qemu_smp

However, I am still hesitant to make the changes suggested. It may now be necessary to specify additional arguments in minimega to get your desired configuration, but the smp argument format in QEMU has not changed, and I don't want minimega to break that convention. It would be unintuitive if minimega's vcpus parameter set the cores value in QEMU, and the currently proposed changes would not allow setting the number of cpus at all.

Does it sound ok to specify your desired core count explicitly in phenix using the PR open there? That still seems like the better approach to me.

Narratot commented 4 months ago

@jacdavi That states the problem as the docker is still using the 4.2 Qemu version. I will test it with Ubuntu 22.04 as a base image instead of Ubuntu 20.04. As this will use Qemu 6.2 instead of 4.2 which should also resolve the error.

Narratot commented 4 months ago

A short update, from my testing the last commit should also fix the problem. If that is okay for you I would close this request and open another for changing the docker version from 20.04 to 22.04