Re: [PATCH] qemu-options.hx: Update descriptions of memory options for NUMA node

[Yohei Kojima]

Thank you for the review. I will reflect them in the next version.

On 2023/04/11 21:57, Alex Bennée wrote:
> 
> Yohei Kojima <y-koj@outlook.jp> writes:
> 
>> This commit adds the following description:
>> 1. `memdev` option is recommended over `mem` option (see [1,2])
>> 2. users must specify memory for all NUMA nodes (see [2])
>>
>> This commit also separates descriptions for `mem` and `memdev` into two
>> paragraphs. The old doc describes legacy `mem` option first, and it was
>> a bit confusing.
>>
>> Related documantations:
>> [1] https://wiki.qemu.org/ChangeLog/5.1#Incompatible_changes
>> [2] https://www.qemu.org/docs/master/about/removed-features.html
>>
>> Signed-off-by: Yohei Kojima <y-koj@outlook.jp>
>> ---
>>  qemu-options.hx | 25 ++++++++++++++++---------
>>  1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 59bdf67a2c..174f0d0c2d 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -405,15 +405,22 @@ SRST
>>          -numa node,nodeid=0 -numa node,nodeid=1 \
>>          -numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=1,socket-id=1
>>  
>> -    Legacy '\ ``mem``\ ' assigns a given RAM amount to a node (not supported
>> -    for 5.1 and newer machine types). '\ ``memdev``\ ' assigns RAM from
>> -    a given memory backend device to a node. If '\ ``mem``\ ' and
>> -    '\ ``memdev``\ ' are omitted in all nodes, RAM is split equally between 
>> them.
>> -
>> -
>> -    '\ ``mem``\ ' and '\ ``memdev``\ ' are mutually exclusive.
>> -    Furthermore, if one node uses '\ ``memdev``\ ', all of them have to
>> -    use it.
>> +    '\ ``memdev``\ ' option assigns RAM from a given memory backend
>> +    device to a node. It is recommended to use '\ ``memdev``\ ' option
>> +    over legacy '\ ``mem``\ ' option. This is because '\ ``memdev``\ '
>> +    option provides better performance and more control over the
>> +    backend's RAM (e.g. '\ ``prealloc``\ ' parameter of
>> +    '\ ``-memory-backend-ram``\ ' allows memory preallocation).
>> +
>> +    For compatibility reasons, legacy '\ ``mem``\ ' option is
>> +    supported in 5.0 and older machine types. Note that '\ ``mem``\ '
>> +    and '\ ``memdev``\ ' are mutually exclusive. If one node uses
>> +    '\ ``memdev``\ ', the rest nodes have to use '\ ``memdev``\ '
>> +    option, and vice versa.
>> +
>> +    Users must specify memory for all NUMA nodes by '\ ``memdev``\ '
>> +    (or legacy '\ ``mem``\ ' if available). In QEMU 5.2, the support
>> +    for '\ ``-numa node``\ ' without memory specified was removed.
> 
> I think this mixes up memdev and mem too much. It would be better to
> make the lead up to the example just talk about memdev (as it is the
> preferred option) and move the discussion about backwards compatibility
> to after the example. You can use the .. note:: annotation to put it in
> a nice little box, something like:
> 
> .. note::
> 
>     For compatibility reasons, legacy '\ ``mem``\ ' option is
>     supported in 5.0 and older machine types. Note that '\ ``mem``\ '
>     and '\ ``memdev``\ ' are mutually exclusive. If one node uses '\
>     ``memdev``\ ', the rest of the nodes must also use the '\
>     ``memdev``\ ' option, and vice versa.
> 
> 
>>  
>>      '\ ``initiator``\ ' is an additional option that points to an
>>      initiator NUMA node that has best performance (the lowest latency or
> 
>