Re: [Qemu-discuss] Guests don't start with qemu 2.1.0: smbios_get_tables: Assertion `smbios_smp_sockets >= 1' failed

[Jakob Bohm]

On 8/13/2014 3:22 PM, Brian Rak wrote:
> On 8/13/2014 7:33 AM, Jakob Bohm wrote:
> > On 8/12/2014 8:40 PM, Brian Rak wrote:
> > > I've been testing 2.1.0, and qemu will no longer start up with our CPU
> > > configuration.  I get the error:
> > >
> > > qemu-kvm: /root/qemu/src/hw/i386/smbios.c:825: smbios_get_tables:
> > > Assertion `smbios_smp_sockets >= 1' failed.
> > >
> > > The relevant parts of my command line:
> > >
> > > -smp 4,sockets=2,cores=12,threads=2
> > >
> > > This is ultimately coming from my libvirt config of:
> > >
> > >    <vcpu placement='static'>4</vcpu>
> > >    <cpu mode='custom' match='exact'>
> > >      <topology sockets='2' cores='12' threads='2'/>
> > >    </cpu>
> > >
> > >
> > > Is this configuration no longer supported?  Up until 2.1.0, this was
> > > handled well.. for any CPU count < 12, I got one socket with the correct
> > > number of cores.  For anything over 12 vcpus, I got two sockets.  This
> > > makes Windows licensing happy.
> > >
> > > I took a look at the code, and it seems like it should just cap the
> > > minimum number of sockets at 1, rather then this assert.
> > >
> > > --- ../src_clean/hw/i386/smbios.c       2014-08-01 10:12:17.000000000
> > > -0400
> > > +++ hw/i386/smbios.c    2014-08-12 14:39:40.560485901 -0400
> > > @@ -822,7 +822,7 @@
> > >           smbios_build_type_3_table();
> > >
> > >           smbios_smp_sockets = smp_cpus / (smp_cores * smp_threads);
> > > -        assert(smbios_smp_sockets >= 1);
> > > +       if (smbios_smp_sockets < 1) smbios_smp_sockets = 1;
> > >
> > >           for (i = 0; i < smbios_smp_sockets; i++) {
> > >               smbios_build_type_4_table(i);
> > >
> > > Is there a downside to this patch?
> >
> > Have you checked why the variable is 0 and not 1 (or 2), maybe there is
> > a deeper bug which leaves it at 0, even when it should be 2 (or more).
> >
> > Remember, the place that reports an error is rarely the cause of the
> > problem.
> >
> > Enjoy
> >
> > Jakob
>
> My command line is this:
>
> -smp 4,sockets=2,cores=12,threads=2
>
> Which means, we get:
>
> smbios_smp_sockets = 4 / (12 * 2) = 0.1666
>
> Which means the assert is correct.  This has been a valid configuration
> of CPUs/cores up until 2.1.0.  I'd have to go update the config on all
> of our instances if I wanted to upgrade without this patch :/

Ah, the bug is that the handling of fractional results is wrong, and 
your code just hides it for the case where the result is between 0 and 
1, while doing nothing for fractional results above 1.0.

If smbios_smp_sockets is a floating point type (float or double), then 
the correct fix is to change the assert to

  assert(smbios_smp_sockets > 0.0);

If smbios_smp_sockets is an integer type (int, unsigned, etc.), then the
correct fix is to make the division round up and keep the assert(), like
this:

  assert(smp_cpus >= 1); // Next calculation assumes this
  smbios_smp_sockets = 1 + (smp_cpus - 1) / (smp_cores * smp_threads);
  assert(smbios_smp_sockets >= 1);

Otherwise, consider what happens with the following option settings:

  -smp 28,sockets=2,cores=12,threads=2

Here the original division would result in 1.1666.  Which needs to be
treated as floating point or rounded up to 2 sockets.


Enjoy

Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S.  http://www.wisemo.com
Transformervej 29, 2730 Herlev, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded