Re: [PATCH v2 1/2] qapi/migration.json: add reason to MIGRATION event

[Markus Armbruster]

Roman Khapov <rkhapov@yandex-team.ru> writes:

> To be clear: you meant that the description of the event must be extended, 
> similar to its description on the commit message? Or you don't see its proper 
> usage at all?

The commit message doesn't really tell me either why and how anybody
would use @reason.  Once we have a common understanding there, improving
the doc comment should be straightforward.

> If the first is true, then I agree, the description can be improved, and it 
> will be useful, thanks! Will add it in the next version of the patch soon.
>
> On 2/16/24 11:17, Markus Armbruster wrote:
>> Roman Khapov <rkhapov@yandex-team.ru> writes:
>>
>>> This commit adds the optional field reason for the events, which
>>> contains the string, describing reason of status changing.
>>> For example: reason of migration fail.
>>>
>>> Function migrate_set_state now accepts 4th argument: the reason to
>>> pass to event. Every call of this function appended with NULL argument.
>>>
>>> Also migrate_set_state_err_reason was added to form reason from Error*
>>>
>>> Signed-off-by: Roman Khapov <rkhapov@yandex-team.ru>
>>
>> [...]
>>
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 5a565d9b8d..33bb5b7f50 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -1392,6 +1392,7 @@
>>>  # Emitted when a migration event happens
>>>  #
>>>  # @status: @MigrationStatus describing the current migration status.
>>> +# @reason: Optional description of status changing reason.
>>
>> Intended use?
>>
>> When is it present?
>>
>>>  #
>>>  # Since: 2.4
>>>  #
>>> @@ -1402,7 +1403,7 @@
>>>  #     "data": {"status": "completed"} }
>>>  ##
>>>  { 'event': 'MIGRATION',
>>> -  'data': {'status': 'MigrationStatus'}}
>>> +  'data': {'status': 'MigrationStatus', '*reason': 'str'}}
>>>  ##
>>>  # @MIGRATION_PASS: