Re: [libvirt] [PATCH 04/10] qemu: command: Always execute memory device formatter

On 10/16/2015 08:11 AM, Peter Krempa wrote: > Since we already make sure before that the domain configuration is > valid we may execute it always at the cost of doing 0 iterations of the > for loop. > > This patch will simplify later refactor as it will avoid whitespace > changes. > --- > src/qemu/qemu_command.c | 47 +++++++++++++++++++++++------------------------ > 1 file changed, 23 insertions(+), 24 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index a37a4fa..43e81c7 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -9484,38 +9484,37 @@ qemuBuildCommandLine(virConnectPtr conn, > } > } > > - if (virDomainNumaGetNodeCount(def->numa)) { > - if (qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) > + if (virDomainNumaGetNodeCount(def->numa) && > + qemuBuildNumaArgStr(cfg, def, cmd, qemuCaps, nodeset) < 0) > goto error; > > - /* memory hotplug requires NUMA to be enabled - we already checked > - * that memory devices are present only when NUMA is */ > + /* memory hotplug requires NUMA to be enabled - we already checked > + * that memory devices are present only when NUMA is */ The second half of this comment made me wonder where... especially with the removal of the virDomainNumaGetNodeCount call which would essentially be checking if numa was enabled (I think). The virDomainDefHasMemoryHotplug checks memory_slots or max_memory The virDomainDefPostParseMemory could have it, but it's not overt Not that the check is wrong, but it would be nice to fixup the comment to indicate that 'xxx' checked that memory devices are present only when NUMA is. That at least helps ensure any future refactor doesn't impact the assumption made here.