* [PATCH 1/1] numa: check mem or memdev in numa configuration
@ 2022-02-16 16:36 Li Zhang
2022-02-17 9:10 ` Igor Mammedov
0 siblings, 1 reply; 8+ messages in thread
From: Li Zhang @ 2022-02-16 16:36 UTC (permalink / raw)
To: eduardo, marcel.apfelbaum, f4bug, wangyanan55, qemu-devel; +Cc: Li Zhang
If there is no mem or memdev in numa configuration, it always
reports the error as the following:
total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
This error is confusing and the reason is that total memory of numa nodes
is always 0 if there is no mem or memdev in numa configuration.
So it's better to check mem or memdev in numa configuration.
Signed-off-by: Li Zhang <lizhang@suse.de>
---
hw/core/numa.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/core/numa.c b/hw/core/numa.c
index 1aa05dcf42..11cbec51eb 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
have_memdevs = have_memdevs ? : node->has_memdev;
have_mem = have_mem ? : node->has_mem;
+ if (!node->has_memdev && !node->has_mem) {
+ error_setg(errp, "numa configuration should use mem= or memdev= ");
+ return;
+ }
+
if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
error_setg(errp, "numa configuration should use either mem= or memdev=,"
"mixing both is not allowed");
--
2.31.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-16 16:36 [PATCH 1/1] numa: check mem or memdev in numa configuration Li Zhang
@ 2022-02-17 9:10 ` Igor Mammedov
2022-02-17 9:38 ` Li Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2022-02-17 9:10 UTC (permalink / raw)
To: Li Zhang; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On Wed, 16 Feb 2022 17:36:13 +0100
Li Zhang <lizhang@suse.de> wrote:
> If there is no mem or memdev in numa configuration, it always
> reports the error as the following:
>
> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
>
> This error is confusing and the reason is that total memory of numa nodes
> is always 0 if there is no mem or memdev in numa configuration.
> So it's better to check mem or memdev in numa configuration.
>
> Signed-off-by: Li Zhang <lizhang@suse.de>
> ---
> hw/core/numa.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 1aa05dcf42..11cbec51eb 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>
> have_memdevs = have_memdevs ? : node->has_memdev;
> have_mem = have_mem ? : node->has_mem;
> + if (!node->has_memdev && !node->has_mem) {
> + error_setg(errp, "numa configuration should use mem= or memdev= ");
> + return;
> + }
Wouldn't this breaks memory less numa nodes?
I'd rather add/rephrase to original error message that memory
should be specified explicitly for desired numa nodes.
And I'd not mention 'mem=' since
docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
> +
> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
> error_setg(errp, "numa configuration should use either mem= or memdev=,"
> "mixing both is not allowed");
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-17 9:10 ` Igor Mammedov
@ 2022-02-17 9:38 ` Li Zhang
2022-02-17 10:25 ` Igor Mammedov
0 siblings, 1 reply; 8+ messages in thread
From: Li Zhang @ 2022-02-17 9:38 UTC (permalink / raw)
To: Igor Mammedov; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On 2/17/22 10:10 AM, Igor Mammedov wrote:
> On Wed, 16 Feb 2022 17:36:13 +0100
> Li Zhang <lizhang@suse.de> wrote:
>
>> If there is no mem or memdev in numa configuration, it always
>> reports the error as the following:
>>
>> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
>>
>> This error is confusing and the reason is that total memory of numa nodes
>> is always 0 if there is no mem or memdev in numa configuration.
>> So it's better to check mem or memdev in numa configuration.
>>
>> Signed-off-by: Li Zhang <lizhang@suse.de>
>> ---
>> hw/core/numa.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>> index 1aa05dcf42..11cbec51eb 100644
>> --- a/hw/core/numa.c
>> +++ b/hw/core/numa.c
>> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>>
>> have_memdevs = have_memdevs ? : node->has_memdev;
>> have_mem = have_mem ? : node->has_mem;
>> + if (!node->has_memdev && !node->has_mem) {
>> + error_setg(errp, "numa configuration should use mem= or memdev= ");
>> + return;
>> + }
>
> Wouldn't this breaks memory less numa nodes?
Yes, you are right. It will break it if there more numa nodes
than memory, and the numa nodes have no memory backends specified.
Is it allowed for users to specify more numa nodes than memory?
>
> I'd rather add/rephrase to original error message that memory
> should be specified explicitly for desired numa nodes.
> And I'd not mention 'mem=' since
> docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
Thanks for your suggestions, I will rephrase it.
>
>> +
>> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
>> error_setg(errp, "numa configuration should use either mem= or memdev=,"
>> "mixing both is not allowed");
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-17 9:38 ` Li Zhang
@ 2022-02-17 10:25 ` Igor Mammedov
2022-02-17 11:06 ` Li Zhang
2022-02-17 12:24 ` Li Zhang
0 siblings, 2 replies; 8+ messages in thread
From: Igor Mammedov @ 2022-02-17 10:25 UTC (permalink / raw)
To: Li Zhang; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On Thu, 17 Feb 2022 10:38:32 +0100
Li Zhang <lizhang@suse.de> wrote:
> On 2/17/22 10:10 AM, Igor Mammedov wrote:
> > On Wed, 16 Feb 2022 17:36:13 +0100
> > Li Zhang <lizhang@suse.de> wrote:
> >
> >> If there is no mem or memdev in numa configuration, it always
> >> reports the error as the following:
> >>
> >> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
> >>
> >> This error is confusing and the reason is that total memory of numa nodes
> >> is always 0 if there is no mem or memdev in numa configuration.
> >> So it's better to check mem or memdev in numa configuration.
> >>
> >> Signed-off-by: Li Zhang <lizhang@suse.de>
> >> ---
> >> hw/core/numa.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/hw/core/numa.c b/hw/core/numa.c
> >> index 1aa05dcf42..11cbec51eb 100644
> >> --- a/hw/core/numa.c
> >> +++ b/hw/core/numa.c
> >> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> >>
> >> have_memdevs = have_memdevs ? : node->has_memdev;
> >> have_mem = have_mem ? : node->has_mem;
> >> + if (!node->has_memdev && !node->has_mem) {
> >> + error_setg(errp, "numa configuration should use mem= or memdev= ");
> >> + return;
> >> + }
> >
> > Wouldn't this breaks memory less numa nodes?
>
> Yes, you are right. It will break it if there more numa nodes
> than memory, and the numa nodes have no memory backends specified.
>
> Is it allowed for users to specify more numa nodes than memory?
yep, I think we support it at least for one of the targets
(but I don't remember which one(s))
>
> >
> > I'd rather add/rephrase to original error message that memory
> > should be specified explicitly for desired numa nodes.
> > And I'd not mention 'mem=' since
> > docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
>
> Thanks for your suggestions, I will rephrase it.
>
> >
> >> +
> >> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
> >> error_setg(errp, "numa configuration should use either mem= or memdev=,"
> >> "mixing both is not allowed");
> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-17 10:25 ` Igor Mammedov
@ 2022-02-17 11:06 ` Li Zhang
2022-02-17 12:24 ` Li Zhang
1 sibling, 0 replies; 8+ messages in thread
From: Li Zhang @ 2022-02-17 11:06 UTC (permalink / raw)
To: Igor Mammedov; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On 2/17/22 11:25 AM, Igor Mammedov wrote:
> On Thu, 17 Feb 2022 10:38:32 +0100
> Li Zhang <lizhang@suse.de> wrote:
>
>> On 2/17/22 10:10 AM, Igor Mammedov wrote:
>>> On Wed, 16 Feb 2022 17:36:13 +0100
>>> Li Zhang <lizhang@suse.de> wrote:
>>>
>>>> If there is no mem or memdev in numa configuration, it always
>>>> reports the error as the following:
>>>>
>>>> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
>>>>
>>>> This error is confusing and the reason is that total memory of numa nodes
>>>> is always 0 if there is no mem or memdev in numa configuration.
>>>> So it's better to check mem or memdev in numa configuration.
>>>>
>>>> Signed-off-by: Li Zhang <lizhang@suse.de>
>>>> ---
>>>> hw/core/numa.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>>> index 1aa05dcf42..11cbec51eb 100644
>>>> --- a/hw/core/numa.c
>>>> +++ b/hw/core/numa.c
>>>> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>>>>
>>>> have_memdevs = have_memdevs ? : node->has_memdev;
>>>> have_mem = have_mem ? : node->has_mem;
>>>> + if (!node->has_memdev && !node->has_mem) {
>>>> + error_setg(errp, "numa configuration should use mem= or memdev= ");
>>>> + return;
>>>> + }
>>>
>>> Wouldn't this breaks memory less numa nodes?
>>
>> Yes, you are right. It will break it if there more numa nodes
>> than memory, and the numa nodes have no memory backends specified.
>>
>> Is it allowed for users to specify more numa nodes than memory?
> yep, I think we support it at least for one of the targets
> (but I don't remember which one(s))
>
Ah, I see. Thanks.
>>
>>>
>>> I'd rather add/rephrase to original error message that memory
>>> should be specified explicitly for desired numa nodes.
>>> And I'd not mention 'mem=' since
>>> docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
>>
>> Thanks for your suggestions, I will rephrase it.
>>
>>>
>>>> +
>>>> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
>>>> error_setg(errp, "numa configuration should use either mem= or memdev=,"
>>>> "mixing both is not allowed");
>>>
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-17 10:25 ` Igor Mammedov
2022-02-17 11:06 ` Li Zhang
@ 2022-02-17 12:24 ` Li Zhang
2022-02-17 13:33 ` Igor Mammedov
1 sibling, 1 reply; 8+ messages in thread
From: Li Zhang @ 2022-02-17 12:24 UTC (permalink / raw)
To: Igor Mammedov; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On 2/17/22 11:25 AM, Igor Mammedov wrote:
> On Thu, 17 Feb 2022 10:38:32 +0100
> Li Zhang <lizhang@suse.de> wrote:
>
>> On 2/17/22 10:10 AM, Igor Mammedov wrote:
>>> On Wed, 16 Feb 2022 17:36:13 +0100
>>> Li Zhang <lizhang@suse.de> wrote:
>>>
>>>> If there is no mem or memdev in numa configuration, it always
>>>> reports the error as the following:
>>>>
>>>> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
>>>>
>>>> This error is confusing and the reason is that total memory of numa nodes
>>>> is always 0 if there is no mem or memdev in numa configuration.
>>>> So it's better to check mem or memdev in numa configuration.
>>>>
>>>> Signed-off-by: Li Zhang <lizhang@suse.de>
>>>> ---
>>>> hw/core/numa.c | 5 +++++
>>>> 1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>>> index 1aa05dcf42..11cbec51eb 100644
>>>> --- a/hw/core/numa.c
>>>> +++ b/hw/core/numa.c
>>>> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>>>>
>>>> have_memdevs = have_memdevs ? : node->has_memdev;
>>>> have_mem = have_mem ? : node->has_mem;
>>>> + if (!node->has_memdev && !node->has_mem) {
>>>> + error_setg(errp, "numa configuration should use mem= or memdev= ");
>>>> + return;
>>>> + }
>>>
>>> Wouldn't this breaks memory less numa nodes?
>>
>> Yes, you are right. It will break it if there more numa nodes
>> than memory, and the numa nodes have no memory backends specified.
>>
>> Is it allowed for users to specify more numa nodes than memory?
> yep, I think we support it at least for one of the targets
> (but I don't remember which one(s))
>
Is it okay if I put a warning here, instead of an error and return?
It won't break the special case. I wonder if it is annoying to get
the warning.
Thanks
Li
>>
>>>
>>> I'd rather add/rephrase to original error message that memory
>>> should be specified explicitly for desired numa nodes.
>>> And I'd not mention 'mem=' since
>>> docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
>>
>> Thanks for your suggestions, I will rephrase it.
>>
>>>
>>>> +
>>>> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
>>>> error_setg(errp, "numa configuration should use either mem= or memdev=,"
>>>> "mixing both is not allowed");
>>>
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-17 12:24 ` Li Zhang
@ 2022-02-17 13:33 ` Igor Mammedov
2022-02-17 15:22 ` Li Zhang
0 siblings, 1 reply; 8+ messages in thread
From: Igor Mammedov @ 2022-02-17 13:33 UTC (permalink / raw)
To: Li Zhang; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On Thu, 17 Feb 2022 13:24:08 +0100
Li Zhang <lizhang@suse.de> wrote:
> On 2/17/22 11:25 AM, Igor Mammedov wrote:
> > On Thu, 17 Feb 2022 10:38:32 +0100
> > Li Zhang <lizhang@suse.de> wrote:
> >
> >> On 2/17/22 10:10 AM, Igor Mammedov wrote:
> >>> On Wed, 16 Feb 2022 17:36:13 +0100
> >>> Li Zhang <lizhang@suse.de> wrote:
> >>>
> >>>> If there is no mem or memdev in numa configuration, it always
> >>>> reports the error as the following:
> >>>>
> >>>> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
> >>>>
> >>>> This error is confusing and the reason is that total memory of numa nodes
> >>>> is always 0 if there is no mem or memdev in numa configuration.
> >>>> So it's better to check mem or memdev in numa configuration.
> >>>>
> >>>> Signed-off-by: Li Zhang <lizhang@suse.de>
> >>>> ---
> >>>> hw/core/numa.c | 5 +++++
> >>>> 1 file changed, 5 insertions(+)
> >>>>
> >>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
> >>>> index 1aa05dcf42..11cbec51eb 100644
> >>>> --- a/hw/core/numa.c
> >>>> +++ b/hw/core/numa.c
> >>>> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
> >>>>
> >>>> have_memdevs = have_memdevs ? : node->has_memdev;
> >>>> have_mem = have_mem ? : node->has_mem;
> >>>> + if (!node->has_memdev && !node->has_mem) {
> >>>> + error_setg(errp, "numa configuration should use mem= or memdev= ");
> >>>> + return;
> >>>> + }
> >>>
> >>> Wouldn't this breaks memory less numa nodes?
> >>
> >> Yes, you are right. It will break it if there more numa nodes
> >> than memory, and the numa nodes have no memory backends specified.
> >>
> >> Is it allowed for users to specify more numa nodes than memory?
> > yep, I think we support it at least for one of the targets
> > (but I don't remember which one(s))
> >
>
> Is it okay if I put a warning here, instead of an error and return?
> It won't break the special case. I wonder if it is annoying to get
> the warning.
issuing warning in perfectly valid case (memory-less node)
doesn't look like a good thing to do.
there is already a error message,
"total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)"
I'd suggest to just fix this error message to be less confusing
instead of adding dubious warning elsewhere.
>
> Thanks
> Li
>
> >>
> >>>
> >>> I'd rather add/rephrase to original error message that memory
> >>> should be specified explicitly for desired numa nodes.
> >>> And I'd not mention 'mem=' since
> >>> docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
> >>
> >> Thanks for your suggestions, I will rephrase it.
> >>
> >>>
> >>>> +
> >>>> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
> >>>> error_setg(errp, "numa configuration should use either mem= or memdev=,"
> >>>> "mixing both is not allowed");
> >>>
> >>
> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/1] numa: check mem or memdev in numa configuration
2022-02-17 13:33 ` Igor Mammedov
@ 2022-02-17 15:22 ` Li Zhang
0 siblings, 0 replies; 8+ messages in thread
From: Li Zhang @ 2022-02-17 15:22 UTC (permalink / raw)
To: Igor Mammedov; +Cc: eduardo, wangyanan55, f4bug, qemu-devel
On 2/17/22 2:33 PM, Igor Mammedov wrote:
> On Thu, 17 Feb 2022 13:24:08 +0100
> Li Zhang <lizhang@suse.de> wrote:
>
>> On 2/17/22 11:25 AM, Igor Mammedov wrote:
>>> On Thu, 17 Feb 2022 10:38:32 +0100
>>> Li Zhang <lizhang@suse.de> wrote:
>>>
>>>> On 2/17/22 10:10 AM, Igor Mammedov wrote:
>>>>> On Wed, 16 Feb 2022 17:36:13 +0100
>>>>> Li Zhang <lizhang@suse.de> wrote:
>>>>>
>>>>>> If there is no mem or memdev in numa configuration, it always
>>>>>> reports the error as the following:
>>>>>>
>>>>>> total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)
>>>>>>
>>>>>> This error is confusing and the reason is that total memory of numa nodes
>>>>>> is always 0 if there is no mem or memdev in numa configuration.
>>>>>> So it's better to check mem or memdev in numa configuration.
>>>>>>
>>>>>> Signed-off-by: Li Zhang <lizhang@suse.de>
>>>>>> ---
>>>>>> hw/core/numa.c | 5 +++++
>>>>>> 1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>>>>> index 1aa05dcf42..11cbec51eb 100644
>>>>>> --- a/hw/core/numa.c
>>>>>> +++ b/hw/core/numa.c
>>>>>> @@ -132,6 +132,11 @@ static void parse_numa_node(MachineState *ms, NumaNodeOptions *node,
>>>>>>
>>>>>> have_memdevs = have_memdevs ? : node->has_memdev;
>>>>>> have_mem = have_mem ? : node->has_mem;
>>>>>> + if (!node->has_memdev && !node->has_mem) {
>>>>>> + error_setg(errp, "numa configuration should use mem= or memdev= ");
>>>>>> + return;
>>>>>> + }
>>>>>
>>>>> Wouldn't this breaks memory less numa nodes?
>>>>
>>>> Yes, you are right. It will break it if there more numa nodes
>>>> than memory, and the numa nodes have no memory backends specified.
>>>>
>>>> Is it allowed for users to specify more numa nodes than memory?
>>> yep, I think we support it at least for one of the targets
>>> (but I don't remember which one(s))
>>>
>>
>> Is it okay if I put a warning here, instead of an error and return?
>> It won't break the special case. I wonder if it is annoying to get
>> the warning.
> issuing warning in perfectly valid case (memory-less node)
> doesn't look like a good thing to do.
>
> there is already a error message,
>
> "total memory for NUMA nodes (0x0) should equal RAM size (0x100000000)"
>
> I'd suggest to just fix this error message to be less confusing
> instead of adding dubious warning elsewhere.
>
OK, thanks for your suggestion.
>>
>> Thanks
>> Li
>>
>>>>
>>>>>
>>>>> I'd rather add/rephrase to original error message that memory
>>>>> should be specified explicitly for desired numa nodes.
>>>>> And I'd not mention 'mem=' since
>>>>> docs/about/removed-features.rst:``-numa node,mem=...`` (removed in 5.1)
>>>>
>>>> Thanks for your suggestions, I will rephrase it.
>>>>
>>>>>
>>>>>> +
>>>>>> if ((node->has_mem && have_memdevs) || (node->has_memdev && have_mem)) {
>>>>>> error_setg(errp, "numa configuration should use either mem= or memdev=,"
>>>>>> "mixing both is not allowed");
>>>>>
>>>>
>>>
>>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-02-17 15:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-16 16:36 [PATCH 1/1] numa: check mem or memdev in numa configuration Li Zhang
2022-02-17 9:10 ` Igor Mammedov
2022-02-17 9:38 ` Li Zhang
2022-02-17 10:25 ` Igor Mammedov
2022-02-17 11:06 ` Li Zhang
2022-02-17 12:24 ` Li Zhang
2022-02-17 13:33 ` Igor Mammedov
2022-02-17 15:22 ` Li Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).