diff mbox

[v2,02/15] blockjob: Decouple the ID from the device name in the BlockJob struct

Message ID 6034f1747fb838368a057df8463084b6a2326fe4.1466598035.git.berto@igalia.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alberto Garcia June 22, 2016, 12:24 p.m. UTC
Block jobs are identified by the name of the BlockBackend of the BDS
where the job was started.

We want block jobs to have unique, arbitrary identifiers that are not
tied to a block device, so this patch decouples the ID from the device
name in the BlockJob structure.

The ID is generated automatically for the moment, in later patches
we'll allow the user to set it.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/mirror.c            |  3 ++-
 blockjob.c                | 18 +++++++++++-------
 include/block/blockjob.h  | 12 ++++++++----
 include/qapi/qmp/qerror.h |  3 ---
 include/qemu/id.h         |  1 +
 util/id.c                 |  1 +
 6 files changed, 23 insertions(+), 15 deletions(-)

Comments

Kevin Wolf June 22, 2016, 12:42 p.m. UTC | #1
Am 22.06.2016 um 14:24 hat Alberto Garcia geschrieben:
> Block jobs are identified by the name of the BlockBackend of the BDS
> where the job was started.

Let me rephrase that: Block jobs are identified by an ID which defaults
to the name of the BlockBackend where the job was started and can't
currently be set by the user.

> We want block jobs to have unique, arbitrary identifiers that are not
> tied to a block device, so this patch decouples the ID from the device
> name in the BlockJob structure.
> 
> The ID is generated automatically for the moment, in later patches
> we'll allow the user to set it.

This approach requires us to keep track of both a device name and an ID
in order to maintain compatibility, and we always need to check both
when a job is referenced. This model is already a pain with node-name
and BB name, I wouldn't want to introduce more instances.

Why don't we go the easy route and make the ID configurable, but still
default to the device name?

> Signed-off-by: Alberto Garcia <berto@igalia.com>

> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
>      /**
> +     * BlockBackend name of the BDS owning the job at the time when
> +     * the job is started. For compatibility with clients that don't
> +     * support the ID field.
> +     */
> +    char *device;

This in particular feels like a step backwards. This addition is
tightening the coupling between jobs and devices instead of removing it.

> @@ -464,7 +468,7 @@ BlockJobInfo *block_job_query(BlockJob *job)
>  {
>      BlockJobInfo *info = g_new0(BlockJobInfo, 1);
>      info->type      = g_strdup(BlockJobType_lookup[job->driver->job_type]);
> -    info->device    = g_strdup(job->id);
> +    info->device    = g_strdup(job->device);
>      info->len       = job->len;
>      info->busy      = job->busy;
>      info->paused    = job->pause_count > 0;
> @@ -486,7 +490,7 @@ static void block_job_iostatus_set_err(BlockJob *job, int error)
>  void block_job_event_cancelled(BlockJob *job)
>  {
>      qapi_event_send_block_job_cancelled(job->driver->job_type,
> -                                        job->id,
> +                                        job->device,
>                                          job->len,
>                                          job->offset,
>                                          job->speed,
> @@ -496,7 +500,7 @@ void block_job_event_cancelled(BlockJob *job)
>  void block_job_event_completed(BlockJob *job, const char *msg)
>  {
>      qapi_event_send_block_job_completed(job->driver->job_type,
> -                                        job->id,
> +                                        job->device,
>                                          job->len,
>                                          job->offset,
>                                          job->speed,
> @@ -510,7 +514,7 @@ void block_job_event_ready(BlockJob *job)
>      job->ready = true;
>  
>      qapi_event_send_block_job_ready(job->driver->job_type,
> -                                    job->id,
> +                                    job->device,
>                                      job->len,
>                                      job->offset,
>                                      job->speed, &error_abort);
> @@ -538,7 +542,7 @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
>      default:
>          abort();
>      }
> -    qapi_event_send_block_job_error(job->id,
> +    qapi_event_send_block_job_error(job->device,
>                                      is_read ? IO_OPERATION_TYPE_READ :
>                                      IO_OPERATION_TYPE_WRITE,
>                                      action, &error_abort);

These monitor related functions are the only users of the device name.
Let's simply redefine the respective QMP fields to refer to the job ID
rather than the device (because identifying the job is really what they
are used for; once we allow more than one block job per device, putting
the device there will be completely useless).

All of these redefinitions are backwards compatible. Only clients which
are new enough to set an individual ID can get something that isn't a
device name here. And they will be prepared for it.

Kevin
Alberto Garcia June 22, 2016, 2:42 p.m. UTC | #2
On Wed 22 Jun 2016 02:42:24 PM CEST, Kevin Wolf wrote:
>> We want block jobs to have unique, arbitrary identifiers that are not
>> tied to a block device, so this patch decouples the ID from the
>> device name in the BlockJob structure.
>> 
>> The ID is generated automatically for the moment, in later patches
>> we'll allow the user to set it.
>
> This approach requires us to keep track of both a device name and an
> ID in order to maintain compatibility, and we always need to check
> both when a job is referenced. This model is already a pain with
> node-name and BB name, I wouldn't want to introduce more instances.
>
> Why don't we go the easy route and make the ID configurable, but still
> default to the device name?

That was my first approach when I started to write the series, but I
discarded it for several reasons:

- More confusing API: we'd have many instances of fields and parameters
  named 'device' holding something that is not a device name or anything
  similar.

  We'd have e.g. block-commit(id, device), and the created job would
  have a 'device' that is not the user-specified 'device' but the
  user-specified 'id', which has no relation to devices.

- More risk of collisions: with the current system if no ID is specified
  a new one is be generated. That's guaranteed to be valid and unique.
  If we default to the device name we have no such guarantee.

- Potential compatibility break: there's a field that used to hold the
  name of the device but it no longer does.

I don't think any of them is a showstopper, and as you said if a client
is recent enough to understand job IDs it should also expect the device
field to hold something different from a device name. But I'm not a big
fan of this kind of scenarios where the semantics of a returned value
depend on what we can assume that the caller is able to handle.

I thought adding a new 'ID' field was simpler. The device name is still
a device name (where it makes sense). The default ID is guaranteed to be
valid and guaranteed not to clash with user-defined IDs. The API is (in
my opinion) more clear.

The only problems that I can think of:

- BlockJobInfo and the events expose the 'device' field which is
  superfluous.
- block-job-{pause,resume,...} can take an ID or a device name.

All that said, I'm open to change the implementation, but I'd like to
make sure that the problems of keeping the ID and device name merged are
considered.

Berto
Kevin Wolf June 22, 2016, 3:49 p.m. UTC | #3
Am 22.06.2016 um 16:42 hat Alberto Garcia geschrieben:
> On Wed 22 Jun 2016 02:42:24 PM CEST, Kevin Wolf wrote:
> >> We want block jobs to have unique, arbitrary identifiers that are not
> >> tied to a block device, so this patch decouples the ID from the
> >> device name in the BlockJob structure.
> >> 
> >> The ID is generated automatically for the moment, in later patches
> >> we'll allow the user to set it.
> >
> > This approach requires us to keep track of both a device name and an
> > ID in order to maintain compatibility, and we always need to check
> > both when a job is referenced. This model is already a pain with
> > node-name and BB name, I wouldn't want to introduce more instances.
> >
> > Why don't we go the easy route and make the ID configurable, but still
> > default to the device name?
> 
> That was my first approach when I started to write the series, but I
> discarded it for several reasons:
> 
> - More confusing API: we'd have many instances of fields and parameters
>   named 'device' holding something that is not a device name or anything
>   similar.
> 
>   We'd have e.g. block-commit(id, device), and the created job would
>   have a 'device' that is not the user-specified 'device' but the
>   user-specified 'id', which has no relation to devices.

Yes, the 'device' field ends up having the wrong name then. This is
ugly, but I think having code to deal with two ways of addressing jobs,
in two different namespaces, is a cost too high to be justifiable just
for making things look nicer.

I seem to remember that John is going to introduce a new set of job
management functions anyway while he generalises block jobs to just
background jobs, so in that case the ugliness would be confined to a
deprecated legacy interface. I think that's tolerable.

> - More risk of collisions: with the current system if no ID is specified
>   a new one is be generated. That's guaranteed to be valid and unique.
>   If we default to the device name we have no such guarantee.

The expectation is that if a client passes an ID for one job, it will
pass IDs for all jobs. As long as you never pass an ID, the interface
looks exactly the same as it used to. If you pass it always, you're
obviously responsible for choosing unique IDs.

The only vaguely interesting case is if you mix both styles and then
create a new job with an explicit ID that you already used for a block
device. In this case you get an error, and I'd argue you deserve it.

> - Potential compatibility break: there's a field that used to hold the
>   name of the device but it no longer does.

Old clients never set an explicit ID, so what they get in this field is
always the default, which is a device name.

I don't see the compatibility problem?

> I don't think any of them is a showstopper, and as you said if a client
> is recent enough to understand job IDs it should also expect the device
> field to hold something different from a device name. But I'm not a big
> fan of this kind of scenarios where the semantics of a returned value
> depend on what we can assume that the caller is able to handle.

It's really just another case of making something configurable and
letting it default to the previously hardcoded value. Pretty much a
normal thing to do.

> I thought adding a new 'ID' field was simpler. The device name is still
> a device name (where it makes sense). The default ID is guaranteed to be
> valid and guaranteed not to clash with user-defined IDs. The API is (in
> my opinion) more clear.
> 
> The only problems that I can think of:
> 
> - BlockJobInfo and the events expose the 'device' field which is
>   superfluous.
> - block-job-{pause,resume,...} can take an ID or a device name.

Yes. There are two parts that I don't like about this.

The first one is that we need additional code to keep track of the
device name and to look it up.

The other, more important one is that it couples block jobs more tightly
with a BDS:

* What do you with a background job that doesn't have a device
  name because it doesn't work on a BDS? Will 'device' become optional
  everywhere? But how is this less problematic for compatibility than
  returning non-device-name IDs? (To be clear, I don't think either is a
  real problem, but you can hardly dismiss one and accept the other.)

* And what do you do once we allow more than one job per device? Then
  the device name isn't suitable for addressing the job any more. And
  letting the client use the device name after it started the first job,
  but not any more after it started the second one, feels wrong.

Kevin
Alberto Garcia June 23, 2016, 12:47 p.m. UTC | #4
On Wed 22 Jun 2016 05:49:28 PM CEST, Kevin Wolf wrote:
>> I thought adding a new 'ID' field was simpler. The device name is
>> still a device name (where it makes sense). The default ID is
>> guaranteed to be valid and guaranteed not to clash with user-defined
>> IDs. The API is (in my opinion) more clear.
>> 
>> The only problems that I can think of:
>> 
>> - BlockJobInfo and the events expose the 'device' field which is
>>   superfluous.
>> - block-job-{pause,resume,...} can take an ID or a device name.
>
> Yes. There are two parts that I don't like about this.
>
> The first one is that we need additional code to keep track of the
> device name and to look it up.

I think this part is negligible, but ok.

> The other, more important one is that it couples block jobs more
> tightly with a BDS:
>
> * What do you with a background job that doesn't have a device name
>   because it doesn't work on a BDS? Will 'device' become optional
>   everywhere? But how is this less problematic for compatibility than
>   returning non-device-name IDs? (To be clear, I don't think either is
>   a real problem, but you can hardly dismiss one and accept the
>   other.)

> * And what do you do once we allow more than one job per device? Then
>   the device name isn't suitable for addressing the job any more. And
>   letting the client use the device name after it started the first
>   job, but not any more after it started the second one, feels wrong.

Fair enough. Unless Max, Eric or someone else has something else to add
I'll give it a try and see how it looks.

Berto
Max Reitz June 29, 2016, 5:20 p.m. UTC | #5
On 23.06.2016 14:47, Alberto Garcia wrote:
> On Wed 22 Jun 2016 05:49:28 PM CEST, Kevin Wolf wrote:
>>> I thought adding a new 'ID' field was simpler. The device name is
>>> still a device name (where it makes sense). The default ID is
>>> guaranteed to be valid and guaranteed not to clash with user-defined
>>> IDs. The API is (in my opinion) more clear.
>>>
>>> The only problems that I can think of:
>>>
>>> - BlockJobInfo and the events expose the 'device' field which is
>>>   superfluous.
>>> - block-job-{pause,resume,...} can take an ID or a device name.
>>
>> Yes. There are two parts that I don't like about this.
>>
>> The first one is that we need additional code to keep track of the
>> device name and to look it up.
> 
> I think this part is negligible, but ok.
> 
>> The other, more important one is that it couples block jobs more
>> tightly with a BDS:
>>
>> * What do you with a background job that doesn't have a device name
>>   because it doesn't work on a BDS? Will 'device' become optional
>>   everywhere? But how is this less problematic for compatibility than
>>   returning non-device-name IDs? (To be clear, I don't think either is
>>   a real problem, but you can hardly dismiss one and accept the
>>   other.)
> 
>> * And what do you do once we allow more than one job per device? Then
>>   the device name isn't suitable for addressing the job any more. And
>>   letting the client use the device name after it started the first
>>   job, but not any more after it started the second one, feels wrong.
> 
> Fair enough. Unless Max, Eric or someone else has something else to add
> I'll give it a try and see how it looks.

Sorry for the late response, but then again I don't actually have an
opinion either way.

The thing I feel most strongly about is the issue of storing a general
ID in a field named "device". However, as Kevin hinted at this becoming
irrelevant with John's work on decoupling block jobs from the block layer.

(And it probably will, because we don't want to call e.g.
block-job-pause that way then anymore, so John's probably going to add
newly named commands anyway, even if they do the same as the old ones. I
don't know.)

But this also means that I don't understand the first point of the
second part of what you quoted from Kevin here. I think he's referring
to what qemu returns e.g. in query-block-jobs. But why should
query-block-jobs return non-*block*-jobs? I think it should always omit
all background jobs that are not related to the block layer. Or at least
that would make sense.

The second point of the second part doesn't really strike with me
either. If a client uses the device name to identify a block job, they
should be used to a version of qemu that isn't capable of having more
than one job per device anyway, so they shouldn't launch more than a
single job per device to begin with.


So I don't think any of the pros and cons is a very strong point. I
personally feel like having a single ID field is more natural and making
the device name its default value is very intuitive, but I take issue
with its naming ("device"). So I don't care either way.

(Maybe, aside from John's work, we could get the naming issue out of the
way by introducing something like QMP aliases...?)

Max
Alberto Garcia June 30, 2016, 1:03 p.m. UTC | #6
On Wed 29 Jun 2016 07:20:55 PM CEST, Max Reitz wrote:
>>>> I thought adding a new 'ID' field was simpler. The device name is
>>>> still a device name (where it makes sense). The default ID is
>>>> guaranteed to be valid and guaranteed not to clash with
>>>> user-defined IDs. The API is (in my opinion) more clear.
>>>>
>>>> The only problems that I can think of:
>>>>
>>>> - BlockJobInfo and the events expose the 'device' field which is
>>>>   superfluous.
>>>> - block-job-{pause,resume,...} can take an ID or a device name.
>>>
>>> Yes. There are two parts that I don't like about this.
>>>
>>> The first one is that we need additional code to keep track of the
>>> device name and to look it up.
>> 
>> I think this part is negligible, but ok.
>> 
>>> The other, more important one is that it couples block jobs more
>>> tightly with a BDS:
>>>
>>> * What do you with a background job that doesn't have a device name
>>>   because it doesn't work on a BDS? Will 'device' become optional
>>>   everywhere? But how is this less problematic for compatibility than
>>>   returning non-device-name IDs? (To be clear, I don't think either is
>>>   a real problem, but you can hardly dismiss one and accept the
>>>   other.)
>> 
>>> * And what do you do once we allow more than one job per device? Then
>>>   the device name isn't suitable for addressing the job any more. And
>>>   letting the client use the device name after it started the first
>>>   job, but not any more after it started the second one, feels wrong.
>> 
>> Fair enough. Unless Max, Eric or someone else has something else to add
>> I'll give it a try and see how it looks.
>
> Sorry for the late response, but then again I don't actually have an
> opinion either way.
>
> The thing I feel most strongly about is the issue of storing a general
> ID in a field named "device". However, as Kevin hinted at this
> becoming irrelevant with John's work on decoupling block jobs from the
> block layer.

I actually forgot to Cc him, I'm doing it now.

The idea is that I don't want to add anything now that is going to cause
headaches later. Adding a new 'id' field to block jobs and keeping
'device' feels more natural to me, but reusing the 'device' field and
allowing any ID set by the user requires less changes both to the code
and the API.

Berto
John Snow July 1, 2016, 6:48 p.m. UTC | #7
On 06/30/2016 09:03 AM, Alberto Garcia wrote:
> On Wed 29 Jun 2016 07:20:55 PM CEST, Max Reitz wrote:
>>>>> I thought adding a new 'ID' field was simpler. The device name is
>>>>> still a device name (where it makes sense). The default ID is
>>>>> guaranteed to be valid and guaranteed not to clash with
>>>>> user-defined IDs. The API is (in my opinion) more clear.
>>>>>
>>>>> The only problems that I can think of:
>>>>>
>>>>> - BlockJobInfo and the events expose the 'device' field which is
>>>>>   superfluous.
>>>>> - block-job-{pause,resume,...} can take an ID or a device name.
>>>>
>>>> Yes. There are two parts that I don't like about this.
>>>>
>>>> The first one is that we need additional code to keep track of the
>>>> device name and to look it up.
>>>
>>> I think this part is negligible, but ok.
>>>
>>>> The other, more important one is that it couples block jobs more
>>>> tightly with a BDS:
>>>>
>>>> * What do you with a background job that doesn't have a device name
>>>>   because it doesn't work on a BDS? Will 'device' become optional
>>>>   everywhere? But how is this less problematic for compatibility than
>>>>   returning non-device-name IDs? (To be clear, I don't think either is
>>>>   a real problem, but you can hardly dismiss one and accept the
>>>>   other.)
>>>
>>>> * And what do you do once we allow more than one job per device? Then
>>>>   the device name isn't suitable for addressing the job any more. And
>>>>   letting the client use the device name after it started the first
>>>>   job, but not any more after it started the second one, feels wrong.
>>>
>>> Fair enough. Unless Max, Eric or someone else has something else to add
>>> I'll give it a try and see how it looks.
>>
>> Sorry for the late response, but then again I don't actually have an
>> opinion either way.
>>
>> The thing I feel most strongly about is the issue of storing a general
>> ID in a field named "device". However, as Kevin hinted at this
>> becoming irrelevant with John's work on decoupling block jobs from the
>> block layer.
> 
> I actually forgot to Cc him, I'm doing it now.
> 
> The idea is that I don't want to add anything now that is going to cause
> headaches later. Adding a new 'id' field to block jobs and keeping
> 'device' feels more natural to me, but reusing the 'device' field and
> allowing any ID set by the user requires less changes both to the code
> and the API.
> 
> Berto
> 

Reviewing everything now, sorry for being MIA, and thank you for keeping
me in the loop.

--js
diff mbox

Patch

diff --git a/block/mirror.c b/block/mirror.c
index a04ed9c..bcb1999 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -755,7 +755,8 @@  static void mirror_complete(BlockJob *job, Error **errp)
     target = blk_bs(s->target);
 
     if (!s->synced) {
-        error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+        error_setg(errp, "The active block job '%s' cannot be completed",
+                   job->id);
         return;
     }
 
diff --git a/blockjob.c b/blockjob.c
index 90c4e26..8bcb5ea 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -33,6 +33,7 @@ 
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
 #include "qemu/coroutine.h"
+#include "qemu/id.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
 #include "qapi-event.h"
@@ -125,7 +126,8 @@  void *block_job_create(const BlockJobDriver *driver, BlockDriverState *bs,
     bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 
     job->driver        = driver;
-    job->id            = g_strdup(bdrv_get_device_name(bs));
+    job->device        = g_strdup(bdrv_get_device_name(bs));
+    job->id            = id_generate(ID_JOB);
     job->blk           = blk;
     job->cb            = cb;
     job->opaque        = opaque;
@@ -168,6 +170,7 @@  void block_job_unref(BlockJob *job)
                                         block_job_detach_aio_context, job);
         blk_unref(job->blk);
         error_free(job->blocker);
+        g_free(job->device);
         g_free(job->id);
         QLIST_REMOVE(job, job_list);
         g_free(job);
@@ -289,7 +292,8 @@  void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)
 void block_job_complete(BlockJob *job, Error **errp)
 {
     if (job->pause_count || job->cancelled || !job->driver->complete) {
-        error_setg(errp, QERR_BLOCK_JOB_NOT_READY, job->id);
+        error_setg(errp, "The active block job '%s' cannot be completed",
+                   job->id);
         return;
     }
 
@@ -464,7 +468,7 @@  BlockJobInfo *block_job_query(BlockJob *job)
 {
     BlockJobInfo *info = g_new0(BlockJobInfo, 1);
     info->type      = g_strdup(BlockJobType_lookup[job->driver->job_type]);
-    info->device    = g_strdup(job->id);
+    info->device    = g_strdup(job->device);
     info->len       = job->len;
     info->busy      = job->busy;
     info->paused    = job->pause_count > 0;
@@ -486,7 +490,7 @@  static void block_job_iostatus_set_err(BlockJob *job, int error)
 void block_job_event_cancelled(BlockJob *job)
 {
     qapi_event_send_block_job_cancelled(job->driver->job_type,
-                                        job->id,
+                                        job->device,
                                         job->len,
                                         job->offset,
                                         job->speed,
@@ -496,7 +500,7 @@  void block_job_event_cancelled(BlockJob *job)
 void block_job_event_completed(BlockJob *job, const char *msg)
 {
     qapi_event_send_block_job_completed(job->driver->job_type,
-                                        job->id,
+                                        job->device,
                                         job->len,
                                         job->offset,
                                         job->speed,
@@ -510,7 +514,7 @@  void block_job_event_ready(BlockJob *job)
     job->ready = true;
 
     qapi_event_send_block_job_ready(job->driver->job_type,
-                                    job->id,
+                                    job->device,
                                     job->len,
                                     job->offset,
                                     job->speed, &error_abort);
@@ -538,7 +542,7 @@  BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
     default:
         abort();
     }
-    qapi_event_send_block_job_error(job->id,
+    qapi_event_send_block_job_error(job->device,
                                     is_read ? IO_OPERATION_TYPE_READ :
                                     IO_OPERATION_TYPE_WRITE,
                                     action, &error_abort);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 7dc720c..856486a 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -106,14 +106,18 @@  struct BlockJob {
     BlockBackend *blk;
 
     /**
-     * The ID of the block job. Currently the BlockBackend name of the BDS
-     * owning the job at the time when the job is started.
-     *
-     * TODO Decouple block job IDs from BlockBackend names
+     * The ID of the block job.
      */
     char *id;
 
     /**
+     * BlockBackend name of the BDS owning the job at the time when
+     * the job is started. For compatibility with clients that don't
+     * support the ID field.
+     */
+    char *device;
+
+    /**
      * The coroutine that executes the job.  If not NULL, it is
      * reentered when busy is false and the job is cancelled.
      */
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index d08652a..6586c9f 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -19,9 +19,6 @@ 
 #define QERR_BASE_NOT_FOUND \
     "Base '%s' not found"
 
-#define QERR_BLOCK_JOB_NOT_READY \
-    "The active block job for device '%s' cannot be completed"
-
 #define QERR_BUS_NO_HOTPLUG \
     "Bus '%s' does not support hotplugging"
 
diff --git a/include/qemu/id.h b/include/qemu/id.h
index 7d90335..c6c73ca 100644
--- a/include/qemu/id.h
+++ b/include/qemu/id.h
@@ -4,6 +4,7 @@ 
 typedef enum IdSubSystems {
     ID_QDEV,
     ID_BLOCK,
+    ID_JOB,
     ID_MAX      /* last element, used as array size */
 } IdSubSystems;
 
diff --git a/util/id.c b/util/id.c
index 6141352..eb5478b 100644
--- a/util/id.c
+++ b/util/id.c
@@ -34,6 +34,7 @@  bool id_wellformed(const char *id)
 static const char *const id_subsys_str[ID_MAX] = {
     [ID_QDEV]  = "qdev",
     [ID_BLOCK] = "block",
+    [ID_JOB]   = "job",
 };
 
 /*