On 2023/1/30 22:03, Juan Quintela wrote:
> Jiang Jiacheng <jiangjiach...@huawei.com> wrote:
>> 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.
>
> I preffer the query migrationthreads, thanks.
>>
>>>
>>> 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.
>
> I mean that pthread_t (what you are passing here) is an int on linux.
> Not sure on other OS's.
>
I'm sorry about my misunderstanding. I use 'int' for thread-id just like
cpu or iothread's thread-id, and it is get from 'qemu_get_thread_id'. So
I think it is enough.
>>>> +##
>>>> +# @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?
>
> I wouldn't do it, but it is up to you.
>
> My (little) understanding of what you want to do is that you want all
> the threads, so I see no reason to have a query for a single one.
>
Thanks for your suggestion. As you said, I need all the threads info,
and the specified thread info can be got easily from the new interface.
The old interface seems to be redundant indeed.
Thanks, Jiang Jiacheng
> Later, Juan.
>
