Re: [libvirt] [PATCH 2/6] util: Introduce virHostCPUGetInfoParseCPUInfo()
Andrea Bolognani <abologna(a)redhat.com> [2017-12-13, 12:34PM +0100]:
So you'd leave it in virHostCPUGetInfoPopulateLinux()? Why?
I found it more important that all the initialization (cpu, nodes, etc.)
is in one place. But it actually doesn't matter.
With my approach, the value is changed only in
virHostCPUGetInfoParseCPUInfo(), which makes IMHO more sense than
spreading the code that changes it across two functions.
> > + /* 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.
Most functions in libvirt either perform all tasks succesfully or
return a failure, so failing here is in line both with that and
with the existing behavior.
So for example for S390 we have introduced CPU frequency information in
/proc/cpuinfo only recently. That means that depending on your kernel
version, you either read freq. info and the rest of the stuff or you
discard the whole CPU. I found this highly unintuitive.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list(a)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(a)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
Attachments:
- signature.asc (application/pgp-signature — 906 bytes)