Re: [PATCH 2/3] migration: implement query migration threadinfo by name
Jiang Jiacheng wrote: > On 2023/1/30 22:03, Juan Quintela wrote: >> Jiang Jiacheng wrote: >>> On 2023/1/30 12:27, Juan Quintela wrote: 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. Ok, fine enough. Thanks. Later, Juan.
Re: [PATCH 2/3] migration: implement query migration threadinfo by name
On 2023/1/30 22:03, Juan Quintela wrote:
> Jiang Jiacheng wrote:
>> On 2023/1/30 12:27, Juan Quintela wrote:
>>> Jiang Jiacheng 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
>>>
>>> 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.
>
Re: [PATCH 2/3] migration: implement query migration threadinfo by name
Jiang Jiacheng wrote:
> On 2023/1/30 12:27, Juan Quintela wrote:
>> Jiang Jiacheng 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
>>
>> 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.
Re: [PATCH 2/3] migration: implement query migration threadinfo by name
On 2023/1/30 12:27, Juan Quintela wrote:
> Jiang Jiacheng 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
>
> 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.
>
Re: [PATCH 2/3] migration: implement query migration threadinfo by name
Jiang Jiacheng 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
I like this query interface better than the way you expose the thread
name in the 1st place.
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.
> +##
> +# @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?
Later, Juan.
[PATCH 2/3] migration: implement query migration threadinfo by name
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
---
migration/meson.build | 1 +
migration/threadinfo.c | 70 ++
migration/threadinfo.h | 30 ++
qapi/migration.json| 30 ++
4 files changed, 131 insertions(+)
create mode 100644 migration/threadinfo.c
create mode 100644 migration/threadinfo.h
diff --git a/migration/meson.build b/migration/meson.build
index 690487cf1a..ed7d27f11a 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -25,6 +25,7 @@ softmmu_ss.add(files(
'savevm.c',
'socket.c',
'tls.c',
+ 'threadinfo.c',
), gnutls)
softmmu_ss.add(when: rdma, if_true: files('rdma.c'))
diff --git a/migration/threadinfo.c b/migration/threadinfo.c
new file mode 100644
index 00..0df668d427
--- /dev/null
+++ b/migration/threadinfo.c
@@ -0,0 +1,70 @@
+/*
+ * Migration Threads info
+ *
+ * Copyright (c) 2022 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ * Jiang Jiacheng
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "threadinfo.h"
+
+static QLIST_HEAD(, MigrationThread) migration_threads;
+
+MigrationThread* MigrationThreadAdd(const char *name, int thread_id)
+{
+MigrationThread *thread = NULL;
+
+thread = g_new0(MigrationThread, 1);
+thread->name = (char*)name;
+thread->thread_id = thread_id;
+
+QLIST_INSERT_HEAD(_threads, thread, node);
+
+return thread;
+}
+
+void MigrationThreadDel(MigrationThread* thread)
+{
+if (thread) {
+QLIST_REMOVE(thread, node);
+ g_free(thread);
+}
+}
+
+MigrationThread* MigrationThreadQuery(const char* name)
+{
+MigrationThread *thread = NULL;
+
+QLIST_FOREACH(thread, _threads, node) {
+if (!strcmp(thread->name, name)) {
+return thread;
+}
+}
+
+return NULL;
+}
+
+MigrationThreadInfo* qmp_query_migrationthreads(const char* name, Error **errp)
+{
+MigrationThread *thread = NULL;
+MigrationThreadInfo *info = NULL;
+
+thread = MigrationThreadQuery(name);
+if (!thread) {
+goto err;
+}
+
+info = g_new0(MigrationThreadInfo, 1);
+info->name = g_strdup(thread->name);
+info->thread_id = thread->thread_id;
+
+return info;
+
+err:
+error_setg(errp, "thread '%s' doesn't exist", name);
+return NULL;
+}
diff --git a/migration/threadinfo.h b/migration/threadinfo.h
new file mode 100644
index 00..f62a6ff261
--- /dev/null
+++ b/migration/threadinfo.h
@@ -0,0 +1,30 @@
+/*
+ * Migration Threads info
+ *
+ * Copyright (c) 2022 HUAWEI TECHNOLOGIES CO., LTD.
+ *
+ * Authors:
+ * Jiang Jiacheng
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/queue.h"
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-migration.h"
+
+typedef struct MigrationThread MigrationThread;
+
+struct MigrationThread {
+char *name; /* the name of migration thread */
+int thread_id; /* ID of the underlying host thread */
+QLIST_ENTRY(MigrationThread) node;
+};
+
+MigrationThread *MigrationThreadAdd(const char *name, int thread_id);
+
+void MigrationThreadDel(MigrationThread* info);
+
+MigrationThread* MigrationThreadQuery(const char* name);
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'} }
+
+##
+# @query-migrationthreads:
+#
+# 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:
#
--
2.33.0
