[v6] accel: Preparatory cleanups for split-accel

[PATCH v6 03/39] system/runstate: Document qemu_add_vm_change_state_handler()

Posted by Philippe Mathieu-Daudé 5 months, 2 weeks ago

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/system/runstate.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/system/runstate.h b/include/system/runstate.h
index fdd5c4a5172..b6e8d6beab7 100644
--- a/include/system/runstate.h
+++ b/include/system/runstate.h
@@ -14,6 +14,16 @@ void runstate_replay_enable(void);
 typedef void VMChangeStateHandler(void *opaque, bool running, RunState state);
 typedef int VMChangeStateHandlerWithRet(void *opaque, bool running, RunState state);
 
+/**
+ * qemu_add_vm_change_state_handler:
+ * @cb: the callback to invoke
+ * @opaque: user data passed to the callback
+ *
+ * Register a callback function that is invoked when the vm starts or stops
+ * running.
+ *
+ * Returns: an entry to be freed using qemu_del_vm_change_state_handler()
+ */
 VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
                                                      void *opaque);
 VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
-- 
2.49.0

Re: [PATCH v6 03/39] system/runstate: Document qemu_add_vm_change_state_handler()

Posted by Zhao Liu 5 months, 2 weeks ago

On Thu, Jul 03, 2025 at 07:32:09PM +0200, Philippe Mathieu-Daudé wrote:
> Date: Thu,  3 Jul 2025 19:32:09 +0200
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH v6 03/39] system/runstate: Document
>  qemu_add_vm_change_state_handler()
> X-Mailer: git-send-email 2.49.0
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  include/system/runstate.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH v6 03/39] system/runstate: Document qemu_add_vm_change_state_handler()

Posted by Xiaoyao Li 5 months, 2 weeks ago

On 7/4/2025 1:32 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/system/runstate.h | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/include/system/runstate.h b/include/system/runstate.h
> index fdd5c4a5172..b6e8d6beab7 100644
> --- a/include/system/runstate.h
> +++ b/include/system/runstate.h
> @@ -14,6 +14,16 @@ void runstate_replay_enable(void);
>   typedef void VMChangeStateHandler(void *opaque, bool running, RunState state);
>   typedef int VMChangeStateHandlerWithRet(void *opaque, bool running, RunState state);
>   
> +/**
> + * qemu_add_vm_change_state_handler:
> + * @cb: the callback to invoke
> + * @opaque: user data passed to the callback
> + *
> + * Register a callback function that is invoked when the vm starts or stops
> + * running.
> + *
> + * Returns: an entry to be freed using qemu_del_vm_change_state_handler()
> + */

qemu_add_vm_change_state_handler_prio() and 
qemu_add_vm_change_state_handler_prio_full() put the document in the 
implementation in system/runstate.c.

Please make them consistent.

>   VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>                                                        void *opaque);
>   VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(

Re: [PATCH v6 03/39] system/runstate: Document qemu_add_vm_change_state_handler()

Posted by Alex Bennée 5 months ago

Xiaoyao Li <xiaoyao.li@intel.com> writes:

> On 7/4/2025 1:32 AM, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   include/system/runstate.h | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>> diff --git a/include/system/runstate.h b/include/system/runstate.h
>> index fdd5c4a5172..b6e8d6beab7 100644
>> --- a/include/system/runstate.h
>> +++ b/include/system/runstate.h
>> @@ -14,6 +14,16 @@ void runstate_replay_enable(void);
>>   typedef void VMChangeStateHandler(void *opaque, bool running, RunState state);
>>   typedef int VMChangeStateHandlerWithRet(void *opaque, bool running, RunState state);
>>   +/**
>> + * qemu_add_vm_change_state_handler:
>> + * @cb: the callback to invoke
>> + * @opaque: user data passed to the callback
>> + *
>> + * Register a callback function that is invoked when the vm starts or stops
>> + * running.
>> + *
>> + * Returns: an entry to be freed using qemu_del_vm_change_state_handler()
>> + */
>
> qemu_add_vm_change_state_handler_prio() and
> qemu_add_vm_change_state_handler_prio_full() put the document in the
> implementation in system/runstate.c.

Generally APIs to the rest of QEMU should be documented in the headers.
Comments on individual functions or internal details are fine to live in
the C files.

>
> Please make them consistent.
>
>>   VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>>                                                        void *opaque);
>>   VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v6 03/39] system/runstate: Document qemu_add_vm_change_state_handler()

Posted by Xiaoyao Li 5 months ago

On 7/15/2025 5:02 PM, Alex Bennée wrote:
> Xiaoyao Li <xiaoyao.li@intel.com> writes:
> 
>> On 7/4/2025 1:32 AM, Philippe Mathieu-Daudé wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    include/system/runstate.h | 10 ++++++++++
>>>    1 file changed, 10 insertions(+)
>>> diff --git a/include/system/runstate.h b/include/system/runstate.h
>>> index fdd5c4a5172..b6e8d6beab7 100644
>>> --- a/include/system/runstate.h
>>> +++ b/include/system/runstate.h
>>> @@ -14,6 +14,16 @@ void runstate_replay_enable(void);
>>>    typedef void VMChangeStateHandler(void *opaque, bool running, RunState state);
>>>    typedef int VMChangeStateHandlerWithRet(void *opaque, bool running, RunState state);
>>>    +/**
>>> + * qemu_add_vm_change_state_handler:
>>> + * @cb: the callback to invoke
>>> + * @opaque: user data passed to the callback
>>> + *
>>> + * Register a callback function that is invoked when the vm starts or stops
>>> + * running.
>>> + *
>>> + * Returns: an entry to be freed using qemu_del_vm_change_state_handler()
>>> + */
>>
>> qemu_add_vm_change_state_handler_prio() and
>> qemu_add_vm_change_state_handler_prio_full() put the document in the
>> implementation in system/runstate.c.
> 
> Generally APIs to the rest of QEMU should be documented in the headers.
> Comments on individual functions or internal details are fine to live in
> the C files.

I totally understand it.

I was not asking to put the document into C files, but to ...

>>
>> Please make them consistent.

... make them consistent. IOW, I would expect an additional patch to 
move the document of qemu_add_vm_change_state_handler_prio() and 
qemu_add_vm_change_state_handler_prio_full() from C files to this header 
file.

>>>    VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler *cb,
>>>                                                         void *opaque);
>>>    VMChangeStateEntry *qemu_add_vm_change_state_handler_prio(
>