On 2023/1/30 12:27, Juan Quintela wrote:
> Jiang Jiacheng <jiangjiach...@huawei.com> wrote:
>> Introduce interface query-migrationthreads. The interface is use
>> with the migration thread name reported by qemu and returns with
>> migration thread name and its pid.
>> Introduce threadinfo.c to manage threads with migration.
>>
>> Signed-off-by: Jiang Jiacheng <jiangjiach...@huawei.com>
>
> I like this query interface better than the way you expose the thread
> name in the 1st place.
The event in patch1 is used to pass the thread name since libvirt
doesn't know the name of the migration thread but the interface below
need its name.
I think the event can be dropped if we store the thread name in
libvirt(if the migration thread name is fixed in qemu) or using the
'query-migrationthreads' you mentioned below.
>
> But once that we are here, why don't we just "tweak" abit the interface:
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index b0cf366ac0..e93c3e560a 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1970,6 +1970,36 @@
>> { 'command': 'query-vcpu-dirty-limit',
>> 'returns': [ 'DirtyLimitInfo' ] }
>>
>> +##
>> +# @MigrationThreadInfo:
>> +#
>> +# Information about migrationthreads
>> +#
>> +# @name: the name of migration thread
>> +#
>> +# @thread-id: ID of the underlying host thread
>> +#
>> +# Since: 7.2
>> +##
>> +{ 'struct': 'MigrationThreadInfo',
>> + 'data': {'name': 'str',
>> + 'thread-id': 'int'} }
>
> 1st, it is an int enough for all architectures? I know that for linux
> and friends it is, but not sure about windows and other weird systems.
>
It is only enough for migration pin which I want to implement. But I
think this struct can be easily expand if we need other information
about migration thread.
>> +##
>> +# @query-migrationthreads:
>
> What about renaming this to:
>
> @query-migrationthread (I removed the last 's')
>
> because it returns the info of a single thread.
>
>> +#
>> +# Returns threadInfo about the thread specified by name
>> +#
>> +# data: migration thread name
>> +#
>> +# returns: information about the specified thread
>> +#
>> +# Since: 7.2
>> +##
>> +{ 'command': 'query-migrationthreads',
>> + 'data': { 'name': 'str' },
>> + 'returns': 'MigrationThreadInfo' }
>> +
>> ##
>> # @snapshot-save:
>> #
>
> And leaves the @query-migrationthreads name for something in the spirit of
>
>> +{ 'command': 'query-migrationthreads',
>> + 'returns': ['str'] }
>
> That returns all the migration threads names.
>
> Or perhaps even
>
>> +{ 'command': 'query-migrationthreads',
>> + 'returns': ['MigrationThreadInfo'] }
>
> And call it a day?
I think it's the best. If in this way, should we keep the interface to
query specified thread?
Thanks, Jiang Jiacheng
>
> Later, Juan.
>
