qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 2/6] blockjob: Introduce block_job_complete_s


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v4 2/6] blockjob: Introduce block_job_complete_sync()
Date: Wed, 16 Apr 2014 23:43:25 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0

On 16.04.2014 03:48, Fam Zheng wrote:
On Sat, 04/12 20:57, Max Reitz wrote:
Implement block_job_complete_sync() by doing the exact same thing as
block_job_cancel_sync() does, only with calling block_job_complete()
instead of block_job_cancel().

Signed-off-by: Max Reitz <address@hidden>
---
  blockjob.c               | 24 ++++++++++++++++++++++--
  include/block/blockjob.h | 15 +++++++++++++++
  2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index b3ce14c..d12f3ea 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -165,7 +165,9 @@ static void block_job_cancel_cb(void *opaque, int ret)
      data->cb(data->opaque, ret);
  }
-int block_job_cancel_sync(BlockJob *job)
+static int block_job_finish_sync(BlockJob *job,
+                                 void (*finish)(BlockJob *, Error **errp),
+                                 Error **errp)
  {
      struct BlockCancelData data;
      BlockDriverState *bs = job->bs;
@@ -181,13 +183,31 @@ int block_job_cancel_sync(BlockJob *job)
      data.ret = -EINPROGRESS;
      job->cb = block_job_cancel_cb;
      job->opaque = &data;
-    block_job_cancel(job);
+    finish(job, errp);
      while (data.ret == -EINPROGRESS) {
          qemu_aio_wait();
      }
      return (data.cancelled && data.ret == 0) ? -ECANCELED : data.ret;
  }
+/* A wrapper around block_job_cancel() taking an Error ** parameter so it may be
+ * used with block_job_finish_sync() without the need for (rather nasty)
+ * function pointer casts there. */
Why not just change the prototype of block_job_cancel to have this error
pointer?

Well, there are external callers of block_job_cancel() which I would have to change and I don't really like calling a function with NULL for the Error ** parameter if it has one; even though this version of block_job_cancel wouldn't actually make use of that parameter.

So, what I'd end up doing is implementing error handling for block_job_cancel() in these external callers without any actual need for it, which seems to me like more effort than implementing this wrapper. Furthermore, I personally like having this wrapper directly above block_job_cancel_sync() as it explains nicely why the latter may give NULL for the Error ** parameter of block_job_finish_sync().

I'll change it, though, if you insist. ;-)

Max

Thanks,
Fam

+static void block_job_cancel_err(BlockJob *job, Error **errp)
+{
+    block_job_cancel(job);
+}
+
+int block_job_cancel_sync(BlockJob *job)
+{
+    return block_job_finish_sync(job, &block_job_cancel_err, NULL);
+}
+
+int block_job_complete_sync(BlockJob *job, Error **errp)
+{
+    return block_job_finish_sync(job, &block_job_complete, errp);
+}
+
  void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns)
  {
      assert(job->busy);
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d76de62..626ea42 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -253,6 +253,21 @@ bool block_job_is_paused(BlockJob *job);
  int block_job_cancel_sync(BlockJob *job);
/**
+ * block_job_complete_sync:
+ * @job: The job to be completed.
+ * @errp: Error object which may be set by block_job_complete(); this is not
+ *        necessarily set on every error, the job return value has to be
+ *        checked as well.
+ *
+ * Synchronously complete the job.  The completion callback is called before 
the
+ * function returns, unless it is NULL (which is permissible when using this
+ * function).
+ *
+ * Returns the return value from the job.
+ */
+int block_job_complete_sync(BlockJob *job, Error **errp);
+
+/**
   * block_job_iostatus_reset:
   * @job: The job whose I/O status should be reset.
   *
--
1.9.2





reply via email to

[Prev in Thread] Current Thread [Next in Thread]