diff mbox series

[v2] hw/machine: Remove the Zero check of nb_numa_nodes for numa_complete_configuration()

Message ID 20180704132239.6506-1-douly.fnst@cn.fujitsu.com
State New
Headers show
Series [v2] hw/machine: Remove the Zero check of nb_numa_nodes for numa_complete_configuration() | expand

Commit Message

Dou Liyang July 4, 2018, 1:22 p.m. UTC 
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(-)

Comments

no-reply@patchew.org July 4, 2018, 1:31 p.m. UTC | #1 
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
Eduardo Habkost July 4, 2018, 6:06 p.m. UTC | #2 
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.
Igor Mammedov July 10, 2018, 7:42 a.m. UTC | #3 
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.
Dou Liyang July 10, 2018, 8:15 a.m. UTC | #4 
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.
> 
> 
> 
>
Eduardo Habkost July 10, 2018, 10:27 a.m. UTC | #5 
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.
Dou Liyang July 10, 2018, 10:33 a.m. UTC | #6 
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 mbox series

Patch

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.