diff mbox series

[10/42] job: Add JobDriver.job_type

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

Commit Message

Kevin Wolf May 9, 2018, 4:26 p.m. UTC
This moves the job_type field from BlockJobDriver to JobDriver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/block/blockjob_int.h |  3 ---
 include/qemu/job.h           | 11 +++++++++++
 block/backup.c               |  2 +-
 block/commit.c               |  2 +-
 block/mirror.c               |  4 ++--
 block/stream.c               |  2 +-
 blockjob.c                   | 16 +++++++---------
 job.c                        | 10 ++++++++++
 8 files changed, 33 insertions(+), 17 deletions(-)

Comments

Max Reitz May 11, 2018, 10:53 p.m. UTC | #1
On 2018-05-09 18:26, Kevin Wolf wrote:
> This moves the job_type field from BlockJobDriver to JobDriver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/block/blockjob_int.h |  3 ---
>  include/qemu/job.h           | 11 +++++++++++
>  block/backup.c               |  2 +-
>  block/commit.c               |  2 +-
>  block/mirror.c               |  4 ++--
>  block/stream.c               |  2 +-
>  blockjob.c                   | 16 +++++++---------
>  job.c                        | 10 ++++++++++
>  8 files changed, 33 insertions(+), 17 deletions(-)
> 

[...]

> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index b4b49f19e1..c87e951c8a 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h

[...]

> @@ -57,4 +62,10 @@ struct JobDriver {
>   */
>  void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
>  
> +/** Returns the JobType of a given Job. */
> +JobType job_type(Job *job);
> +
> +/** Returns the enum string for the JobType of a given Job. */
> +const char *job_type_str(Job *job);
> +

Is there a good reason for these not to take a const Job *?

Depending on the answer:

Reviewed-by: Max Reitz <mreitz@redhat.com>

>  #endif
Kevin Wolf May 14, 2018, 11:31 a.m. UTC | #2
Am 12.05.2018 um 00:53 hat Max Reitz geschrieben:
> On 2018-05-09 18:26, Kevin Wolf wrote:
> > This moves the job_type field from BlockJobDriver to JobDriver.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  include/block/blockjob_int.h |  3 ---
> >  include/qemu/job.h           | 11 +++++++++++
> >  block/backup.c               |  2 +-
> >  block/commit.c               |  2 +-
> >  block/mirror.c               |  4 ++--
> >  block/stream.c               |  2 +-
> >  blockjob.c                   | 16 +++++++---------
> >  job.c                        | 10 ++++++++++
> >  8 files changed, 33 insertions(+), 17 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/include/qemu/job.h b/include/qemu/job.h
> > index b4b49f19e1..c87e951c8a 100644
> > --- a/include/qemu/job.h
> > +++ b/include/qemu/job.h
> 
> [...]
> 
> > @@ -57,4 +62,10 @@ struct JobDriver {
> >   */
> >  void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
> >  
> > +/** Returns the JobType of a given Job. */
> > +JobType job_type(Job *job);
> > +
> > +/** Returns the enum string for the JobType of a given Job. */
> > +const char *job_type_str(Job *job);
> > +
> 
> Is there a good reason for these not to take a const Job *?
> 
> Depending on the answer:
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>

Not really. I'll change it and take your R-b.

Kevin
John Snow May 14, 2018, 8:12 p.m. UTC | #3
On 05/11/2018 06:53 PM, Max Reitz wrote:
> On 2018-05-09 18:26, Kevin Wolf wrote:
>> This moves the job_type field from BlockJobDriver to JobDriver.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

>> ---
>>  include/block/blockjob_int.h |  3 ---
>>  include/qemu/job.h           | 11 +++++++++++
>>  block/backup.c               |  2 +-
>>  block/commit.c               |  2 +-
>>  block/mirror.c               |  4 ++--
>>  block/stream.c               |  2 +-
>>  blockjob.c                   | 16 +++++++---------
>>  job.c                        | 10 ++++++++++
>>  8 files changed, 33 insertions(+), 17 deletions(-)
>>
> 
> [...]
> 
>> diff --git a/include/qemu/job.h b/include/qemu/job.h
>> index b4b49f19e1..c87e951c8a 100644
>> --- a/include/qemu/job.h
>> +++ b/include/qemu/job.h
> 
> [...]
> 
>> @@ -57,4 +62,10 @@ struct JobDriver {
>>   */
>>  void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
>>  
>> +/** Returns the JobType of a given Job. */
>> +JobType job_type(Job *job);
>> +
>> +/** Returns the enum string for the JobType of a given Job. */
>> +const char *job_type_str(Job *job);
>> +
> 
> Is there a good reason for these not to take a const Job *?
> 
> Depending on the answer:
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
>>  #endif
> 

Max is just very excited to talk about Rust some more, I can tell.
diff mbox series

Patch

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 8e7e0a2f57..1e62d6dd30 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -38,9 +38,6 @@  struct BlockJobDriver {
     /** Generic JobDriver callbacks and settings */
     JobDriver job_driver;
 
-    /** String describing the operation, part of query-block-jobs QMP API */
-    JobType job_type;
-
     /** Mandatory: Entrypoint for the Coroutine. */
     CoroutineEntry *start;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index b4b49f19e1..c87e951c8a 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -26,6 +26,8 @@ 
 #ifndef JOB_H
 #define JOB_H
 
+#include "qapi/qapi-types-block-core.h"
+
 typedef struct JobDriver JobDriver;
 
 /**
@@ -45,6 +47,9 @@  typedef struct Job {
 struct JobDriver {
     /** Derived Job struct size */
     size_t instance_size;
+
+    /** Enum describing the operation */
+    JobType job_type;
 };
 
 
@@ -57,4 +62,10 @@  struct JobDriver {
  */
 void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
 
+/** Returns the JobType of a given Job. */
+JobType job_type(Job *job);
+
+/** Returns the enum string for the JobType of a given Job. */
+const char *job_type_str(Job *job);
+
 #endif
diff --git a/block/backup.c b/block/backup.c
index c49ea92dca..baf8d432da 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -525,8 +525,8 @@  static void coroutine_fn backup_run(void *opaque)
 static const BlockJobDriver backup_job_driver = {
     .job_driver = {
         .instance_size          = sizeof(BackupBlockJob),
+        .job_type               = JOB_TYPE_BACKUP,
     },
-    .job_type               = JOB_TYPE_BACKUP,
     .start                  = backup_run,
     .commit                 = backup_commit,
     .abort                  = backup_abort,
diff --git a/block/commit.c b/block/commit.c
index afa2b2bacf..32d29c890e 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -217,8 +217,8 @@  out:
 static const BlockJobDriver commit_job_driver = {
     .job_driver = {
         .instance_size = sizeof(CommitBlockJob),
+        .job_type      = JOB_TYPE_COMMIT,
     },
-    .job_type      = JOB_TYPE_COMMIT,
     .start         = commit_run,
 };
 
diff --git a/block/mirror.c b/block/mirror.c
index 243a58f465..054972bdc1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -988,8 +988,8 @@  static void mirror_drain(BlockJob *job)
 static const BlockJobDriver mirror_job_driver = {
     .job_driver = {
         .instance_size          = sizeof(MirrorBlockJob),
+        .job_type               = JOB_TYPE_MIRROR,
     },
-    .job_type               = JOB_TYPE_MIRROR,
     .start                  = mirror_run,
     .complete               = mirror_complete,
     .pause                  = mirror_pause,
@@ -1000,8 +1000,8 @@  static const BlockJobDriver mirror_job_driver = {
 static const BlockJobDriver commit_active_job_driver = {
     .job_driver = {
         .instance_size          = sizeof(MirrorBlockJob),
+        .job_type               = JOB_TYPE_COMMIT,
     },
-    .job_type               = JOB_TYPE_COMMIT,
     .start                  = mirror_run,
     .complete               = mirror_complete,
     .pause                  = mirror_pause,
diff --git a/block/stream.c b/block/stream.c
index 048bceb5d0..cb723f190a 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -211,8 +211,8 @@  out:
 static const BlockJobDriver stream_job_driver = {
     .job_driver = {
         .instance_size = sizeof(StreamBlockJob),
+        .job_type      = JOB_TYPE_STREAM,
     },
-    .job_type      = JOB_TYPE_STREAM,
     .start         = stream_run,
 };
 
diff --git a/blockjob.c b/blockjob.c
index 788b8cf686..3cf28eb730 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -295,9 +295,7 @@  static void block_job_detach_aio_context(void *opaque)
 static char *child_job_get_parent_desc(BdrvChild *c)
 {
     BlockJob *job = c->opaque;
-    return g_strdup_printf("%s job '%s'",
-                           JobType_str(job->driver->job_type),
-                           job->job.id);
+    return g_strdup_printf("%s job '%s'", job_type_str(&job->job), job->job.id);
 }
 
 static void child_job_drained_begin(BdrvChild *c)
@@ -839,7 +837,7 @@  BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
         return NULL;
     }
     info = g_new0(BlockJobInfo, 1);
-    info->type      = g_strdup(JobType_str(job->driver->job_type));
+    info->type      = g_strdup(job_type_str(&job->job));
     info->device    = g_strdup(job->job.id);
     info->len       = job->len;
     info->busy      = atomic_read(&job->busy);
@@ -870,7 +868,7 @@  static void block_job_event_cancelled(BlockJob *job)
         return;
     }
 
-    qapi_event_send_block_job_cancelled(job->driver->job_type,
+    qapi_event_send_block_job_cancelled(job_type(&job->job),
                                         job->job.id,
                                         job->len,
                                         job->offset,
@@ -884,7 +882,7 @@  static void block_job_event_completed(BlockJob *job, const char *msg)
         return;
     }
 
-    qapi_event_send_block_job_completed(job->driver->job_type,
+    qapi_event_send_block_job_completed(job_type(&job->job),
                                         job->job.id,
                                         job->len,
                                         job->offset,
@@ -898,7 +896,7 @@  static int block_job_event_pending(BlockJob *job)
 {
     block_job_state_transition(job, BLOCK_JOB_STATUS_PENDING);
     if (!job->auto_finalize && !block_job_is_internal(job)) {
-        qapi_event_send_block_job_pending(job->driver->job_type,
+        qapi_event_send_block_job_pending(job_type(&job->job),
                                           job->job.id,
                                           &error_abort);
     }
@@ -972,7 +970,7 @@  void *block_job_create(const char *job_id, const BlockJobDriver *driver,
                    block_job_sleep_timer_cb, job);
 
     error_setg(&job->blocker, "block device is in use by block job: %s",
-               JobType_str(driver->job_type));
+               job_type_str(&job->job));
     block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
     bs->job = job;
 
@@ -1176,7 +1174,7 @@  void block_job_event_ready(BlockJob *job)
         return;
     }
 
-    qapi_event_send_block_job_ready(job->driver->job_type,
+    qapi_event_send_block_job_ready(job_type(&job->job),
                                     job->job.id,
                                     job->len,
                                     job->offset,
diff --git a/job.c b/job.c
index 87fd48468c..f00f401502 100644
--- a/job.c
+++ b/job.c
@@ -29,6 +29,16 @@ 
 #include "qemu/job.h"
 #include "qemu/id.h"
 
+JobType job_type(Job *job)
+{
+    return job->driver->job_type;
+}
+
+const char *job_type_str(Job *job)
+{
+    return JobType_str(job_type(job));
+}
+
 void *job_create(const char *job_id, const JobDriver *driver, Error **errp)
 {
     Job *job;