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

> -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.

> + /* 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.