[libvirt] [PATCH 2/6] util: Introduce virHostCPUGetInfoParseCPUInfo()

[Bjoern Walk]

Andrea Bolognani <abologna at redhat.com> [2017-12-11, 05:40PM +0100]:
> This handles only the /proc/cpuinfo part of virHostCPUGetInfo().
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/util/virhostcpu.c | 60 +++++++++++++++++++++++++++++++++------------------
>  1 file changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index c485a9721..4d5c56659 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -508,33 +508,17 @@ virHostCPUHasValidSubcoreConfiguration(int threads_per_subcore)
>      return ret;
>  }
> 
> -int
> -virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
> -                               virArch arch,
> -                               unsigned int *cpus,
> -                               unsigned int *mhz,
> -                               unsigned int *nodes,
> -                               unsigned int *sockets,
> -                               unsigned int *cores,
> -                               unsigned int *threads)
> +
> +static int
> +virHostCPUGetInfoParseCPUInfo(FILE *cpuinfo,
> +                              virArch arch,
> +                              unsigned int *mhz)
>  {
> -    virBitmapPtr present_cpus_map = NULL;
> -    virBitmapPtr online_cpus_map = NULL;
>      char line[1024];
> -    DIR *nodedir = NULL;
> -    struct dirent *nodedirent = NULL;
> -    int nodecpus, nodecores, nodesockets, nodethreads, offline = 0;
> -    int threads_per_subcore = 0;
> -    unsigned int node;
>      int ret = -1;
> -    char *sysfs_nodedir = NULL;
> -    char *sysfs_cpudir = NULL;
> -    int direrr;
> 
>      *mhz = 0;

I wouldn't move this initialization.

> -    *cpus = *nodes = *sockets = *cores = *threads = 0;
> 
> -    /* Start with parsing CPU clock speed from /proc/cpuinfo */
>      while (fgets(line, sizeof(line), cpuinfo) != NULL) {
>          if (ARCH_IS_X86(arch)) {
>              char *buf = line;
> @@ -614,6 +598,40 @@ virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
>          }
>      }
> 
> +    ret = 0;
> +
> + cleanup:
> +    return ret;
> +}
> +
> +int
> +virHostCPUGetInfoPopulateLinux(FILE *cpuinfo,
> +                               virArch arch,
> +                               unsigned int *cpus,
> +                               unsigned int *mhz,
> +                               unsigned int *nodes,
> +                               unsigned int *sockets,
> +                               unsigned int *cores,
> +                               unsigned int *threads)
> +{
> +    virBitmapPtr present_cpus_map = NULL;
> +    virBitmapPtr online_cpus_map = NULL;
> +    DIR *nodedir = NULL;
> +    struct dirent *nodedirent = NULL;
> +    int nodecpus, nodecores, nodesockets, nodethreads, offline = 0;
> +    int threads_per_subcore = 0;
> +    unsigned int node;
> +    int ret = -1;
> +    char *sysfs_nodedir = NULL;
> +    char *sysfs_cpudir = NULL;
> +    int direrr;
> +
> +    *cpus = *nodes = *sockets = *cores = *threads = 0;
> +
> +    /* Start with parsing CPU clock speed from /proc/cpuinfo */
> +    if (virHostCPUGetInfoParseCPUInfo(cpuinfo, arch, mhz) < 0)
> +        goto cleanup;

Why do we cleanup here and abandon the rest of the information? Since
the information in /proc/cpuinfo is kind of volatile in its format,
shouldn't we be liberal in what we accept? If we can't parse it, we just
report mhz = 0, but gathering the rest of the information in this
function is still valuable.

> +
>      /* Get information about what CPUs are present in the host and what
>       * CPUs are online, so that we don't have to so for each node */
>      present_cpus_map = virHostCPUGetPresentBitmap();
> -- 
> 2.14.3
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

-- 
IBM Systems
Linux on z Systems & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bwalk at de.ibm.com
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 906 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20171213/207f4486/attachment-0001.sig>