Message ID | 20180704132239.6506-1-douly.fnst@cn.fujitsu.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20180704132239.6506-1-douly.fnst@cn.fujitsu.com Subject: [Qemu-devel] [PATCH v2] hw/machine: Remove the Zero check of nb_numa_nodes for numa_complete_configuration() === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1530709942-87947-1-git-send-email-jingqi.liu@intel.com -> patchew/1530709942-87947-1-git-send-email-jingqi.liu@intel.com * [new tag] patchew/20180704132239.6506-1-douly.fnst@cn.fujitsu.com -> patchew/20180704132239.6506-1-douly.fnst@cn.fujitsu.com Switched to a new branch 'test' 371fe7e8f1 hw/machine: Remove the Zero check of nb_numa_nodes for numa_complete_configuration() === OUTPUT BEGIN === Checking PATCH 1/1: hw/machine: Remove the Zero check of nb_numa_nodes for numa_complete_configuration()... ERROR: braces {} are necessary for all arms of this statement #31: FILE: hw/core/machine.c:795: + if (nb_numa_nodes) [...] total: 1 errors, 0 warnings, 12 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On Wed, Jul 04, 2018 at 09:22:39PM +0800, Dou Liyang wrote: > Commit 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") > broke the commit 7b8be49d36fc("NUMA: Enable adding NUMA node implicitly"). > > The machine_run_board_init() doesn't do NUMA setup if nb_numa_nodes=0, > but the numa_complete_configuration need add a new node if memory hotplug > is enabled (slots > 0) even nb_numa_nodes=0. > > So, Remove the check for numa_complete_configuration() to fix this. > > Fixes 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Thanks! Queued on machine-next.
On Wed, 4 Jul 2018 21:22:39 +0800 Dou Liyang <douly.fnst@cn.fujitsu.com> wrote: > Commit 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") > broke the commit 7b8be49d36fc("NUMA: Enable adding NUMA node implicitly"). > > The machine_run_board_init() doesn't do NUMA setup if nb_numa_nodes=0, > but the numa_complete_configuration need add a new node if memory hotplug > is enabled (slots > 0) even nb_numa_nodes=0. > > So, Remove the check for numa_complete_configuration() to fix this. > > Fixes 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") > Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> > --- > v1 --> v2: > -Fix it to pass the "make check". > --- > hw/core/machine.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 2077328bcc..32b3c5a7d3 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -791,10 +791,9 @@ void machine_run_board_init(MachineState *machine) > { > MachineClass *machine_class = MACHINE_GET_CLASS(machine); > > - if (nb_numa_nodes) { > - numa_complete_configuration(machine); > + numa_complete_configuration(machine); > + if (nb_numa_nodes) > machine_numa_finish_cpu_init(machine); > - } missing curly brackets, should look like: if (nb_numa_nodes) { machine_numa_finish_cpu_init(machine); } with that fixed Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > /* If the machine supports the valid_cpu_types check and the user > * specified a CPU with -cpu check here that the user CPU is supported.
Hi Igor, At 07/10/2018 03:42 PM, Igor Mammedov wrote: > On Wed, 4 Jul 2018 21:22:39 +0800 > Dou Liyang <douly.fnst@cn.fujitsu.com> wrote: > >> Commit 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") >> broke the commit 7b8be49d36fc("NUMA: Enable adding NUMA node implicitly"). >> >> The machine_run_board_init() doesn't do NUMA setup if nb_numa_nodes=0, >> but the numa_complete_configuration need add a new node if memory hotplug >> is enabled (slots > 0) even nb_numa_nodes=0. >> >> So, Remove the check for numa_complete_configuration() to fix this. >> >> Fixes 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") >> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> >> --- >> v1 --> v2: >> -Fix it to pass the "make check". >> --- >> hw/core/machine.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/hw/core/machine.c b/hw/core/machine.c >> index 2077328bcc..32b3c5a7d3 100644 >> --- a/hw/core/machine.c >> +++ b/hw/core/machine.c >> @@ -791,10 +791,9 @@ void machine_run_board_init(MachineState *machine) >> { >> MachineClass *machine_class = MACHINE_GET_CLASS(machine); >> >> - if (nb_numa_nodes) { >> - numa_complete_configuration(machine); >> + numa_complete_configuration(machine); >> + if (nb_numa_nodes) >> machine_numa_finish_cpu_init(machine); >> - } > missing curly brackets, should look like: > if (nb_numa_nodes) { > machine_numa_finish_cpu_init(machine); > } > > with that fixed > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > Thank you, Igor. Due to Eduardo has sent the pull request of this patch, Can I fix this and send v2 patch now? Thanks, dou >> >> /* If the machine supports the valid_cpu_types check and the user >> * specified a CPU with -cpu check here that the user CPU is supported. > > > >
On Tue, Jul 10, 2018 at 04:15:30PM +0800, Dou Liyang wrote: [...] > > > + if (nb_numa_nodes) > > > machine_numa_finish_cpu_init(machine); > > > - } > > missing curly brackets, should look like: > > if (nb_numa_nodes) { > > machine_numa_finish_cpu_init(machine); > > } > > > > with that fixed > > Reviewed-by: Igor Mammedov <imammedo@redhat.com> > > > > Thank you, Igor. > > Due to Eduardo has sent the pull request of this patch, > Can I fix this and send v2 patch now? You can fix this on a follow-up patch and CC qemu-trivial.
Hi Eduardo, At 07/10/2018 06:27 PM, Eduardo Habkost wrote: > On Tue, Jul 10, 2018 at 04:15:30PM +0800, Dou Liyang wrote: > [...] >>>> + if (nb_numa_nodes) >>>> machine_numa_finish_cpu_init(machine); >>>> - } >>> missing curly brackets, should look like: >>> if (nb_numa_nodes) { >>> machine_numa_finish_cpu_init(machine); >>> } >>> >>> with that fixed >>> Reviewed-by: Igor Mammedov <imammedo@redhat.com> >>> >> >> Thank you, Igor. >> >> Due to Eduardo has sent the pull request of this patch, >> Can I fix this and send v2 patch now? > > You can fix this on a follow-up patch and CC qemu-trivial. > Ok, I see. ;-) Thanks, dou
diff --git a/hw/core/machine.c b/hw/core/machine.c index 2077328bcc..32b3c5a7d3 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -791,10 +791,9 @@ void machine_run_board_init(MachineState *machine) { MachineClass *machine_class = MACHINE_GET_CLASS(machine); - if (nb_numa_nodes) { - numa_complete_configuration(machine); + numa_complete_configuration(machine); + if (nb_numa_nodes) machine_numa_finish_cpu_init(machine); - } /* If the machine supports the valid_cpu_types check and the user * specified a CPU with -cpu check here that the user CPU is supported.
Commit 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") broke the commit 7b8be49d36fc("NUMA: Enable adding NUMA node implicitly"). The machine_run_board_init() doesn't do NUMA setup if nb_numa_nodes=0, but the numa_complete_configuration need add a new node if memory hotplug is enabled (slots > 0) even nb_numa_nodes=0. So, Remove the check for numa_complete_configuration() to fix this. Fixes 7a3099fc9c5c("numa: postpone options post-processing till machine_run_board_init()") Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> --- v1 --> v2: -Fix it to pass the "make check". --- hw/core/machine.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)