Re: [libvirt] [PATCH 1/2] qemu: Invert condition nesting in qemuDomainDefValidate()
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.
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.
--
Andrea Bolognani / Red Hat / Virtualization