diff mbox series

[v2,37/40] job: Add query-jobs QMP command

Message ID 20180518132114.4070-38-kwolf@redhat.com
State New
Headers show
Series Generic background jobs | expand

Commit Message

Kevin Wolf May 18, 2018, 1:21 p.m. UTC
This adds a minimal query-jobs implementation that shouldn't pose many
design questions. It can later be extended to expose more information,
and especially job-specific information.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/job.json      | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/job.h |  3 +++
 job-qmp.c          | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 job.c              |  2 +-
 4 files changed, 103 insertions(+), 1 deletion(-)

Comments

Eric Blake May 18, 2018, 6:14 p.m. UTC | #1
On 05/18/2018 08:21 AM, Kevin Wolf wrote:
> This adds a minimal query-jobs implementation that shouldn't pose many
> design questions. It can later be extended to expose more information,
> and especially job-specific information.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qapi/job.json      | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   include/qemu/job.h |  3 +++
>   job-qmp.c          | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   job.c              |  2 +-
>   4 files changed, 103 insertions(+), 1 deletion(-)

> +#
> +# @current-progress:    Progress made until now. The unit is arbitrary and the
> +#                       value can only meaningfully be used for the ratio of
> +#                       @offset to @len. The value is monotonically increasing.

s/@offset to @len/@current-progress to @total-progress/

> +#
> +# @total-progress:      Estimated @offset value at the completion of the job.

s/@offset/@current-progress/

> +#                       This value can arbitrarily change while the job is
> +#                       running, in both directions.
> +#
> +# @error:               If this field is present, the job failed; if it is
> +#                       still missing in the CONCLUDED state, this indicates
> +#                       successful completion.
> +#
> +#                       The value is a human-readable error message to describe
> +#                       the reason for the job failure. It should not be parsed
> +#                       by applications.
> +#
> +# Since: 2.13
> +##
> +{ 'struct': 'JobInfo',
> +  'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus',
> +            'current-progress': 'int', 'total-progress': 'int',
> +            '*error': 'str' } }
> +
Eric Blake May 18, 2018, 6:22 p.m. UTC | #2
On 05/18/2018 08:21 AM, Kevin Wolf wrote:
> This adds a minimal query-jobs implementation that shouldn't pose many
> design questions. It can later be extended to expose more information,
> and especially job-specific information.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   qapi/job.json      | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   include/qemu/job.h |  3 +++
>   job-qmp.c          | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   job.c              |  2 +-
>   4 files changed, 103 insertions(+), 1 deletion(-)
> 

> +##
> +# @JobInfo:
> +#
> +# Information about a job.
> +#
> +# @id:                  The job identifier

> +##
> +{ 'struct': 'JobInfo',
> +  'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus',
> +            'current-progress': 'int', 'total-progress': 'int',
> +            '*error': 'str' } }

Is it worth exposing whether a job is auto-finalize and auto-complete? 
Goes back to the issue of whether clients of the new job API would ever 
want/need to rely on the auto- features; while clients of the old 
blockjob API that get the auto- features by default will never be 
calling the new query-jobs command.
Kevin Wolf May 22, 2018, 10:44 a.m. UTC | #3
Am 18.05.2018 um 20:22 hat Eric Blake geschrieben:
> On 05/18/2018 08:21 AM, Kevin Wolf wrote:
> > This adds a minimal query-jobs implementation that shouldn't pose many
> > design questions. It can later be extended to expose more information,
> > and especially job-specific information.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >   qapi/job.json      | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >   include/qemu/job.h |  3 +++
> >   job-qmp.c          | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   job.c              |  2 +-
> >   4 files changed, 103 insertions(+), 1 deletion(-)
> > 
> 
> > +##
> > +# @JobInfo:
> > +#
> > +# Information about a job.
> > +#
> > +# @id:                  The job identifier
> 
> > +##
> > +{ 'struct': 'JobInfo',
> > +  'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus',
> > +            'current-progress': 'int', 'total-progress': 'int',
> > +            '*error': 'str' } }
> 
> Is it worth exposing whether a job is auto-finalize and auto-complete? Goes
> back to the issue of whether clients of the new job API would ever want/need
> to rely on the auto- features; while clients of the old blockjob API that
> get the auto- features by default will never be calling the new query-jobs
> command.

There are probably more fields that could be exposed. For most of them,
it's not obvious whether we want to expose them, so I went for the
minimal useful information here. We can always add more information to a
query command; but we can't take information away later if it turns out
that exposing it was a bad idea.

Kevin
diff mbox series

Patch

diff --git a/qapi/job.json b/qapi/job.json
index b84dc6c820..932d5928e9 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -205,3 +205,48 @@ 
 # Since: 2.13
 ##
 { 'command': 'job-finalize', 'data': { 'id': 'str' } }
+
+##
+# @JobInfo:
+#
+# Information about a job.
+#
+# @id:                  The job identifier
+#
+# @type:                The kind of job that is being performed
+#
+# @status:              Current job state/status
+#
+# @current-progress:    Progress made until now. The unit is arbitrary and the
+#                       value can only meaningfully be used for the ratio of
+#                       @offset to @len. The value is monotonically increasing.
+#
+# @total-progress:      Estimated @offset value at the completion of the job.
+#                       This value can arbitrarily change while the job is
+#                       running, in both directions.
+#
+# @error:               If this field is present, the job failed; if it is
+#                       still missing in the CONCLUDED state, this indicates
+#                       successful completion.
+#
+#                       The value is a human-readable error message to describe
+#                       the reason for the job failure. It should not be parsed
+#                       by applications.
+#
+# Since: 2.13
+##
+{ 'struct': 'JobInfo',
+  'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus',
+            'current-progress': 'int', 'total-progress': 'int',
+            '*error': 'str' } }
+
+##
+# @query-jobs:
+#
+# Return information about jobs.
+#
+# Returns: a list with a @JobInfo for each active job
+#
+# Since: 2.13
+##
+{ 'command': 'query-jobs', 'returns': ['JobInfo'] }
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 92d1d249fe..8c8badf75e 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -392,6 +392,9 @@  JobType job_type(const Job *job);
 /** Returns the enum string for the JobType of a given Job. */
 const char *job_type_str(const Job *job);
 
+/** Returns true if the job should not be visible to the management layer. */
+bool job_is_internal(Job *job);
+
 /** Returns whether the job is scheduled for cancellation. */
 bool job_is_cancelled(Job *job);
 
diff --git a/job-qmp.c b/job-qmp.c
index b2e18cfd9c..7f38f63336 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -132,3 +132,57 @@  void qmp_job_dismiss(const char *id, Error **errp)
     job_dismiss(&job, errp);
     aio_context_release(aio_context);
 }
+
+static JobInfo *job_query_single(Job *job, Error **errp)
+{
+    JobInfo *info;
+    const char *errmsg = NULL;
+
+    assert(!job_is_internal(job));
+
+    if (job->ret < 0) {
+        errmsg = strerror(-job->ret);
+    }
+
+    info = g_new(JobInfo, 1);
+    *info = (JobInfo) {
+        .id                 = g_strdup(job->id),
+        .type               = job_type(job),
+        .status             = job->status,
+        .current_progress   = job->progress_current,
+        .total_progress     = job->progress_total,
+        .has_error          = !!errmsg,
+        .error              = g_strdup(errmsg),
+    };
+
+    return info;
+}
+
+JobInfoList *qmp_query_jobs(Error **errp)
+{
+    JobInfoList *head = NULL, **p_next = &head;
+    Job *job;
+
+    for (job = job_next(NULL); job; job = job_next(job)) {
+        JobInfoList *elem;
+        AioContext *aio_context;
+
+        if (job_is_internal(job)) {
+            continue;
+        }
+        elem = g_new0(JobInfoList, 1);
+        aio_context = job->aio_context;
+        aio_context_acquire(aio_context);
+        elem->value = job_query_single(job, errp);
+        aio_context_release(aio_context);
+        if (!elem->value) {
+            g_free(elem);
+            qapi_free_JobInfoList(head);
+            return NULL;
+        }
+        *p_next = elem;
+        p_next = &elem->next;
+    }
+
+    return head;
+}
diff --git a/job.c b/job.c
index 8e106e5d15..37f32266ba 100644
--- a/job.c
+++ b/job.c
@@ -158,7 +158,7 @@  static int job_txn_apply(JobTxn *txn, int fn(Job *), bool lock)
     return rc;
 }
 
-static bool job_is_internal(Job *job)
+bool job_is_internal(Job *job)
 {
     return (job->id == NULL);
 }