On 5/20/2024 10:16 PM, Thomas Huth wrote:
> On 21/05/2024 07.08, Thomas Huth wrote:
>> On 20/05/2024 19.47, Zide Chen wrote:
>>> Both cpu-pm and mem-lock are related to system resource overcommit, but
>>> they are separate from each other, in terms of how they are realized,
>>> and of course, they are applied to different system resources.
>>>
>>> It's tempting to use separate command lines to specify their behavior.
>>> e.g., in the following example, the cpu-pm command is quietly
>>> overwritten, and it's not easy to notice it without careful inspection.
>>>
>>> --overcommit mem-lock=on
>>> --overcommit cpu-pm=on
>>>
>>> Fixes: c8c9dc42b7ca ("Remove the deprecated -realtime option")
>>> Signed-off-by: Zide Chen <zide.c...@intel.com>
>>> ---
>>> system/vl.c | 8 ++++++--
>>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/system/vl.c b/system/vl.c
>>> index a3eede5fa5b8..ed682643805b 100644
>>> --- a/system/vl.c
>>> +++ b/system/vl.c
>>> @@ -3545,8 +3545,12 @@ void qemu_init(int argc, char **argv)
>>> if (!opts) {
>>> exit(1);
>>> }
>>> - enable_mlock = qemu_opt_get_bool(opts, "mem-lock",
>>> false);
>>> - enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm",
>>> false);
>>> +
>>> + /* Don't override the -overcommit option if set */
>>> + enable_mlock = enable_mlock ||
>>> + qemu_opt_get_bool(opts, "mem-lock", false);
>>> + enable_cpu_pm = enable_cpu_pm ||
>>> + qemu_opt_get_bool(opts, "cpu-pm", false);
>>> break;
>>> case QEMU_OPTION_compat:
>>> {
>>
>> Reviewed-by: Thomas Huth <th...@redhat.com>
>
> Ah, wait, actually, this is a bad idea, too, since now you cannot
> disable an enabled option anymore. Imagine that you have for example
> enabled the option in the config file, and now you'd like to disable it
> on the command line again - you're stuck with the enabled setting in
> that case.
>
> I think the better solution is to replace the "false" default value at
> the end:
>
> enable_mlock = qemu_opt_get_bool(opts, "mem-lock", enable_mlock);
> enable_cpu_pm = qemu_opt_get_bool(opts, "cpu-pm", enable_cpu_pm);
>
> What do you think about this?
Thank you very much! This is a very good idea! I can update it in V2.
> Thomas
>
