[libvirt PATCH] qemu: fix detection of vCPU pids when multiple dies are present

[Daniel P. Berrangé]

On Mon, Mar 16, 2020 at 01:06:47PM +0000, Daniel P. Berrangé wrote:
> On Mon, Mar 16, 2020 at 09:53:13AM +0100, Peter Krempa wrote:
> > On Fri, Mar 13, 2020 at 16:46:16 +0000, Daniel Berrange wrote:
> > > The logic for querying hotpluggable CPUs needs to sort the list
> > > of CPUs returned by QEMU. Unfortunately our sorting method failed
> > > to use the die_id field, so CPUs were not correctly sorted.
> > > 
> > > This is seen when configuring a guest with partially populated
> > > CPUs
> > > 
> > >   <vcpu placement='static' current='1'>16</vcpu>
> > >   <cpu...>
> > >     <topology sockets='4' dies='2' cores='1' threads='2'/>
> > >   </cpu>
> > > 
> > > Then trying to start it would fail:
> > > 
> > >   # virsh -c qemu:///system start demo
> > >   error: Failed to start domain demo
> > >   error: internal error: qemu didn't report thread id for vcpu '0'
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> > > ---
> > >  src/qemu/qemu_domain.c       | 10 ++++++++--
> > >  src/qemu/qemu_monitor.c      |  2 ++
> > >  src/qemu/qemu_monitor.h      |  2 ++
> > >  src/qemu/qemu_monitor_json.c |  4 ++++
> > >  4 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > Please add a test entry to 'testQemuMonitorCPUInfo' test case for this
> > purpose. You'll need to extend testQemuMonitorCPUInfoFormat.
> 
> That test suite doesn't illustrate the brokenness AFAICT. I added
> the data files and it still passes, even without the fix.

Actually this was a bug in the test case addition which made it silently
do the wrong thing.

I passed current CPUs instead of max CPUs to the test case and thus

qemuMonitorGetCPUInfoHotplug() ended up hitting:

    if (totalvcpus != maxvcpus) {
        VIR_DEBUG("expected '%zu' total vcpus got '%zu'", maxvcpus, totalvcpus);
        return -1;
    }

qemuMonitorGetCPUInfo()  ignores this error though and falls back to
reporting some generic info. I'm not convinced that qemuMonitorGetCPUInfo
is right in ignoring all errors from qemuMonitorGetCPUInfoHotplug but
I've not investigated further.


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 :|