Re: [libvirt] [PATCHv2 3/3] virsh: Use virNodeGetCPUMap if possible
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