Re: [Qemu-devel] [PATCH v5 03/15] block: add BlockJob interface for long-running operations

2012-01-17 Thread Kevin Wolf 
Am 13.01.2012 14:14, schrieb Stefan Hajnoczi:
 Signed-off-by: Stefan Hajnoczi stefa...@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

Re: [Qemu-devel] [PATCH v5 03/15] block: add BlockJob interface for long-running operations

2012-01-17 Thread Stefan Hajnoczi 
On Tue, Jan 17, 2012 at 1:00 PM, Kevin Wolf kw...@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

Re: [Qemu-devel] [PATCH v5 03/15] block: add BlockJob interface for long-running operations

2012-01-17 Thread Kevin Wolf 
Am 17.01.2012 14:33, schrieb Stefan Hajnoczi:
 On Tue, Jan 17, 2012 at 1:00 PM, Kevin Wolf kw...@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

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

2012-01-13 Thread Stefan Hajnoczi 
Signed-off-by: Stefan Hajnoczi stefa...@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);
+} 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 */
-- 
1.7.7.3