Re: [PATCH V2 1/2] migration: Introduce interface query-migrationthreads

[Juan Quintela <quintela@redhat.com>]

Jiang Jiacheng <jiangjiacheng@huawei.com> wrote:
> Introduce interface query-migrationthreads. The interface is used
> to query information about migration threads and returns with
> migration thread's name and its id.
> Introduce threadinfo.c to manage threads with migration.
>
> Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

I will fix this by hand.

> +MigrationThread* MigrationThreadAdd(const char *name, int thread_id)
* on the left instead of the right.

> +{
> +    MigrationThread *thread = NULL;
> +
> +    thread = g_new0(MigrationThread, 1);

This is too much.

      MigrationThread *thread = g_new0(MigrationThread, 1);

If you initialize a variable, don't do a NULL and just in the next line
the right value.


> +    thread->name = (char*)name;

Why not doing it properly and call it just const char * on the header?

> +    thread->thread_id = thread_id;
> +
> +    QLIST_INSERT_HEAD(&migration_threads, thread, node);
> +
> +    return thread;
> +}
> +
> +void MigrationThreadDel(MigrationThread* thread)
> +{
> +    if (thread) {
> +        QLIST_REMOVE(thread, node);

Spaces for indentation

> +	    g_free(thread);

Tabs + spaces.

> +    }
> +}
> +
> +MigrationThreadInfoList* qmp_query_migrationthreads(Error **errp)

* on the worong side.

> +{
> +    MigrationThreadInfoList *head = NULL;
> +    MigrationThreadInfoList **tail = &head;
> +    MigrationThread *thread = NULL;
> +    MigrationThreadInfo *info = NULL;
> +
> +    QLIST_FOREACH(thread, &migration_threads, node) {
> +        info = g_new0(MigrationThreadInfo, 1);

MigrationThreadInfo *info = g_new0(MigrationThreadInfo, 1);

> +        info->name = g_strdup(thread->name);
> +        info->thread_id = thread->thread_id;
> +
> +        QAPI_LIST_APPEND(tail, info);
> +    }
> +
> +    return head;
> +}
> diff --git a/migration/threadinfo.h b/migration/threadinfo.h
> new file mode 100644
> index 0000000000..ebe86a2699
> --- /dev/null
> +++ b/migration/threadinfo.h
> @@ -0,0 +1,28 @@
> +/*
> + *  Migration Threads info
> + *
> + *  Copyright (c) 2022 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + *  Authors:
> + *  Jiang Jiacheng <jiangjiacheng@huawei.com>
> + *
> + *  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);
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 88ecf86ac8..c84fa10e86 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1958,6 +1958,35 @@
>  { '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

            8.0

Same on the next one.

> +##
> +{ 'struct': 'MigrationThreadInfo',
> +  'data': {'name': 'str',
> +           'thread-id': 'int'} }
> +
> +##
> +# @query-migrationthreads:
> +#
> +# Returns information of migration threads
> +#
> +# data: migration thread name
> +#
> +# returns: information about migration threads
> +#
> +# Since: 7.2
> +##
> +{ 'command': 'query-migrationthreads',
> +  'returns': ['MigrationThreadInfo'] }
> +
>  ##
>  # @snapshot-save:
>  #

Later, Juan.