Andrea Bolognani <abologna@xxxxxxxxxx> [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@xxxxxxxxxx > 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@xxxxxxxxxx ------------------------------------------------------------------------ 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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
