[libvirt] [PATCHv2 3/3] virsh: Use virNodeGetCPUMap if possible

[Viktor Mihajlovski]

On 11/01/2012 04:47 AM, Eric Blake wrote:
> On 10/31/2012 08:58 PM, Hu Tao wrote:
>> Isn't VIR_NODEINFO_MAXCPUS buggy? Either don't fall back to nodeinfo
>> or fix it.
We will always have to fall back if we talk to a backlevel libvirt to 
retain compatibility, see below.
>
> Hmm, and I just realized another issue - on the RHEL 5 box I tested,
> there is no /sys/devices/system/cpu/possible, so virNodeGetCPUCount()
> currently fails, even though virNodeGetInfo() succeeded.  I'm going to
> hold off pushing this series until after 1.0.0, to avoid any chance of a
> last-minute unintentional regression.
Too bad I didn't realize the before, this requies another fallback to 
nodeGetInfo (at least there are no offline CPUs on 5.x).
What do you think of pushing 2/3 and 3/3 which are not prone to 
regression and let me rework 1/1 including the fallback and then decide 
whether to pick it up for 1.0.0 or after?
>
> You were asking about VIR_NODEINFO_MAXCPUS:
>
> #define VIR_NODEINFO_MAXCPUS(nodeinfo)
> ((nodeinfo).nodes*(nodeinfo).sockets*(nodeinfo).cores*(nodeinfo).threads)
>
> I can confirm that virNodeGetInfo misbehaves if enough trailing cpus are
> offline, and therefore agree that we need to fix that API:
>
Actually, this is where I initially started off, see the thread at 
https://www.redhat.com/archives/libvir-list/2012-October/msg00399.html
and specifically Daniel's comments on compatibility.

Obviously my first attempt was flawed/naive since it was breaking binary 
compatibility. One possible attempt to fix virGetNodeInfo would have 
been to change the meaning of the virNodeInfo.cpus field. But that would 
be semantically incorrect as the field is denominated as the number of 
active CPUs. Fixing the core/socket/thread detection doesn't seem 
possible using the sysfs interfaces.
Now, the next straight-forward thing might have been a virNodeGetInfo2 
or similar but I thought an API for the host CPU map might be more 
versatile in the long run.

All that said ... it seems that we have to live with the flawed 
semantics of VIR_NODEINFO_MAXCPUS. This series should alleviate this 
problem while still retaining the old semantics (by means of fallback) 
even if a new client talks to an old server.

-- 

Mit freundlichen Grüßen/Kind Regards
    Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294