[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]