Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

[Kevin Wolf]

Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> Add query-block-jobs, which shows the progress of ongoing block device
> operations.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  blockdev.c       |   33 +++++++++++++++++++++++++++++++++
>  hmp.c            |   40 ++++++++++++++++++++++++++++++++++++++++
>  hmp.h            |    1 +
>  monitor.c        |    7 +++++++
>  qapi-schema.json |   32 ++++++++++++++++++++++++++++++++
>  qmp-commands.hx  |   39 +++++++++++++++++++++++++++++++++++++++
>  6 files changed, 152 insertions(+), 0 deletions(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index b276b2f..5b2b128 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -997,3 +997,36 @@ void qmp_block_job_cancel(const char *device, Error 
> **errp)
>      trace_qmp_block_job_cancel(job);
>      block_job_cancel(job);
>  }
> +
> +static void do_qmp_query_block_jobs_one(void *opaque, BlockDriverState *bs)
> +{
> +    BlockJobInfoList **prev = opaque;
> +    BlockJob *job = bs->job;
> +
> +    if (job) {
> +        BlockJobInfoList *elem;
> +        BlockJobInfo *info = g_new(BlockJobInfo, 1);
> +        *info = (BlockJobInfo){
> +            .type = g_strdup(job->job_type->job_type),
> +            .device = g_strdup(bdrv_get_device_name(bs)),
> +            .len = job->len,
> +            .offset = job->offset,
> +            .speed = job->speed,
> +        };

Some spaces to align it nicely?

> +
> +        elem = g_new0(BlockJobInfoList, 1);
> +        elem->value = info;
> +
> +        (*prev)->next = elem;
> +        *prev = elem;

I hate these open-coded lists. But not your fault...

> +    }
> +}
> +
> +BlockJobInfoList *qmp_query_block_jobs(Error **errp)
> +{
> +    /* Dummy is a fake list element for holding the head pointer */
> +    BlockJobInfoList dummy = {};
> +    BlockJobInfoList *prev = &dummy;
> +    bdrv_iterate(do_qmp_query_block_jobs_one, &prev);
> +    return dummy.next;
> +}
> diff --git a/hmp.c b/hmp.c
> index 66d9d0f..c16d6a1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
>      qapi_free_PciInfoList(info);
>  }
>  
> +void hmp_info_block_jobs(Monitor *mon)
> +{
> +    BlockJobInfoList *list;
> +    Error *err = NULL;
> +
> +    list = qmp_query_block_jobs(&err);
> +    assert(!err);
> +
> +    if (!list) {
> +        monitor_printf(mon, "No active jobs\n");
> +        return;
> +    }
> +
> +    while (list) {
> +        /* The HMP output for streaming jobs is special because historically 
> it
> +         * was different from other job types so applications may depend on 
> the
> +         * exact string.
> +         */

Er, what? This is new code. What HMP clients use this string? I know
that libvirt already got support for this before we implemented it, but
shouldn't that be QMP only?

Kevin