Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
Thanks all,
I will send v4 patch to fix the 80 characters limitation issue.
On Sat, Nov 12, 2022 at 6:05 AM Gavin Shan wrote:
>
> On 11/11/22 6:54 PM, Igor Mammedov wrote:
> > On Fri, 11 Nov 2022 17:34:04 +0800
> > Gavin Shan wrote:
> >> On 11/11/22 5:13 PM, Igor Mammedov wrote:
> >>> On Fri, 11 Nov 2022 07:47:16 +0100
> >>> Markus Armbruster wrote:
> Gavin Shan writes:
> > On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
> >> Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
> >> (v5.0.0) changed the default number of threads from number of CPUs
> >> to 1. This was deemed a regression, and fixed in commit f8d426a685
> >> "hostmem: default the amount of prealloc-threads to smp-cpus".
> >> Except the documentation remained unchanged. Update it now.
> >> Signed-off-by: Zhenyu Zhang
> >> ---
> >> v3: Covers historical descriptions (Markus)
> >> v2: The property is changed to smp-cpus since 5.0 (Phild)
> >> ---
> >> qapi/qom.json | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >
> > With the following comments addressed:
> >
> > Reviewed-by: Gavin Shan
> >
> > ---
> >
> > Please consider amending the commit log to something like below.
> >
> > The default "prealloc-threads" value is set to 1 when the property is
> > added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
> > property") in v5.0.0. The default value is conflicting with the sugar
> > property as the value provided by the sugar property is number of CPUs.
>
> What is the sugar property? Can you explain the conflict in a bit more
> detail?
> >>>
> >>> my guess is that Gavin means mem_prealloc compat glue in
> >>> qemu_process_sugar_options()
> >>>
> >>> property value should be set according to following order
> >>>default -> compat -> explicit value
> >>> so I don't see any conflict here.
> >>>
> >>> PS:
> >>> if it we up to me, default would have stayed 1,
> >>> and prealloc-threads fixup to vCPUs number would happen in vl.c
> >>> similar to what is done in qemu_process_sugar_options(),
> >>> keeping backend clean of external dependencies.
> >>>
> >>
> >> Yes, it's the sugar property I was talking about. I'm not sure if
> >> we have a more popular name for this property: compat property or
> >> sugar property.
> >>
> >> When 'mem-prealloc=on' and 'prealloc-threads=xxx' aren't provided,
> >> the value is 1 before commit f8d426a6852c is applied. It's not
> >> inconsistent with 'mem-prealloc=on'. It's the conflict I was talking
> >> about and it's fixed by commit f8d426a6852c
> >
> > default was not supposed to be consistent with legacy mem-prealloc
> > and sugar property takes care of mem-prealloc=on case.
> >
> > so commit message in its current form looks fine to me.
> >
>
> Ok, thanks for your confirm. I think Zhenyu needs to post v4, to fix
> the 80 characters limitation issue. My reviewed-by is still valid.
>
>
> > The conflict has been fixed by commit f8d426a6852c ("hostmem: default
> > the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
> > was missed to be updated accordingly in the commit.
> >
> > Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
> >
> > Signed-off-by: Zhenyu Zhang
> >
> > When a specific commit is mentioned in the commit log, we usually have
> > fixed format like below.
> >
> > commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
> > commit f8d426a6852c ("hostmem: default the amount of prealloc-threads
> > to smp-cpus")
>
> This is certainly a common format, but the other one is also in use.
>
> >> diff --git a/qapi/qom.json b/qapi/qom.json
> >> index 30e76653ad..dfd89bc6d4 100644
> >> --- a/qapi/qom.json
> >> +++ b/qapi/qom.json
> >> @@ -576,7 +576,7 @@
> >> #
> >> # @prealloc: if true, preallocate memory (default: false)
> >> #
> >> -# @prealloc-threads: number of CPU threads to use for prealloc
> >> (default: 1)
> >> +# @prealloc-threads: number of CPU threads to use for prealloc
> >> (default: number of CPUs) (since 5.0)
> >> #
> >> # @prealloc-context: thread context to use for creation of
> >> preallocation threads
> >> #(default: none) (since 7.2)
> >>
> >
> > The line seems exceeding 80 characters. It'd better to limit each line
> > in 75 characters.
> > So you probably need:
> >
> > # @prealloc-threads: number of CPU threads to use for prealloc
> > (default: number of CPUs)
> > #(since 5.0)
>
> Still exceeds :)
>
> I suggested
>
> # @prealloc-threads: number of CPU threads to use for prealloc
> #(default: number of CPUs) (si
Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
On 11/11/22 6:54 PM, Igor Mammedov wrote:
On Fri, 11 Nov 2022 17:34:04 +0800
Gavin Shan wrote:
On 11/11/22 5:13 PM, Igor Mammedov wrote:
On Fri, 11 Nov 2022 07:47:16 +0100
Markus Armbruster wrote:
Gavin Shan writes:
On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
(v5.0.0) changed the default number of threads from number of CPUs
to 1. This was deemed a regression, and fixed in commit f8d426a685
"hostmem: default the amount of prealloc-threads to smp-cpus".
Except the documentation remained unchanged. Update it now.
Signed-off-by: Zhenyu Zhang
---
v3: Covers historical descriptions (Markus)
v2: The property is changed to smp-cpus since 5.0 (Phild)
---
qapi/qom.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
With the following comments addressed:
Reviewed-by: Gavin Shan
---
Please consider amending the commit log to something like below.
The default "prealloc-threads" value is set to 1 when the property is
added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
property") in v5.0.0. The default value is conflicting with the sugar
property as the value provided by the sugar property is number of CPUs.
What is the sugar property? Can you explain the conflict in a bit more
detail?
my guess is that Gavin means mem_prealloc compat glue in
qemu_process_sugar_options()
property value should be set according to following order
default -> compat -> explicit value
so I don't see any conflict here.
PS:
if it we up to me, default would have stayed 1,
and prealloc-threads fixup to vCPUs number would happen in vl.c
similar to what is done in qemu_process_sugar_options(),
keeping backend clean of external dependencies.
Yes, it's the sugar property I was talking about. I'm not sure if
we have a more popular name for this property: compat property or
sugar property.
When 'mem-prealloc=on' and 'prealloc-threads=xxx' aren't provided,
the value is 1 before commit f8d426a6852c is applied. It's not
inconsistent with 'mem-prealloc=on'. It's the conflict I was talking
about and it's fixed by commit f8d426a6852c
default was not supposed to be consistent with legacy mem-prealloc
and sugar property takes care of mem-prealloc=on case.
so commit message in its current form looks fine to me.
Ok, thanks for your confirm. I think Zhenyu needs to post v4, to fix
the 80 characters limitation issue. My reviewed-by is still valid.
The conflict has been fixed by commit f8d426a6852c ("hostmem: default
the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
was missed to be updated accordingly in the commit.
Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
Signed-off-by: Zhenyu Zhang
When a specific commit is mentioned in the commit log, we usually have
fixed format like below.
commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
commit f8d426a6852c ("hostmem: default the amount of prealloc-threads to
smp-cpus")
This is certainly a common format, but the other one is also in use.
diff --git a/qapi/qom.json b/qapi/qom.json
index 30e76653ad..dfd89bc6d4 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -576,7 +576,7 @@
#
# @prealloc: if true, preallocate memory (default: false)
#
-# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
+# @prealloc-threads: number of CPU threads to use for prealloc (default:
number of CPUs) (since 5.0)
#
# @prealloc-context: thread context to use for creation of preallocation
threads
#(default: none) (since 7.2)
The line seems exceeding 80 characters. It'd better to limit each line in 75
characters.
So you probably need:
# @prealloc-threads: number of CPU threads to use for prealloc (default:
number of CPUs)
#(since 5.0)
Still exceeds :)
I suggested
# @prealloc-threads: number of CPU threads to use for prealloc
#(default: number of CPUs) (since 5.0)
Markus's suggestion works :)
Thanks,
Gavin
Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
On Fri, 11 Nov 2022 17:34:04 +0800
Gavin Shan wrote:
> On 11/11/22 5:13 PM, Igor Mammedov wrote:
> > On Fri, 11 Nov 2022 07:47:16 +0100
> > Markus Armbruster wrote:
> >> Gavin Shan writes:
> >>> On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
> Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
> (v5.0.0) changed the default number of threads from number of CPUs
> to 1. This was deemed a regression, and fixed in commit f8d426a685
> "hostmem: default the amount of prealloc-threads to smp-cpus".
> Except the documentation remained unchanged. Update it now.
> Signed-off-by: Zhenyu Zhang
> ---
> v3: Covers historical descriptions (Markus)
> v2: The property is changed to smp-cpus since 5.0 (Phild)
> ---
> qapi/qom.json | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> >>>
> >>> With the following comments addressed:
> >>>
> >>> Reviewed-by: Gavin Shan
> >>>
> >>> ---
> >>>
> >>> Please consider amending the commit log to something like below.
> >>>
> >>> The default "prealloc-threads" value is set to 1 when the property is
> >>> added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
> >>> property") in v5.0.0. The default value is conflicting with the sugar
> >>> property as the value provided by the sugar property is number of CPUs.
> >>
> >> What is the sugar property? Can you explain the conflict in a bit more
> >> detail?
> >
> > my guess is that Gavin means mem_prealloc compat glue in
> > qemu_process_sugar_options()
> >
> > property value should be set according to following order
> > default -> compat -> explicit value
> > so I don't see any conflict here.
> >
> > PS:
> > if it we up to me, default would have stayed 1,
> > and prealloc-threads fixup to vCPUs number would happen in vl.c
> > similar to what is done in qemu_process_sugar_options(),
> > keeping backend clean of external dependencies.
> >
>
> Yes, it's the sugar property I was talking about. I'm not sure if
> we have a more popular name for this property: compat property or
> sugar property.
>
> When 'mem-prealloc=on' and 'prealloc-threads=xxx' aren't provided,
> the value is 1 before commit f8d426a6852c is applied. It's not
> inconsistent with 'mem-prealloc=on'. It's the conflict I was talking
> about and it's fixed by commit f8d426a6852c
default was not supposed to be consistent with legacy mem-prealloc
and sugar property takes care of mem-prealloc=on case.
so commit message in its current form looks fine to me.
> >>
> >>> The conflict has been fixed by commit f8d426a6852c ("hostmem: default
> >>> the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
> >>> was missed to be updated accordingly in the commit.
> >>>
> >>> Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
> >>>
> >>> Signed-off-by: Zhenyu Zhang
> >>>
> >>> When a specific commit is mentioned in the commit log, we usually have
> >>> fixed format like below.
> >>>
> >>> commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
> >>> commit f8d426a6852c ("hostmem: default the amount of prealloc-threads to
> >>> smp-cpus")
> >>
> >> This is certainly a common format, but the other one is also in use.
> >>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 30e76653ad..dfd89bc6d4 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -576,7 +576,7 @@
> #
> # @prealloc: if true, preallocate memory (default: false)
> #
> -# @prealloc-threads: number of CPU threads to use for prealloc
> (default: 1)
> +# @prealloc-threads: number of CPU threads to use for prealloc
> (default: number of CPUs) (since 5.0)
> #
> # @prealloc-context: thread context to use for creation of
> preallocation threads
> #(default: none) (since 7.2)
>
> >>>
> >>> The line seems exceeding 80 characters. It'd better to limit each line in
> >>> 75 characters.
> >>> So you probably need:
> >>>
> >>> # @prealloc-threads: number of CPU threads to use for prealloc
> >>> (default: number of CPUs)
> >>> #(since 5.0)
> >>
> >> Still exceeds :)
> >>
> >> I suggested
> >>
> >># @prealloc-threads: number of CPU threads to use for prealloc
> >>#(default: number of CPUs) (since 5.0)
> >>
> >
>
> Markus's suggestion works :)
>
> Thanks,
> Gavin
>
>
Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
On 11/11/22 5:13 PM, Igor Mammedov wrote:
On Fri, 11 Nov 2022 07:47:16 +0100
Markus Armbruster wrote:
Gavin Shan writes:
On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
(v5.0.0) changed the default number of threads from number of CPUs
to 1. This was deemed a regression, and fixed in commit f8d426a685
"hostmem: default the amount of prealloc-threads to smp-cpus".
Except the documentation remained unchanged. Update it now.
Signed-off-by: Zhenyu Zhang
---
v3: Covers historical descriptions (Markus)
v2: The property is changed to smp-cpus since 5.0 (Phild)
---
qapi/qom.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
With the following comments addressed:
Reviewed-by: Gavin Shan
---
Please consider amending the commit log to something like below.
The default "prealloc-threads" value is set to 1 when the property is
added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
property") in v5.0.0. The default value is conflicting with the sugar
property as the value provided by the sugar property is number of CPUs.
What is the sugar property? Can you explain the conflict in a bit more
detail?
my guess is that Gavin means mem_prealloc compat glue in
qemu_process_sugar_options()
property value should be set according to following order
default -> compat -> explicit value
so I don't see any conflict here.
PS:
if it we up to me, default would have stayed 1,
and prealloc-threads fixup to vCPUs number would happen in vl.c
similar to what is done in qemu_process_sugar_options(),
keeping backend clean of external dependencies.
Yes, it's the sugar property I was talking about. I'm not sure if
we have a more popular name for this property: compat property or
sugar property.
When 'mem-prealloc=on' and 'prealloc-threads=xxx' aren't provided,
the value is 1 before commit f8d426a6852c is applied. It's not
inconsistent with 'mem-prealloc=on'. It's the conflict I was talking
about and it's fixed by commit f8d426a6852c
The conflict has been fixed by commit f8d426a6852c ("hostmem: default
the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
was missed to be updated accordingly in the commit.
Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
Signed-off-by: Zhenyu Zhang
When a specific commit is mentioned in the commit log, we usually have
fixed format like below.
commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
commit f8d426a6852c ("hostmem: default the amount of prealloc-threads to
smp-cpus")
This is certainly a common format, but the other one is also in use.
diff --git a/qapi/qom.json b/qapi/qom.json
index 30e76653ad..dfd89bc6d4 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -576,7 +576,7 @@
#
# @prealloc: if true, preallocate memory (default: false)
#
-# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
+# @prealloc-threads: number of CPU threads to use for prealloc (default:
number of CPUs) (since 5.0)
#
# @prealloc-context: thread context to use for creation of preallocation
threads
#(default: none) (since 7.2)
The line seems exceeding 80 characters. It'd better to limit each line in 75
characters.
So you probably need:
# @prealloc-threads: number of CPU threads to use for prealloc (default:
number of CPUs)
#(since 5.0)
Still exceeds :)
I suggested
# @prealloc-threads: number of CPU threads to use for prealloc
#(default: number of CPUs) (since 5.0)
Markus's suggestion works :)
Thanks,
Gavin
Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
On Fri, 11 Nov 2022 07:47:16 +0100
Markus Armbruster wrote:
> Gavin Shan writes:
>
> > Hi Zhenyu,
> >
> > On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
> >> Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
> >> (v5.0.0) changed the default number of threads from number of CPUs
> >> to 1. This was deemed a regression, and fixed in commit f8d426a685
> >> "hostmem: default the amount of prealloc-threads to smp-cpus".
> >> Except the documentation remained unchanged. Update it now.
> >> Signed-off-by: Zhenyu Zhang
> >> ---
> >> v3: Covers historical descriptions (Markus)
> >> v2: The property is changed to smp-cpus since 5.0 (Phild)
> >> ---
> >> qapi/qom.json | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >
> > With the following comments addressed:
> >
> > Reviewed-by: Gavin Shan
> >
> > ---
> >
> > Please consider amending the commit log to something like below.
> >
> > The default "prealloc-threads" value is set to 1 when the property is
> > added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
> > property") in v5.0.0. The default value is conflicting with the sugar
> > property as the value provided by the sugar property is number of CPUs.
>
> What is the sugar property? Can you explain the conflict in a bit more
> detail?
my guess is that Gavin means mem_prealloc compat glue in
qemu_process_sugar_options()
property value should be set according to following order
default -> compat -> explicit value
so I don't see any conflict here.
PS:
if it we up to me, default would have stayed 1,
and prealloc-threads fixup to vCPUs number would happen in vl.c
similar to what is done in qemu_process_sugar_options(),
keeping backend clean of external dependencies.
>
> > The conflict has been fixed by commit f8d426a6852c ("hostmem: default
> > the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
> > was missed to be updated accordingly in the commit.
> >
> > Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
> >
> > Signed-off-by: Zhenyu Zhang
> >
> > When a specific commit is mentioned in the commit log, we usually have
> > fixed format like below.
> >
> > commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
> > commit f8d426a6852c ("hostmem: default the amount of prealloc-threads to
> > smp-cpus")
>
> This is certainly a common format, but the other one is also in use.
>
> >> diff --git a/qapi/qom.json b/qapi/qom.json
> >> index 30e76653ad..dfd89bc6d4 100644
> >> --- a/qapi/qom.json
> >> +++ b/qapi/qom.json
> >> @@ -576,7 +576,7 @@
> >> #
> >> # @prealloc: if true, preallocate memory (default: false)
> >> #
> >> -# @prealloc-threads: number of CPU threads to use for prealloc (default:
> >> 1)
> >> +# @prealloc-threads: number of CPU threads to use for prealloc (default:
> >> number of CPUs) (since 5.0)
> >> #
> >> # @prealloc-context: thread context to use for creation of preallocation
> >> threads
> >> #(default: none) (since 7.2)
> >>
> >
> > The line seems exceeding 80 characters. It'd better to limit each line in
> > 75 characters.
> > So you probably need:
> >
> > # @prealloc-threads: number of CPU threads to use for prealloc
> > (default: number of CPUs)
> > #(since 5.0)
>
> Still exceeds :)
>
> I suggested
>
> # @prealloc-threads: number of CPU threads to use for prealloc
> #(default: number of CPUs) (since 5.0)
>
Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
Gavin Shan writes:
> Hi Zhenyu,
>
> On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
>> Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
>> (v5.0.0) changed the default number of threads from number of CPUs
>> to 1. This was deemed a regression, and fixed in commit f8d426a685
>> "hostmem: default the amount of prealloc-threads to smp-cpus".
>> Except the documentation remained unchanged. Update it now.
>> Signed-off-by: Zhenyu Zhang
>> ---
>> v3: Covers historical descriptions (Markus)
>> v2: The property is changed to smp-cpus since 5.0 (Phild)
>> ---
>> qapi/qom.json | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>
> With the following comments addressed:
>
> Reviewed-by: Gavin Shan
>
> ---
>
> Please consider amending the commit log to something like below.
>
> The default "prealloc-threads" value is set to 1 when the property is
> added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
> property") in v5.0.0. The default value is conflicting with the sugar
> property as the value provided by the sugar property is number of CPUs.
What is the sugar property? Can you explain the conflict in a bit more
detail?
> The conflict has been fixed by commit f8d426a6852c ("hostmem: default
> the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
> was missed to be updated accordingly in the commit.
>
> Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
>
> Signed-off-by: Zhenyu Zhang
>
> When a specific commit is mentioned in the commit log, we usually have
> fixed format like below.
>
> commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
> commit f8d426a6852c ("hostmem: default the amount of prealloc-threads to
> smp-cpus")
This is certainly a common format, but the other one is also in use.
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index 30e76653ad..dfd89bc6d4 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -576,7 +576,7 @@
>> #
>> # @prealloc: if true, preallocate memory (default: false)
>> #
>> -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
>> +# @prealloc-threads: number of CPU threads to use for prealloc (default:
>> number of CPUs) (since 5.0)
>> #
>> # @prealloc-context: thread context to use for creation of preallocation
>> threads
>> #(default: none) (since 7.2)
>>
>
> The line seems exceeding 80 characters. It'd better to limit each line in 75
> characters.
> So you probably need:
>
> # @prealloc-threads: number of CPU threads to use for prealloc (default:
> number of CPUs)
> #(since 5.0)
Still exceeds :)
I suggested
# @prealloc-threads: number of CPU threads to use for prealloc
#(default: number of CPUs) (since 5.0)
Re: [PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
Hi Zhenyu,
On 11/11/22 11:05 AM, Zhenyu Zhang wrote:
Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property"
(v5.0.0) changed the default number of threads from number of CPUs
to 1. This was deemed a regression, and fixed in commit f8d426a685
"hostmem: default the amount of prealloc-threads to smp-cpus".
Except the documentation remained unchanged. Update it now.
Signed-off-by: Zhenyu Zhang
---
v3: Covers historical descriptions (Markus)
v2: The property is changed to smp-cpus since 5.0 (Phild)
---
qapi/qom.json | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
With the following comments addressed:
Reviewed-by: Gavin Shan
---
Please consider amending the commit log to something like below.
The default "prealloc-threads" value is set to 1 when the property is
added by commit ffac16fab33b ("hostmem: introduce "prealloc-threads"
property") in v5.0.0. The default value is conflicting with the sugar
property as the value provided by the sugar property is number of CPUs.
The conflict has been fixed by commit f8d426a6852c ("hostmem: default
the amount of prealloc-threads to smp-cpus"). However, 'qapi/qom.json'
was missed to be updated accordingly in the commit.
Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c.
Signed-off-by: Zhenyu Zhang
When a specific commit is mentioned in the commit log, we usually have
fixed format like below.
commit ffac16fab33b ("hostmem: introduce "prealloc-threads" property")
commit f8d426a6852c ("hostmem: default the amount of prealloc-threads to
smp-cpus")
diff --git a/qapi/qom.json b/qapi/qom.json
index 30e76653ad..dfd89bc6d4 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -576,7 +576,7 @@
#
# @prealloc: if true, preallocate memory (default: false)
#
-# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
+# @prealloc-threads: number of CPU threads to use for prealloc (default:
number of CPUs) (since 5.0)
#
# @prealloc-context: thread context to use for creation of preallocation
threads
#(default: none) (since 7.2)
The line seems exceeding 80 characters. It'd better to limit each line in 75
characters.
So you probably need:
# @prealloc-threads: number of CPU threads to use for prealloc (default:
number of CPUs)
#(since 5.0)
Thanks,
Gavin
[PATCH v3] qapi/qom: Memory backend property prealloc-threads doc fix
Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property" (v5.0.0) changed the default number of threads from number of CPUs to 1. This was deemed a regression, and fixed in commit f8d426a685 "hostmem: default the amount of prealloc-threads to smp-cpus". Except the documentation remained unchanged. Update it now. Signed-off-by: Zhenyu Zhang --- v3: Covers historical descriptions (Markus) v2: The property is changed to smp-cpus since 5.0 (Phild) --- qapi/qom.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/qom.json b/qapi/qom.json index 30e76653ad..dfd89bc6d4 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -576,7 +576,7 @@ # # @prealloc: if true, preallocate memory (default: false) # -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1) +# @prealloc-threads: number of CPU threads to use for prealloc (default: number of CPUs) (since 5.0) # # @prealloc-context: thread context to use for creation of preallocation threads #(default: none) (since 7.2) -- 2.31.1
