qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RFC v2 3/6] replication: Split out backup_do_che


From: Changlong Xie
Subject: Re: [Qemu-devel] [PATCH RFC v2 3/6] replication: Split out backup_do_checkpoint() from secondary_do_checkpoint()
Date: Tue, 20 Dec 2016 20:41:11 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0

On 12/05/2016 04:35 PM, zhanghailiang wrote:
The helper backup_do_checkpoint() will be used for primary related
codes. Here we split it out from secondary_do_checkpoint().

Besides, it is unnecessary to call backup_do_checkpoint() in
replication starting and normally stop replication path.
We only need call it while do real checkpointing.

Signed-off-by: zhanghailiang <address@hidden>
---
  block/replication.c | 36 +++++++++++++++++++-----------------
  1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/block/replication.c b/block/replication.c
index e87ae87..35e9ab3 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -332,20 +332,8 @@ static bool 
replication_recurse_is_first_non_filter(BlockDriverState *bs,

  static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
  {
-    Error *local_err = NULL;
      int ret;

-    if (!s->secondary_disk->bs->job) {
-        error_setg(errp, "Backup job was cancelled unexpectedly");
-        return;
-    }
-
-    backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
-
      ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs);
      if (ret < 0) {
          error_setg(errp, "Cannot make active disk empty");
@@ -558,6 +546,8 @@ static void replication_start(ReplicationState *rs, 
ReplicationMode mode,
              return;
          }
          block_job_start(job);
+
+        secondary_do_checkpoint(s, errp);
          break;
      default:
          aio_context_release(aio_context);
@@ -566,10 +556,6 @@ static void replication_start(ReplicationState *rs, 
ReplicationMode mode,

      s->replication_state = BLOCK_REPLICATION_RUNNING;

-    if (s->mode == REPLICATION_MODE_SECONDARY) {
-        secondary_do_checkpoint(s, errp);
-    }
-
      s->error = 0;
      aio_context_release(aio_context);
  }
@@ -579,13 +565,29 @@ static void replication_do_checkpoint(ReplicationState 
*rs, Error **errp)
      BlockDriverState *bs = rs->opaque;
      BDRVReplicationState *s;
      AioContext *aio_context;
+    Error *local_err = NULL;

      aio_context = bdrv_get_aio_context(bs);
      aio_context_acquire(aio_context);
      s = bs->opaque;

-    if (s->mode == REPLICATION_MODE_SECONDARY) {
+    switch (s->mode) {
+    case REPLICATION_MODE_PRIMARY:
+        break;
+    case REPLICATION_MODE_SECONDARY:
+        if (!s->secondary_disk->bs->job) {
+            error_setg(errp, "Backup job was cancelled unexpectedly");
+            break;
+        }
+        backup_do_checkpoint(s->secondary_disk->bs->job, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            break;
+        }
          secondary_do_checkpoint(s, errp);
+        break;
+    default:
+        abort();
      }
      aio_context_release(aio_context);
  }


Looks good to me

Reviewed-by: Changlong Xie <address@hidden>





reply via email to

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