On Fri, Dec 15, 2017 at 14:11:41 +0100, Andrea Bolognani wrote: > On Fri, 2017-12-15 at 13:18 +0100, Peter Krempa wrote: > > On Thu, Dec 14, 2017 at 17:29:50 +0100, Andrea Bolognani wrote: > > > While at the moment we're only performing a single check that is > > > connected to vCPU hotplugging, we're going to introduce a second > > > one soon. Move the topology check underneath the capability check > > > to make that easier; as a bonus, doing so allows us to reduce the > > > scope of the 'topologycpus' variable. > > > > You know that generally we prefer variables declared at the top scope? > > Is that so? I've seen several instances of the opposite, and it > makes to me not to pollute the function scope with one-use > variables when there are usually enough variables that actually > need to be accessed throughout the function. But I can leave the > declaration where it is if you like it better that way. Fair enough. We only state that we should declare it at the beginning of a scope, so while generally most are declared at function scoep, this does not violate the contributor guidelines. > > > Also you change the version of qemu in the comment without explanation > > here. Rather than bragging how you reduced scope, please document that > > change. > > Right, I meant to include the explanation but forgot :) > > It's very simple, anyway: the version number was wrong, since > QEMU introduced query-hotpluggable-cpus in 2.7 rather than 2.5. While this is true for the presence of query-hotpluggable-cpus. The paragraph is specifically saying that QEMU rejects such configurations starting from 2.5., but the code checks them only since query-hotpluggable-cpus (which was introduced in 2.7). Your modification is thus incorrect. You can add statement that query-hotpluggable-cpus was added in 2.7. but mangling it as you did is not correct in my opinion.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
