[v5,03/15] block: add BlockJob interface for long-running operations

Submitted by Stefan Hajnoczi on Jan. 13, 2012, 1:14 p.m.

Details

Message ID 1326460457-19446-4-git-send-email-stefanha@linux.vnet.ibm.com
State New
Headers show

Commit Message

Stefan Hajnoczi Jan. 13, 2012, 1:14 p.m.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
---
 block.c     |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 block_int.h |   40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 0 deletions(-)

Comments

Kevin Wolf Jan. 17, 2012, 1 p.m.
Am 13.01.2012 14:14, schrieb Stefan Hajnoczi:
> Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
> ---
>  block.c     |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  block_int.h |   40 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 88 insertions(+), 0 deletions(-)
> 
> diff --git a/block.c b/block.c
> index daf92c2..d588ee8 100644
> --- a/block.c
> +++ b/block.c
> @@ -3877,3 +3877,51 @@ out:
>  
>      return ret;
>  }
> +
> +void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
> +                       BlockDriverCompletionFunc *cb, void *opaque)
> +{
> +    BlockJob *job;
> +
> +    if (bs->job || bdrv_in_use(bs)) {
> +        return NULL;
> +    }
> +    bdrv_set_in_use(bs, 1);
> +
> +    job = g_malloc0(job_type->instance_size);
> +    job->job_type      = job_type;
> +    job->bs            = bs;
> +    job->cb            = cb;
> +    job->opaque        = opaque;
> +    bs->job = job;
> +    return job;
> +}
> +
> +void block_job_complete(BlockJob *job, int ret)
> +{
> +    BlockDriverState *bs = job->bs;
> +
> +    assert(bs->job == job);
> +    job->cb(job->opaque, ret);
> +    bs->job = NULL;
> +    g_free(job);
> +    bdrv_set_in_use(bs, 0);
> +}
> +
> +int block_job_set_speed(BlockJob *job, int64_t value)
> +{
> +    if (!job->job_type->set_speed) {
> +        return -ENOTSUP;
> +    }
> +    return job->job_type->set_speed(job, value);
> +}
> +
> +void block_job_cancel(BlockJob *job)
> +{
> +    job->cancelled = true;
> +}
> +
> +bool block_job_is_cancelled(BlockJob *job)
> +{
> +    return job->cancelled;
> +}
> diff --git a/block_int.h b/block_int.h
> index 5362180..316443e 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -69,6 +69,36 @@ typedef struct BlockIOBaseValue {
>      uint64_t ios[2];
>  } BlockIOBaseValue;
>  
> +typedef void BlockJobCancelFunc(void *opaque);
> +typedef struct BlockJob BlockJob;
> +typedef struct BlockJobType {
> +    /** Derived BlockJob struct size */
> +    size_t instance_size;
> +
> +    /** String describing the operation, part of query-block-jobs QMP API */
> +    const char *job_type;
> +
> +    /** Optional callback for job types that support setting a speed limit */
> +    int (*set_speed)(BlockJob *job, int64_t value);

Would be worth mentioning what the unit of value is.

Kevin
Stefan Hajnoczi Jan. 17, 2012, 1:33 p.m.
On Tue, Jan 17, 2012 at 1:00 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 13.01.2012 14:14, schrieb Stefan Hajnoczi:
>> +typedef struct BlockJobType {
>> +    /** Derived BlockJob struct size */
>> +    size_t instance_size;
>> +
>> +    /** String describing the operation, part of query-block-jobs QMP API */
>> +    const char *job_type;
>> +
>> +    /** Optional callback for job types that support setting a speed limit */
>> +    int (*set_speed)(BlockJob *job, int64_t value);
>
> Would be worth mentioning what the unit of value is.

I left this open on purpose so future block jobs could support
block_job_set_speed with whatever unit makes sense for them.  At the
interface level it's an arbitrary int64_t.  Each block job type can
decide how to interpret the values.

I could add "The meaning of value and its units depend on the block
job type".  Or do you think it's problematic to allow different
meanings?

Stefan
Kevin Wolf Jan. 17, 2012, 1:44 p.m.
Am 17.01.2012 14:33, schrieb Stefan Hajnoczi:
> On Tue, Jan 17, 2012 at 1:00 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 13.01.2012 14:14, schrieb Stefan Hajnoczi:
>>> +typedef struct BlockJobType {
>>> +    /** Derived BlockJob struct size */
>>> +    size_t instance_size;
>>> +
>>> +    /** String describing the operation, part of query-block-jobs QMP API */
>>> +    const char *job_type;
>>> +
>>> +    /** Optional callback for job types that support setting a speed limit */
>>> +    int (*set_speed)(BlockJob *job, int64_t value);
>>
>> Would be worth mentioning what the unit of value is.
> 
> I left this open on purpose so future block jobs could support
> block_job_set_speed with whatever unit makes sense for them.  At the
> interface level it's an arbitrary int64_t.  Each block job type can
> decide how to interpret the values.

I see.

> I could add "The meaning of value and its units depend on the block
> job type".  Or do you think it's problematic to allow different
> meanings?

Might be confusing to have different meanings. But we can leave it open
for now and commit the comment as it is.

Kevin

Patch hide | download patch | download mbox

diff --git a/block.c b/block.c
index daf92c2..d588ee8 100644
--- a/block.c
+++ b/block.c
@@ -3877,3 +3877,51 @@  out:
 
     return ret;
 }
+
+void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
+                       BlockDriverCompletionFunc *cb, void *opaque)
+{
+    BlockJob *job;
+
+    if (bs->job || bdrv_in_use(bs)) {
+        return NULL;
+    }
+    bdrv_set_in_use(bs, 1);
+
+    job = g_malloc0(job_type->instance_size);
+    job->job_type      = job_type;
+    job->bs            = bs;
+    job->cb            = cb;
+    job->opaque        = opaque;
+    bs->job = job;
+    return job;
+}
+
+void block_job_complete(BlockJob *job, int ret)
+{
+    BlockDriverState *bs = job->bs;
+
+    assert(bs->job == job);
+    job->cb(job->opaque, ret);
+    bs->job = NULL;
+    g_free(job);
+    bdrv_set_in_use(bs, 0);
+}
+
+int block_job_set_speed(BlockJob *job, int64_t value)
+{
+    if (!job->job_type->set_speed) {
+        return -ENOTSUP;
+    }
+    return job->job_type->set_speed(job, value);
+}
+
+void block_job_cancel(BlockJob *job)
+{
+    job->cancelled = true;
+}
+
+bool block_job_is_cancelled(BlockJob *job)
+{
+    return job->cancelled;
+}
diff --git a/block_int.h b/block_int.h
index 5362180..316443e 100644
--- a/block_int.h
+++ b/block_int.h
@@ -69,6 +69,36 @@  typedef struct BlockIOBaseValue {
     uint64_t ios[2];
 } BlockIOBaseValue;
 
+typedef void BlockJobCancelFunc(void *opaque);
+typedef struct BlockJob BlockJob;
+typedef struct BlockJobType {
+    /** Derived BlockJob struct size */
+    size_t instance_size;
+
+    /** String describing the operation, part of query-block-jobs QMP API */
+    const char *job_type;
+
+    /** Optional callback for job types that support setting a speed limit */
+    int (*set_speed)(BlockJob *job, int64_t value);
+} BlockJobType;
+
+/**
+ * Long-running operation on a BlockDriverState
+ */
+struct BlockJob {
+    const BlockJobType *job_type;
+    BlockDriverState *bs;
+    bool cancelled;
+
+    /* These fields are published by the query-block-jobs QMP API */
+    int64_t offset;
+    int64_t len;
+    int64_t speed;
+
+    BlockDriverCompletionFunc *cb;
+    void *opaque;
+};
+
 struct BlockDriver {
     const char *format_name;
     int instance_size;
@@ -269,6 +299,9 @@  struct BlockDriverState {
     void *private;
 
     QLIST_HEAD(, BdrvTrackedRequest) tracked_requests;
+
+    /* long-running background operation */
+    BlockJob *job;
 };
 
 struct BlockDriverAIOCB {
@@ -292,4 +325,11 @@  void bdrv_set_io_limits(BlockDriverState *bs,
 int is_windows_drive(const char *filename);
 #endif
 
+void *block_job_create(const BlockJobType *job_type, BlockDriverState *bs,
+                       BlockDriverCompletionFunc *cb, void *opaque);
+void block_job_complete(BlockJob *job, int ret);
+int block_job_set_speed(BlockJob *job, int64_t value);
+void block_job_cancel(BlockJob *job);
+bool block_job_is_cancelled(BlockJob *job);
+
 #endif /* BLOCK_INT_H */