On Sat, Oct 08, 2011 at 12:07:08AM +0530, Prerna Saxena wrote:
> Libvirt presently depends on /proc/cpuinfo to gather information about
> the x86 host. Parsing of /proc/cpuinfo is arch-specific; the
> information fields are also not consistent.
> A cleaner way would be to use Sysfs. Both x86 and PowerPC specific
> information can be obtained from sysfs with different arch specific
> parsing routines.
>
> ---
> src/nodeinfo.c | 106
> +++++++++++++++++++-------------------------------------
> 1 files changed, 36 insertions(+), 70 deletions(-)
>
> diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> index 6448b79..cdd339d 100644
> --- a/src/nodeinfo.c
> +++ b/src/nodeinfo.c
> @@ -30,6 +30,7 @@
> #include <errno.h>
> #include <dirent.h>
> #include <sys/utsname.h>
> +#include <sched.h>
>
> #if HAVE_NUMACTL
> # define NUMA_VERSION1_COMPATIBILITY 1
> @@ -191,6 +192,11 @@ static int parse_socket(unsigned int cpu)
> return ret;
> }
>
> +static int parse_core(unsigned int cpu)
> +{
> + return get_cpu_value(cpu, "topology/core_id", false);
> +}
> +
> int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
> virNodeInfoPtr nodeinfo,
> bool need_hyperthreads)
> @@ -199,15 +205,14 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
> DIR *cpudir = NULL;
> struct dirent *cpudirent = NULL;
> unsigned int cpu;
> - unsigned long cur_threads;
> - int socket;
> - unsigned long long socket_mask = 0;
> - unsigned int remaining;
> + unsigned long core, socket, cur_threads;
> + cpu_set_t core_mask;
> + cpu_set_t socket_mask;
> int online;
>
> nodeinfo->cpus = 0;
> nodeinfo->mhz = 0;
> - nodeinfo->cores = 1;
> + nodeinfo->cores = 0;
>
> nodeinfo->nodes = 1;
> # if HAVE_NUMACTL
> @@ -221,20 +226,10 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
> /* NOTE: hyperthreads are ignored here; they are parsed out of /sys */
> while (fgets(line, sizeof(line), cpuinfo) != NULL) {
> char *buf = line;
> - if (STRPREFIX(buf, "processor")) { /* aka a single logical CPU */
> - buf += 9;
> - while (*buf && c_isspace(*buf))
> - buf++;
> - if (*buf != ':') {
> - nodeReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("parsing cpuinfo processor"));
> - return -1;
> - }
> - nodeinfo->cpus++;
> # if defined(__x86_64__) || \
> defined(__amd64__) || \
> defined(__i386__)
> - } else if (STRPREFIX(buf, "cpu MHz")) {
> + if (STRPREFIX(buf, "cpu MHz")) {
> char *p;
> unsigned int ui;
> buf += 9;
> @@ -249,24 +244,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
> /* Accept trailing fractional part. */
> && (*p == '\0' || *p == '.' || c_isspace(*p)))
> nodeinfo->mhz = ui;
> - } else if (STRPREFIX(buf, "cpu cores")) { /* aka cores */
> - char *p;
> - unsigned int id;
> - buf += 9;
> - while (*buf && c_isspace(*buf))
> - buf++;
> - if (*buf != ':' || !buf[1]) {
> - nodeReportError(VIR_ERR_INTERNAL_ERROR,
> - _("parsing cpuinfo cpu cores %c"), *buf);
> - return -1;
> - }
> - if (virStrToLong_ui(buf+1, &p, 10, &id) == 0
> - && (*p == '\0' || c_isspace(*p))
> - && id > nodeinfo->cores)
> - nodeinfo->cores = id;
> # elif defined(__powerpc__) || \
> defined(__powerpc64__)
> - } else if (STRPREFIX(buf, "clock")) {
> + if (STRPREFIX(buf, "clock")) {
> char *p;
> unsigned int ui;
> buf += 5;
> @@ -281,53 +261,30 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
> /* Accept trailing fractional part. */
> && (*p == '\0' || *p == '.' || c_isspace(*p)))
> nodeinfo->mhz = ui;
> -# elif defined(__s390__) || \
> - defined(__s390x__)
> - } else if (STRPREFIX(buf, "# processors")) {
> - char *p;
> - unsigned int ui;
> - buf += 12;
> - while (*buf && c_isspace(*buf))
> - buf++;
> - if (*buf != ':' || !buf[1]) {
> - nodeReportError(VIR_ERR_INTERNAL_ERROR,
> - _("parsing number of processors %c"), *buf);
> - return -1;
> - }
> - if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0
> - && (*p == '\0' || c_isspace(*p)))
> - nodeinfo->cpus = ui;
> /* No other interesting infos are available in /proc/cpuinfo.
> * However, there is a line identifying processor's version,
> * identification and machine, but we don't want it to be caught
> * and parsed in next iteration, because it is not in expected
> * format and thus lead to error. */
> - break;
> # else
> # warning Parser for /proc/cpuinfo needs to be adapted for your architecture
> # endif
> }
> }
>
> - if (!nodeinfo->cpus) {
> - nodeReportError(VIR_ERR_INTERNAL_ERROR,
> - "%s", _("no cpus found"));
> - return -1;
> - }
> -
> - if (!need_hyperthreads)
> - return 0;
> -
> - /* OK, we've parsed what we can out of /proc/cpuinfo. Get the socket
> - * and thread information from /sys
> + /* OK, we've parsed clock speed out of /proc/cpuinfo. Get the core,
> socket
> + * thread and topology information from /sys
> */
> - remaining = nodeinfo->cpus;
> cpudir = opendir(CPU_SYS_PATH);
> if (cpudir == NULL) {
> virReportSystemError(errno, _("cannot opendir %s"), CPU_SYS_PATH);
> return -1;
> }
> - while ((errno = 0), remaining && (cpudirent = readdir(cpudir))) {
> +
> + CPU_ZERO(&core_mask);
> + CPU_ZERO(&socket_mask);
> +
> + while (cpudirent = readdir(cpudir)) {
> if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1)
> continue;
>
> @@ -338,15 +295,19 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
> }
> if (!online)
> continue;
> - remaining--;
> + nodeinfo->cpus++;
>
> - socket = parse_socket(cpu);
> - if (socket < 0) {
> - closedir(cpudir);
> - return -1;
> + /* Parse core */
> + core = parse_core(cpu);
> + if (!CPU_ISSET(core, &core_mask)) {
> + CPU_SET(core, &core_mask);
> + nodeinfo->cores++;
> }
> - if (!(socket_mask & (1 << socket))) {
> - socket_mask |= (1 << socket);
> +
> + /* Parse socket */
> + socket = parse_socket(cpu);
> + if (!CPU_ISSET(socket, &socket_mask)) {
> + CPU_SET(socket, &socket_mask);
> nodeinfo->sockets++;
> }
>
> @@ -367,7 +328,12 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo,
>
> closedir(cpudir);
>
> - /* there should always be at least one socket and one thread */
> + /* there should always be at least one cpu, socket and one thread */
> + if (nodeinfo->cpus == 0) {
> + nodeReportError(VIR_ERR_INTERNAL_ERROR,
> + "%s", _("no CPUs found"));
> + return -1;
> + }
> if (nodeinfo->sockets == 0) {
> nodeReportError(VIR_ERR_INTERNAL_ERROR,
> "%s", _("no sockets found"));
I checked the output of 'virsh nodeinfo' before and after this patch on
RHEL5 and RHEL-6 hosts, and it did not change.
When building, however, you get a compiele warning about the unused
parameter 'need_hyperthreads'. How come this was needed before, but
is not used anymore ? If it is redundant, you should remove the
parameter entirely.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
