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. >>> +## >>> +# @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. Later, Juan.