qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/4] colo: Add support for continuous replication
@ 2019-10-05 13:05 Lukas Straub
  2019-10-05 13:05 ` [PATCH v6 1/4] block/replication.c: Ignore requests after failover Lukas Straub
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Lukas Straub @ 2019-10-05 13:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Zhang Chen, Xie Changlong

Hello Everyone,
These Patches add support for continuous replication to colo. This means
that after the Primary fails and the Secondary did a failover, the Secondary
can then become Primary and resume replication to a new Secondary.

Regards,
Lukas Straub

v6:
 - properly documented the position= and insert= options
 - renamed replication test
 - clarified documentation by using different ip's for primary and secondary
 - added Reviewed-by tags

v5:
 - change syntax for the position= parameter
 - fix spelling mistake

v4:
 - fix checkpatch.pl warnings

v3:
 - add test for replication changes
 - check if the filter to be inserted before/behind belongs to the same interface
 - fix the error message for the position= parameter
 - rename term "after" -> "behind" and variable "insert_before" -> "insert_before_flag"
 - document the quorum node on the secondary side
 - simplify quorum parameters in documentation
 - remove trailing spaces in documentation
 - clarify the testing procedure in documentation

v2:
 - fix email formating
 - fix checkpatch.pl warnings
 - fix patchew error
 - clearer commit messages


Lukas Straub (4):
  block/replication.c: Ignore requests after failover
  tests/test-replication.c: Add test for for secondary node continuing
    replication
  net/filter.c: Add Options to insert filters anywhere in the filter
    list
  colo: Update Documentation for continuous replication

 block/replication.c        |  38 ++++++-
 docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++----------
 docs/block-replication.txt |  28 +++--
 include/net/filter.h       |   2 +
 net/filter.c               |  92 +++++++++++++++-
 qemu-options.hx            |  31 +++++-
 tests/test-replication.c   |  52 +++++++++
 7 files changed, 380 insertions(+), 76 deletions(-)

--
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/4] block/replication.c: Ignore requests after failover
  2019-10-05 13:05 [PATCH v6 0/4] colo: Add support for continuous replication Lukas Straub
@ 2019-10-05 13:05 ` Lukas Straub
  2019-10-18 18:46   ` Lukas Straub
  2019-10-23 12:49   ` Max Reitz
  2019-10-05 13:05 ` [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication Lukas Straub
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 16+ messages in thread
From: Lukas Straub @ 2019-10-05 13:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Zhang Chen, Xie Changlong

After failover the Secondary side of replication shouldn't change state, because
it now functions as our primary disk.

In replication_start, replication_do_checkpoint, replication_stop, ignore
the request if current state is BLOCK_REPLICATION_DONE (sucessful failover) or
BLOCK_REPLICATION_FAILOVER (failover in progres i.e. currently merging active
and hidden images into the base image).

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
---
 block/replication.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/block/replication.c b/block/replication.c
index 3d4dedddfc..97cc65c0cf 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -454,6 +454,17 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
     aio_context_acquire(aio_context);
     s = bs->opaque;

+    if (s->stage == BLOCK_REPLICATION_DONE ||
+        s->stage == BLOCK_REPLICATION_FAILOVER) {
+        /*
+         * This case happens when a secondary is promoted to primary.
+         * Ignore the request because the secondary side of replication
+         * doesn't have to do anything anymore.
+         */
+        aio_context_release(aio_context);
+        return;
+    }
+
     if (s->stage != BLOCK_REPLICATION_NONE) {
         error_setg(errp, "Block replication is running or done");
         aio_context_release(aio_context);
@@ -529,8 +540,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
                    "Block device is in use by internal backup job");

         top_bs = bdrv_lookup_bs(s->top_id, s->top_id, NULL);
-        if (!top_bs || !bdrv_is_root_node(top_bs) ||
-            !check_top_bs(top_bs, bs)) {
+        if (!top_bs || !check_top_bs(top_bs, bs)) {
             error_setg(errp, "No top_bs or it is invalid");
             reopen_backing_file(bs, false, NULL);
             aio_context_release(aio_context);
@@ -577,6 +587,17 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
     aio_context_acquire(aio_context);
     s = bs->opaque;

+    if (s->stage == BLOCK_REPLICATION_DONE ||
+        s->stage == BLOCK_REPLICATION_FAILOVER) {
+        /*
+         * This case happens when a secondary was promoted to primary.
+         * Ignore the request because the secondary side of replication
+         * doesn't have to do anything anymore.
+         */
+        aio_context_release(aio_context);
+        return;
+    }
+
     if (s->mode == REPLICATION_MODE_SECONDARY) {
         secondary_do_checkpoint(s, errp);
     }
@@ -593,7 +614,7 @@ static void replication_get_error(ReplicationState *rs, Error **errp)
     aio_context_acquire(aio_context);
     s = bs->opaque;

-    if (s->stage != BLOCK_REPLICATION_RUNNING) {
+    if (s->stage == BLOCK_REPLICATION_NONE) {
         error_setg(errp, "Block replication is not running");
         aio_context_release(aio_context);
         return;
@@ -635,6 +656,17 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
     aio_context_acquire(aio_context);
     s = bs->opaque;

+    if (s->stage == BLOCK_REPLICATION_DONE ||
+        s->stage == BLOCK_REPLICATION_FAILOVER) {
+        /*
+         * This case happens when a secondary was promoted to primary.
+         * Ignore the request because the secondary side of replication
+         * doesn't have to do anything anymore.
+         */
+        aio_context_release(aio_context);
+        return;
+    }
+
     if (s->stage != BLOCK_REPLICATION_RUNNING) {
         error_setg(errp, "Block replication is not running");
         aio_context_release(aio_context);
--
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication
  2019-10-05 13:05 [PATCH v6 0/4] colo: Add support for continuous replication Lukas Straub
  2019-10-05 13:05 ` [PATCH v6 1/4] block/replication.c: Ignore requests after failover Lukas Straub
@ 2019-10-05 13:05 ` Lukas Straub
  2019-10-09  6:03   ` Zhang, Chen
  2019-10-05 13:05 ` [PATCH v6 3/4] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
  2019-10-05 13:05 ` [PATCH v6 4/4] colo: Update Documentation for continuous replication Lukas Straub
  3 siblings, 1 reply; 16+ messages in thread
From: Lukas Straub @ 2019-10-05 13:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Zhang Chen, Xie Changlong

This simulates the case that happens when we resume COLO after failover.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 tests/test-replication.c | 52 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tests/test-replication.c b/tests/test-replication.c
index f085d1993a..8e18ecd998 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -489,6 +489,56 @@ static void test_secondary_stop(void)
     teardown_secondary();
 }

+static void test_secondary_continuous_replication(void)
+{
+    BlockBackend *top_blk, *local_blk;
+    Error *local_err = NULL;
+
+    top_blk = start_secondary();
+    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
+    g_assert(!local_err);
+
+    /* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
+    local_blk = blk_by_name(S_LOCAL_DISK_ID);
+    test_blk_write(local_blk, 0x22, IMG_SIZE / 2, IMG_SIZE / 2, false);
+
+    /* replication will backup s_local_disk to s_hidden_disk */
+    test_blk_read(top_blk, 0x11, IMG_SIZE / 2,
+                  IMG_SIZE / 2, 0, IMG_SIZE, false);
+
+    /* write 0x33 to s_active_disk (0, IMG_SIZE / 2) */
+    test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
+
+    /* do failover (active commit) */
+    replication_stop_all(true, &local_err);
+    g_assert(!local_err);
+
+    /* it should ignore all requests from now on */
+
+    /* start after failover */
+    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
+    g_assert(!local_err);
+
+    /* checkpoint */
+    replication_do_checkpoint_all(&local_err);
+    g_assert(!local_err);
+
+    /* stop */
+    replication_stop_all(true, &local_err);
+    g_assert(!local_err);
+
+    /* read from s_local_disk (0, IMG_SIZE / 2) */
+    test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
+                  0, IMG_SIZE / 2, false);
+
+
+    /* read from s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
+    test_blk_read(top_blk, 0x22, IMG_SIZE / 2,
+                  IMG_SIZE / 2, 0, IMG_SIZE, false);
+
+    teardown_secondary();
+}
+
 static void test_secondary_do_checkpoint(void)
 {
     BlockBackend *top_blk, *local_blk;
@@ -584,6 +634,8 @@ int main(int argc, char **argv)
     g_test_add_func("/replication/secondary/write", test_secondary_write);
     g_test_add_func("/replication/secondary/start", test_secondary_start);
     g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
+    g_test_add_func("/replication/secondary/continuous_replication",
+                    test_secondary_continuous_replication);
     g_test_add_func("/replication/secondary/do_checkpoint",
                     test_secondary_do_checkpoint);
     g_test_add_func("/replication/secondary/get_error_all",
--
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 3/4] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-10-05 13:05 [PATCH v6 0/4] colo: Add support for continuous replication Lukas Straub
  2019-10-05 13:05 ` [PATCH v6 1/4] block/replication.c: Ignore requests after failover Lukas Straub
  2019-10-05 13:05 ` [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication Lukas Straub
@ 2019-10-05 13:05 ` Lukas Straub
  2019-10-05 13:05 ` [PATCH v6 4/4] colo: Update Documentation for continuous replication Lukas Straub
  3 siblings, 0 replies; 16+ messages in thread
From: Lukas Straub @ 2019-10-05 13:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Zhang Chen, Xie Changlong

To switch the Secondary to Primary, we need to insert new filters
before the filter-rewriter.

Add the options insert= and position= to be able to insert filters
anywhere in the filter list.

position should be "head" or "tail" to insert at the head or
tail of the filter list or it should be "id=<id>" to specify
the id of another filter.
insert should be either "before" or "behind" to specify where to
insert the new filter relative to the one specified with position.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
---
 include/net/filter.h |  2 +
 net/filter.c         | 92 +++++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx      | 31 ++++++++++++---
 3 files changed, 119 insertions(+), 6 deletions(-)

diff --git a/include/net/filter.h b/include/net/filter.h
index 49da666ac0..22a723305b 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -62,6 +62,8 @@ struct NetFilterState {
     NetClientState *netdev;
     NetFilterDirection direction;
     bool on;
+    char *position;
+    bool insert_before_flag;
     QTAILQ_ENTRY(NetFilterState) next;
 };

diff --git a/net/filter.c b/net/filter.c
index 28d1930db7..cd2ef9e979 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -171,11 +171,47 @@ static void netfilter_set_status(Object *obj, const char *str, Error **errp)
     }
 }

+static char *netfilter_get_position(Object *obj, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    return g_strdup(nf->position);
+}
+
+static void netfilter_set_position(Object *obj, const char *str, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    nf->position = g_strdup(str);
+}
+
+static char *netfilter_get_insert(Object *obj, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    return nf->insert_before_flag ? g_strdup("before") : g_strdup("behind");
+}
+
+static void netfilter_set_insert(Object *obj, const char *str, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    if (strcmp(str, "before") && strcmp(str, "behind")) {
+        error_setg(errp, "Invalid value for netfilter insert, "
+                         "should be 'before' or 'behind'");
+        return;
+    }
+
+    nf->insert_before_flag = !strcmp(str, "before");
+}
+
 static void netfilter_init(Object *obj)
 {
     NetFilterState *nf = NETFILTER(obj);

     nf->on = true;
+    nf->insert_before_flag = false;
+    nf->position = g_strdup("tail");

     object_property_add_str(obj, "netdev",
                             netfilter_get_netdev_id, netfilter_set_netdev_id,
@@ -187,11 +223,18 @@ static void netfilter_init(Object *obj)
     object_property_add_str(obj, "status",
                             netfilter_get_status, netfilter_set_status,
                             NULL);
+    object_property_add_str(obj, "position",
+                            netfilter_get_position, netfilter_set_position,
+                            NULL);
+    object_property_add_str(obj, "insert",
+                            netfilter_get_insert, netfilter_set_insert,
+                            NULL);
 }

 static void netfilter_complete(UserCreatable *uc, Error **errp)
 {
     NetFilterState *nf = NETFILTER(uc);
+    NetFilterState *position = NULL;
     NetClientState *ncs[MAX_QUEUE_NUM];
     NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
     int queues;
@@ -219,6 +262,41 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
         return;
     }

+    if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) {
+        Object *container;
+        Object *obj;
+        char *position_id;
+
+        if (!g_str_has_prefix(nf->position, "id=")) {
+            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "position",
+                       "'head', 'tail' or 'id=<id>'");
+            return;
+        }
+
+        /* get the id from the string */
+        position_id = g_strndup(nf->position + 3, strlen(nf->position) - 3);
+
+        /* Search for the position to insert before/behind */
+        container = object_get_objects_root();
+        obj = object_resolve_path_component(container, position_id);
+        if (!obj) {
+            error_setg(errp, "filter '%s' not found", position_id);
+            g_free(position_id);
+            return;
+        }
+
+        position = NETFILTER(obj);
+
+        if (position->netdev != ncs[0]) {
+            error_setg(errp, "filter '%s' belongs to a different netdev",
+                        position_id);
+            g_free(position_id);
+            return;
+        }
+
+        g_free(position_id);
+    }
+
     nf->netdev = ncs[0];

     if (nfc->setup) {
@@ -228,7 +306,18 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
             return;
         }
     }
-    QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
+
+    if (position) {
+        if (nf->insert_before_flag) {
+            QTAILQ_INSERT_BEFORE(position, nf, next);
+        } else {
+            QTAILQ_INSERT_AFTER(&nf->netdev->filters, position, nf, next);
+        }
+    } else if (!strcmp(nf->position, "head")) {
+        QTAILQ_INSERT_HEAD(&nf->netdev->filters, nf, next);
+    } else if (!strcmp(nf->position, "tail")) {
+        QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
+    }
 }

 static void netfilter_finalize(Object *obj)
@@ -245,6 +334,7 @@ static void netfilter_finalize(Object *obj)
         QTAILQ_REMOVE(&nf->netdev->filters, nf, next);
     }
     g_free(nf->netdev_id);
+    g_free(nf->position);
 }

 static void default_handle_event(NetFilterState *nf, int event, Error **errp)
diff --git a/qemu-options.hx b/qemu-options.hx
index 08749a3391..d2a6cb7da1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4368,7 +4368,7 @@ applications, they can do this through this parameter. Its format is
 a gnutls priority string as described at
 @url{https://gnutls.org/manual/html_node/Priority-Strings.html}.

-@item -object filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|rx|tx}][,status=@var{on|off}]
+@item -object filter-buffer,id=@var{id},netdev=@var{netdevid},interval=@var{t}[,queue=@var{all|rx|tx}][,status=@var{on|off}][,position=@var{head|tail|id=<id>}][,insert=@var{behind|before}]

 Interval @var{t} can't be 0, this filter batches the packet delivery: all
 packets arriving in a given interval on netdev @var{netdevid} are delayed
@@ -4387,11 +4387,32 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter.
 @option{tx}: the filter is attached to the transmit queue of the netdev,
              where it will receive packets sent by the netdev.

-@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
+position @var{head|tail|id=<id>} is an option to specify where the
+filter should be inserted in the filter list. It can be applied to any
+netfilter.
+
+@option{head}: the filter is inserted at the head of the filter
+               list, before any existing filters.
+
+@option{tail}: the filter is inserted at the tail of the filter
+               list, behind any existing filters (default).
+
+@option{id=<id>}: the filter is inserted before or behind the filter
+                  specified by <id>, see the insert option below.
+
+insert @var{behind|before} is an option to specify where to insert the
+new filter relative to the one specified with position=id=<id>. It can
+be applied to any netfilter.
+
+@option{before}: insert before the specified filter.
+
+@option{behind}: insert behind the specified filter (default).
+
+@item -object filter-mirror,id=@var{id},netdev=@var{netdevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|id=<id>}][,insert=@var{behind|before}]

 filter-mirror on netdev @var{netdevid},mirror net packet to chardev@var{chardevid}, if it has the vnet_hdr_support flag, filter-mirror will mirror packet with vnet_hdr_len.

-@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support]
+@item -object filter-redirector,id=@var{id},netdev=@var{netdevid},indev=@var{chardevid},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,position=@var{head|tail|id=<id>}][,insert=@var{behind|before}]

 filter-redirector on netdev @var{netdevid},redirect filter's net packet to chardev
 @var{chardevid},and redirect indev's packet to filter.if it has the vnet_hdr_support flag,
@@ -4400,7 +4421,7 @@ Create a filter-redirector we need to differ outdev id from indev id, id can not
 be the same. we can just use indev or outdev, but at least one of indev or outdev
 need to be specified.

-@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vnet_hdr_support]
+@item -object filter-rewriter,id=@var{id},netdev=@var{netdevid},queue=@var{all|rx|tx},[vnet_hdr_support][,position=@var{head|tail|id=<id>}][,insert=@var{behind|before}]

 Filter-rewriter is a part of COLO project.It will rewrite tcp packet to
 secondary from primary to keep secondary tcp connection,and rewrite
@@ -4413,7 +4434,7 @@ colo secondary:
 -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
 -object filter-rewriter,id=rew0,netdev=hn0,queue=all

-@item -object filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}]
+@item -object filter-dump,id=@var{id},netdev=@var{dev}[,file=@var{filename}][,maxlen=@var{len}][,position=@var{head|tail|id=<id>}][,insert=@var{behind|before}]

 Dump the network traffic on netdev @var{dev} to the file specified by
 @var{filename}. At most @var{len} bytes (64k by default) per packet are stored.
--
2.20.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 4/4] colo: Update Documentation for continuous replication
  2019-10-05 13:05 [PATCH v6 0/4] colo: Add support for continuous replication Lukas Straub
                   ` (2 preceding siblings ...)
  2019-10-05 13:05 ` [PATCH v6 3/4] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
@ 2019-10-05 13:05 ` Lukas Straub
  2019-10-09  8:36   ` Zhang, Chen
  3 siblings, 1 reply; 16+ messages in thread
From: Lukas Straub @ 2019-10-05 13:05 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Zhang Chen, Xie Changlong

Document the qemu command-line and qmp commands for continuous replication

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++----------
 docs/block-replication.txt |  28 +++--
 2 files changed, 174 insertions(+), 67 deletions(-)

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index ad24680d13..bc1a0ccb99 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -145,35 +145,65 @@ The diagram just shows the main qmp command, you can get the detail
 in test procedure.
 
 == Test procedure ==
-1. Startup qemu
-Primary:
-# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name primary \
-  -device piix3-usb-uhci -vnc :7 \
-  -device usb-tablet -netdev tap,id=hn0,vhost=off \
-  -device virtio-net-pci,id=net-pci0,netdev=hn0 \
-  -drive if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
-         children.0.file.filename=1.raw,\
-         children.0.driver=raw -S
-Secondary:
-# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name secondary \
-  -device piix3-usb-uhci -vnc :7 \
-  -device usb-tablet -netdev tap,id=hn0,vhost=off \
-  -device virtio-net-pci,id=net-pci0,netdev=hn0 \
-  -drive if=none,id=secondary-disk0,file.filename=1.raw,driver=raw,node-name=node0 \
-  -drive if=virtio,id=active-disk0,driver=replication,mode=secondary,\
-         file.driver=qcow2,top-id=active-disk0,\
-         file.file.filename=/mnt/ramfs/active_disk.img,\
-         file.backing.driver=qcow2,\
-         file.backing.file.filename=/mnt/ramfs/hidden_disk.img,\
-         file.backing.backing=secondary-disk0 \
-  -incoming tcp:0:8888
-
-2. On Secondary VM's QEMU monitor, issue command
+Note: Here we are running both instances on the same Host for testing,
+change the IP Addresses if you want to run it on two Hosts. Initally
+127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
+
+== Startup qemu ==
+1. Primary:
+Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all Hosts.
+# imagefolder="/mnt/vms/colo-test-primary"
+
+# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp 1 -qmp stdio \
+   -device piix3-usb-uhci -device usb-tablet -name primary \
+   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper \
+   -device rtl8139,id=e0,netdev=hn0 \
+   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
+   -chardev socket,id=compare1,host=0.0.0.0,port=9004,server,wait \
+   -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait \
+   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
+   -chardev socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait \
+   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
+   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
+   -object filter-redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
+   -object filter-redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
+   -object iothread,id=iothread1 \
+   -object colo-compare,id=comp0,primary_in=compare0-0,secondary_in=compare1,\
+outdev=compare_out0,iothread=iothread1 \
+   -drive if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
+children.0.file.filename=$imagefolder/primary.qcow2,children.0.driver=qcow2 -S
+
+2. Secondary:
+# imagefolder="/mnt/vms/colo-test-secondary"
+# primary_ip=127.0.0.1
+
+# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G
+
+# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2 10G
+
+# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp 1 -qmp stdio \
+   -device piix3-usb-uhci -device usb-tablet -name secondary \
+   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper \
+   -device rtl8139,id=e0,netdev=hn0 \
+   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \
+   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \
+   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
+   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
+   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
+   -drive if=none,id=parent0,file.filename=$imagefolder/primary.qcow2,driver=qcow2 \
+   -drive if=none,id=childs0,driver=replication,mode=secondary,file.driver=qcow2,\
+top-id=childs0,file.file.filename=$imagefolder/secondary-active.qcow2,\
+file.backing.driver=qcow2,file.backing.file.filename=$imagefolder/secondary-hidden.qcow2,\
+file.backing.backing=parent0 \
+   -drive if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
+children.0=childs0 \
+   -incoming tcp:0.0.0.0:9998
+
+
+3. On Secondary VM's QEMU monitor, issue command
 {'execute':'qmp_capabilities'}
-{ 'execute': 'nbd-server-start',
-  'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx', 'port': '8889'} } }
-}
-{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0', 'writable': true } }
+{'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet', 'data': {'host': '0.0.0.0', 'port': '9999'} } } }
+{'execute': 'nbd-server-add', 'arguments': {'device': 'parent0', 'writable': true } }
 
 Note:
   a. The qmp command nbd-server-start and nbd-server-add must be run
@@ -182,44 +212,113 @@ Note:
      same.
   c. It is better to put active disk and hidden disk in ramdisk.
 
-3. On Primary VM's QEMU monitor, issue command:
+4. On Primary VM's QEMU monitor, issue command:
 {'execute':'qmp_capabilities'}
-{ 'execute': 'human-monitor-command',
-  'arguments': {'command-line': 'drive_add -n buddy driver=replication,mode=primary,file.driver=nbd,file.host=xx.xx.xx.xx,file.port=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
-{ 'execute':'x-blockdev-change', 'arguments':{'parent': 'primary-disk0', 'node': 'nbd_client0' } }
-{ 'execute': 'migrate-set-capabilities',
-      'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
-{ 'execute': 'migrate', 'arguments': {'uri': 'tcp:xx.xx.xx.xx:8888' } }
+{'execute': 'human-monitor-command', 'arguments': {'command-line': 'drive_add -n buddy driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,file.port=9999,file.export=parent0,node-name=replication0'}}
+{'execute': 'x-blockdev-change', 'arguments':{'parent': 'colo-disk0', 'node': 'replication0' } }
+{'execute': 'migrate-set-capabilities', 'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
+{'execute': 'migrate', 'arguments': {'uri': 'tcp:127.0.0.2:9998' } }
 
   Note:
   a. There should be only one NBD Client for each primary disk.
-  b. xx.xx.xx.xx is the secondary physical machine's hostname or IP
-  c. The qmp command line must be run after running qmp command line in
+  b. The qmp command line must be run after running qmp command line in
      secondary qemu.
 
-4. After the above steps, you will see, whenever you make changes to PVM, SVM will be synced.
+5. After the above steps, you will see, whenever you make changes to PVM, SVM will be synced.
 You can issue command '{ "execute": "migrate-set-parameters" , "arguments":{ "x-checkpoint-delay": 2000 } }'
-to change the checkpoint period time
+to change the idle checkpoint period time
+
+6. Failover test
+You can kill one of the VMs and Failover on the surviving VM:
+
+If you killed the Secondary, then follow "Primary Failover". After that,
+if you want to resume the replication, follow "Primary resume replication"
+
+If you killed the Primary, then follow "Secondary Failover". After that,
+if you want to resume the replication, follow "Secondary resume replication"
+
+== Primary Failover ==
+The Secondary died, resume on the Primary
+
+{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0', 'child': 'children.1'} }
+{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 'drive_del replication0' } }
+{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
+{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
+{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
+{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
+{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
+{'execute': 'x-colo-lost-heartbeat' }
+
+== Secondary Failover ==
+The Primary died, resume on the Secondary and prepare to become the new Primary
+
+{'execute': 'nbd-server-stop'}
+{'execute': 'x-colo-lost-heartbeat'}
+
+{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
+{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
+{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
+{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
+
+{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '0.0.0.0', 'port': '9003' } }, 'server': true } } } }
+{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '0.0.0.0', 'port': '9004' } }, 'server': true } } } }
+{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '127.0.0.1', 'port': '9001' } }, 'server': true } } } }
+{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0-0', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '127.0.0.1', 'port': '9001' } }, 'server': false } } } }
+{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': true } } } }
+{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out0', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': false } } } }
+
+== Primary resume replication ==
+Resume replication after new Secondary is up.
+
+Start the new Secondary (Steps 2 and 3 above), then on the Primary:
+{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0', 'job-id': 'resync', 'target': 'nbd://127.0.0.2:9999/parent0', 'mode': 'existing', 'format': 'raw', 'sync': 'full'} }
+
+Wait until disk is synced, then:
+{'execute': 'stop'}
+{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync'} }
+
+{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 'drive_add -n buddy driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,file.port=9999,file.export=parent0,node-name=replication0'}}
+{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0', 'node': 'replication0' } }
+
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id': 'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id': 'redire0', 'props': { 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id': 'redire1', 'props': { 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id': 'iothread1' } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare', 'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in': 'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
+
+{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
+{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.2:9998' } }
+
+Note:
+If this Primary previously was a Secondary, then we need to insert the
+filters before the filter-rewriter by using the
+"'insert': 'before', 'position': 'id=rew0'" Options. See below.
+
+== Secondary resume replication ==
+Become Primary and resume replication after new Secondary is up. Note
+that now 127.0.0.1 is the Secondary and 127.0.0.2 is the Primary.
+
+Start the new Secondary (Steps 2 and 3 above, but with primary_ip=127.0.0.2),
+then on the old Secondary:
+{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0', 'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode': 'existing', 'format': 'raw', 'sync': 'full'} }
+
+Wait until disk is synced, then:
+{'execute': 'stop'}
+{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync' } }
 
-5. Failover test
-You can kill Primary VM and run 'x_colo_lost_heartbeat' in Secondary VM's
-monitor at the same time, then SVM will failover and client will not detect this
-change.
+{'execute': 'human-monitor-command', 'arguments':{ 'command-line': 'drive_add -n buddy driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.1,file.port=9999,file.export=parent0,node-name=replication0'}}
+{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0', 'node': 'replication0' } }
 
-Before issuing '{ "execute": "x-colo-lost-heartbeat" }' command, we have to
-issue block related command to stop block replication.
-Primary:
-  Remove the nbd child from the quorum:
-  { 'execute': 'x-blockdev-change', 'arguments': {'parent': 'colo-disk0', 'child': 'children.1'}}
-  { 'execute': 'human-monitor-command','arguments': {'command-line': 'drive_del blk-buddy0'}}
-  Note: there is no qmp command to remove the blockdev now
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before', 'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id': 'iothread1' } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare', 'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in': 'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
 
-Secondary:
-  The primary host is down, so we should do the following thing:
-  { 'execute': 'nbd-server-stop' }
+{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
+{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.1:9998' } }
 
 == TODO ==
-1. Support continuous VM replication.
-2. Support shared storage.
-3. Develop the heartbeat part.
-4. Reduce checkpoint VM’s downtime while doing checkpoint.
+1. Support shared storage.
+2. Develop the heartbeat part.
+3. Reduce checkpoint VM’s downtime while doing checkpoint.
diff --git a/docs/block-replication.txt b/docs/block-replication.txt
index 6bde6737fb..108e9166a8 100644
--- a/docs/block-replication.txt
+++ b/docs/block-replication.txt
@@ -65,12 +65,12 @@ blocks that are already in QEMU.
              ^            ||                            .----------
              |            ||                            | Secondary
         1 Quorum          ||                            '----------
-         /      \         ||
-        /        \        ||
-   Primary    2 filter
-     disk         ^                                                             virtio-blk
-                  |                                                                  ^
-                3 NBD  ------->  3 NBD                                               |
+         /      \         ||                                                           virtio-blk
+        /        \        ||                                                               ^
+   Primary    2 filter                                                                     |
+     disk         ^                                                                   7 Quorum
+                  |                                                                    /
+                3 NBD  ------->  3 NBD                                                /
                 client    ||     server                                          2 filter
                           ||        ^                                                ^
 --------.                 ||        |                                                |
@@ -106,6 +106,10 @@ any state that would otherwise be lost by the speculative write-through
 of the NBD server into the secondary disk. So before block replication,
 the primary disk and secondary disk should contain the same data.
 
+7) The secondary also has a quorum node, so after secondary failover it
+can become the new primary and continue replication.
+
+
 == Failure Handling ==
 There are 7 internal errors when block replication is running:
 1. I/O error on primary disk
@@ -171,16 +175,18 @@ Primary:
      leading whitespace.
   5. The qmp command line must be run after running qmp command line in
      secondary qemu.
-  6. After failover we need remove children.1 (replication driver).
+  6. After primary failover we need remove children.1 (replication driver).
 
 Secondary:
   -drive if=none,driver=raw,file.filename=1.raw,id=colo1 \
-  -drive if=xxx,id=topxxx,driver=replication,mode=secondary,top-id=topxxx\
+  -drive if=none,id=childs1,driver=replication,mode=secondary,top-id=childs1
          file.file.filename=active_disk.qcow2,\
          file.driver=qcow2,\
          file.backing.file.filename=hidden_disk.qcow2,\
          file.backing.driver=qcow2,\
          file.backing.backing=colo1
+  -drive if=xxx,driver=quorum,read-pattern=fifo,id=top-disk1,\
+         vote-threshold=1,children.0=childs1
 
   Then run qmp command in secondary qemu:
     { 'execute': 'nbd-server-start',
@@ -234,6 +240,8 @@ Secondary:
   The primary host is down, so we should do the following thing:
   { 'execute': 'nbd-server-stop' }
 
+Promote Secondary to Primary:
+  see COLO-FT.txt
+
 TODO:
-1. Continuous block replication
-2. Shared disk
+1. Shared disk
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication
  2019-10-05 13:05 ` [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication Lukas Straub
@ 2019-10-09  6:03   ` Zhang, Chen
  2019-10-09 15:19     ` Lukas Straub
  0 siblings, 1 reply; 16+ messages in thread
From: Zhang, Chen @ 2019-10-09  6:03 UTC (permalink / raw)
  To: Lukas Straub, qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Xie Changlong


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Saturday, October 5, 2019 9:06 PM
> To: qemu-devel <qemu-devel@nongnu.org>
> Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> <qemu-block@nongnu.org>
> Subject: [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary
> node continuing replication
> 
> This simulates the case that happens when we resume COLO after failover.
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>  tests/test-replication.c | 52
> ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/tests/test-replication.c b/tests/test-replication.c index
> f085d1993a..8e18ecd998 100644
> --- a/tests/test-replication.c
> +++ b/tests/test-replication.c
> @@ -489,6 +489,56 @@ static void test_secondary_stop(void)
>      teardown_secondary();
>  }
> 
> +static void test_secondary_continuous_replication(void)
> +{
> +    BlockBackend *top_blk, *local_blk;
> +    Error *local_err = NULL;
> +
> +    top_blk = start_secondary();
> +    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
> +    g_assert(!local_err);
> +
> +    /* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
> +    local_blk = blk_by_name(S_LOCAL_DISK_ID);
> +    test_blk_write(local_blk, 0x22, IMG_SIZE / 2, IMG_SIZE / 2, false);
> +
> +    /* replication will backup s_local_disk to s_hidden_disk */
> +    test_blk_read(top_blk, 0x11, IMG_SIZE / 2,
> +                  IMG_SIZE / 2, 0, IMG_SIZE, false);
> +
> +    /* write 0x33 to s_active_disk (0, IMG_SIZE / 2) */
> +    test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
> +
> +    /* do failover (active commit) */
> +    replication_stop_all(true, &local_err);
> +    g_assert(!local_err);
> +
> +    /* it should ignore all requests from now on */

Should we need add teardown_secondary() here?

Thanks
Zhang Chen

> +
> +    /* start after failover */
> +    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
> +    g_assert(!local_err);
> +
> +    /* checkpoint */
> +    replication_do_checkpoint_all(&local_err);
> +    g_assert(!local_err);
> +
> +    /* stop */
> +    replication_stop_all(true, &local_err);
> +    g_assert(!local_err);
> +
> +    /* read from s_local_disk (0, IMG_SIZE / 2) */
> +    test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
> +                  0, IMG_SIZE / 2, false);
> +
> +
> +    /* read from s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
> +    test_blk_read(top_blk, 0x22, IMG_SIZE / 2,
> +                  IMG_SIZE / 2, 0, IMG_SIZE, false);
> +
> +    teardown_secondary();
> +}
> +
>  static void test_secondary_do_checkpoint(void)
>  {
>      BlockBackend *top_blk, *local_blk;
> @@ -584,6 +634,8 @@ int main(int argc, char **argv)
>      g_test_add_func("/replication/secondary/write", test_secondary_write);
>      g_test_add_func("/replication/secondary/start", test_secondary_start);
>      g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
> +    g_test_add_func("/replication/secondary/continuous_replication",
> +                    test_secondary_continuous_replication);
>      g_test_add_func("/replication/secondary/do_checkpoint",
>                      test_secondary_do_checkpoint);
>      g_test_add_func("/replication/secondary/get_error_all",
> --
> 2.20.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 4/4] colo: Update Documentation for continuous replication
  2019-10-05 13:05 ` [PATCH v6 4/4] colo: Update Documentation for continuous replication Lukas Straub
@ 2019-10-09  8:36   ` Zhang, Chen
  2019-10-09 15:16     ` Lukas Straub
  0 siblings, 1 reply; 16+ messages in thread
From: Zhang, Chen @ 2019-10-09  8:36 UTC (permalink / raw)
  To: Lukas Straub, qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Xie Changlong


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Saturday, October 5, 2019 9:06 PM
> To: qemu-devel <qemu-devel@nongnu.org>
> Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> <qemu-block@nongnu.org>
> Subject: [PATCH v6 4/4] colo: Update Documentation for continuous
> replication
> 
> Document the qemu command-line and qmp commands for continuous
> replication
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>  docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++----------
>  docs/block-replication.txt |  28 +++--
>  2 files changed, 174 insertions(+), 67 deletions(-)
> 
> diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> ad24680d13..bc1a0ccb99 100644
> --- a/docs/COLO-FT.txt
> +++ b/docs/COLO-FT.txt
> @@ -145,35 +145,65 @@ The diagram just shows the main qmp command,
> you can get the detail  in test procedure.
> 
>  == Test procedure ==
> -1. Startup qemu
> -Primary:
> -# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name
> primary \
> -  -device piix3-usb-uhci -vnc :7 \
> -  -device usb-tablet -netdev tap,id=hn0,vhost=off \
> -  -device virtio-net-pci,id=net-pci0,netdev=hn0 \
> -  -drive if=virtio,id=primary-disk0,driver=quorum,read-pattern=fifo,vote-
> threshold=1,\
> -         children.0.file.filename=1.raw,\
> -         children.0.driver=raw -S
> -Secondary:
> -# qemu-system-x86_64 -accel kvm -m 2048 -smp 2 -qmp stdio -name
> secondary \
> -  -device piix3-usb-uhci -vnc :7 \
> -  -device usb-tablet -netdev tap,id=hn0,vhost=off \
> -  -device virtio-net-pci,id=net-pci0,netdev=hn0 \
> -  -drive if=none,id=secondary-disk0,file.filename=1.raw,driver=raw,node-
> name=node0 \
> -  -drive if=virtio,id=active-disk0,driver=replication,mode=secondary,\
> -         file.driver=qcow2,top-id=active-disk0,\
> -         file.file.filename=/mnt/ramfs/active_disk.img,\
> -         file.backing.driver=qcow2,\
> -         file.backing.file.filename=/mnt/ramfs/hidden_disk.img,\
> -         file.backing.backing=secondary-disk0 \
> -  -incoming tcp:0:8888
> -
> -2. On Secondary VM's QEMU monitor, issue command
> +Note: Here we are running both instances on the same Host for testing,
> +change the IP Addresses if you want to run it on two Hosts. Initally
> +127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
> +
> +== Startup qemu ==
> +1. Primary:
> +Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all Hosts.
> +# imagefolder="/mnt/vms/colo-test-primary"
> +
> +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp
> 1 -qmp stdio \
> +   -device piix3-usb-uhci -device usb-tablet -name primary \
> +   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> \
> +   -device rtl8139,id=e0,netdev=hn0 \
> +   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
> +   -chardev socket,id=compare1,host=0.0.0.0,port=9004,server,wait \

We should change the host=127.0.0.1 consistent with the expression below.

> +   -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait \
> +   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
> +   -chardev socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait
> \
> +   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
> +   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
> +   -object filter-
> redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
> +   -object filter-
> redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
> +   -object iothread,id=iothread1 \
> +   -object
> +colo-compare,id=comp0,primary_in=compare0-
> 0,secondary_in=compare1,\
> +outdev=compare_out0,iothread=iothread1 \
> +   -drive
> +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
> +children.0.file.filename=$imagefolder/primary.qcow2,children.0.driver=q
> +cow2 -S
> +
> +2. Secondary:
> +# imagefolder="/mnt/vms/colo-test-secondary"
> +# primary_ip=127.0.0.1
> +
> +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G
> +
> +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2 10G
> +

The active disk and hidden disk just need create one time, we can note that here.

> +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp
> 1 -qmp stdio \
> +   -device piix3-usb-uhci -device usb-tablet -name secondary \
> +   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> \
> +   -device rtl8139,id=e0,netdev=hn0 \
> +   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \
> +   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \
> +   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
> +   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
> +   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
> +   -drive
> if=none,id=parent0,file.filename=$imagefolder/primary.qcow2,driver=qcow
> 2 \
> +   -drive
> +if=none,id=childs0,driver=replication,mode=secondary,file.driver=qcow2,
> +\
> +top-id=childs0,file.file.filename=$imagefolder/secondary-active.qcow2,\
> +file.backing.driver=qcow2,file.backing.file.filename=$imagefolder/secon
> +dary-hidden.qcow2,\
> +file.backing.backing=parent0 \
> +   -drive
> +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
> +children.0=childs0 \
> +   -incoming tcp:0.0.0.0:9998
> +
> +
> +3. On Secondary VM's QEMU monitor, issue command
>  {'execute':'qmp_capabilities'}
> -{ 'execute': 'nbd-server-start',
> -  'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx', 'port':
> '8889'} } } -}
> -{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
> 'writable': true } }
> +{'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet',
> +'data': {'host': '0.0.0.0', 'port': '9999'} } } }
> +{'execute': 'nbd-server-add', 'arguments': {'device': 'parent0',
> +'writable': true } }
> 
>  Note:
>    a. The qmp command nbd-server-start and nbd-server-add must be run
> @@ -182,44 +212,113 @@ Note:
>       same.
>    c. It is better to put active disk and hidden disk in ramdisk.
> 
> -3. On Primary VM's QEMU monitor, issue command:
> +4. On Primary VM's QEMU monitor, issue command:
>  {'execute':'qmp_capabilities'}
> -{ 'execute': 'human-monitor-command',
> -  'arguments': {'command-line': 'drive_add -n buddy
> driver=replication,mode=primary,file.driver=nbd,file.host=xx.xx.xx.xx,file.p
> ort=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
> -{ 'execute':'x-blockdev-change', 'arguments':{'parent': 'primary-disk0',
> 'node': 'nbd_client0' } } -{ 'execute': 'migrate-set-capabilities',
> -      'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
> -{ 'execute': 'migrate', 'arguments': {'uri': 'tcp:xx.xx.xx.xx:8888' } }
> +{'execute': 'human-monitor-command', 'arguments': {'command-line':
> +'drive_add -n buddy
> +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,fil
> +e.port=9999,file.export=parent0,node-name=replication0'}}
> +{'execute': 'x-blockdev-change', 'arguments':{'parent': 'colo-disk0',
> +'node': 'replication0' } }
> +{'execute': 'migrate-set-capabilities', 'arguments': {'capabilities': [
> +{'capability': 'x-colo', 'state': true } ] } }
> +{'execute': 'migrate', 'arguments': {'uri': 'tcp:127.0.0.2:9998' } }
> 
>    Note:
>    a. There should be only one NBD Client for each primary disk.
> -  b. xx.xx.xx.xx is the secondary physical machine's hostname or IP
> -  c. The qmp command line must be run after running qmp command line in
> +  b. The qmp command line must be run after running qmp command line in
>       secondary qemu.
> 
> -4. After the above steps, you will see, whenever you make changes to PVM,
> SVM will be synced.
> +5. After the above steps, you will see, whenever you make changes to PVM,
> SVM will be synced.
>  You can issue command '{ "execute": "migrate-set-parameters" ,
> "arguments":{ "x-checkpoint-delay": 2000 } }'
> -to change the checkpoint period time
> +to change the idle checkpoint period time
> +
> +6. Failover test
> +You can kill one of the VMs and Failover on the surviving VM:
> +
> +If you killed the Secondary, then follow "Primary Failover". After
> +that, if you want to resume the replication, follow "Primary resume
> replication"
> +
> +If you killed the Primary, then follow "Secondary Failover". After
> +that, if you want to resume the replication, follow "Secondary resume
> replication"
> +
> +== Primary Failover ==
> +The Secondary died, resume on the Primary
> +
> +{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
> +'child': 'children.1'} }
> +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> +'drive_del replication0' } }
> +{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
> +{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
> +{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
> +{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
> +{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
> +{'execute': 'x-colo-lost-heartbeat' }
> +
> +== Secondary Failover ==
> +The Primary died, resume on the Secondary and prepare to become the
> new
> +Primary
> +
> +{'execute': 'nbd-server-stop'}
> +{'execute': 'x-colo-lost-heartbeat'}
> +
> +{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
> +{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
> +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
> +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
> +
> +{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend':
> +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> +'0.0.0.0', 'port': '9003' } }, 'server': true } } } }

Same like I said before.

Others statement looks good for me.

Thanks
Zhang Chen

> +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> +'0.0.0.0', 'port': '9004' } }, 'server': true } } } }
> +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend':
> +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> +'127.0.0.1', 'port': '9001' } }, 'server': true } } } }
> +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0-0', 'backend':
> +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> +'127.0.0.1', 'port': '9001' } }, 'server': false } } } }
> +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out',
> +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': true } } }
> +}
> +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out0',
> +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': false } }
> +} }
> +
> +== Primary resume replication ==
> +Resume replication after new Secondary is up.
> +
> +Start the new Secondary (Steps 2 and 3 above), then on the Primary:
> +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> +'job-id': 'resync', 'target': 'nbd://127.0.0.2:9999/parent0', 'mode':
> +'existing', 'format': 'raw', 'sync': 'full'} }
> +
> +Wait until disk is synced, then:
> +{'execute': 'stop'}
> +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync'} }
> +
> +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> +'drive_add -n buddy
> +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,fil
> +e.port=9999,file.export=parent0,node-name=replication0'}}
> +{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
> +'node': 'replication0' } }
> +
> +{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror',
> +'id': 'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev':
> +'mirror0' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type':
> +'filter-redirector', 'id': 'redire0', 'props': { 'netdev': 'hn0',
> +'queue': 'rx', 'indev': 'compare_out' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type':
> +'filter-redirector', 'id': 'redire1', 'props': { 'netdev': 'hn0',
> +'queue': 'rx', 'outdev': 'compare0' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> +'iothread1' } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> +
> +{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [
> +{'capability': 'x-colo', 'state': true } ] } }
> +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.2:9998' } }
> +
> +Note:
> +If this Primary previously was a Secondary, then we need to insert the
> +filters before the filter-rewriter by using the
> +"'insert': 'before', 'position': 'id=rew0'" Options. See below.
> +
> +== Secondary resume replication ==
> +Become Primary and resume replication after new Secondary is up. Note
> +that now 127.0.0.1 is the Secondary and 127.0.0.2 is the Primary.
> +
> +Start the new Secondary (Steps 2 and 3 above, but with
> +primary_ip=127.0.0.2), then on the old Secondary:
> +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> +'existing', 'format': 'raw', 'sync': 'full'} }
> +
> +Wait until disk is synced, then:
> +{'execute': 'stop'}
> +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync' } }
> 
> -5. Failover test
> -You can kill Primary VM and run 'x_colo_lost_heartbeat' in Secondary VM's -
> monitor at the same time, then SVM will failover and client will not detect
> this -change.
> +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> +'drive_add -n buddy
> +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.1,fil
> +e.port=9999,file.export=parent0,node-name=replication0'}}
> +{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
> +'node': 'replication0' } }
> 
> -Before issuing '{ "execute": "x-colo-lost-heartbeat" }' command, we have to
> -issue block related command to stop block replication.
> -Primary:
> -  Remove the nbd child from the quorum:
> -  { 'execute': 'x-blockdev-change', 'arguments': {'parent': 'colo-disk0', 'child':
> 'children.1'}}
> -  { 'execute': 'human-monitor-command','arguments': {'command-line':
> 'drive_del blk-buddy0'}}
> -  Note: there is no qmp command to remove the blockdev now
> +{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror',
> +'id': 'm0', 'props': { 'insert': 'before', 'position': 'id=rew0',
> +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type':
> +'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before',
> +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> +'compare_out' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type':
> +'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before',
> +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> +'compare0' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> +'iothread1' } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> 
> -Secondary:
> -  The primary host is down, so we should do the following thing:
> -  { 'execute': 'nbd-server-stop' }
> +{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [
> +{'capability': 'x-colo', 'state': true } ] } }
> +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.1:9998' } }
> 
>  == TODO ==
> -1. Support continuous VM replication.
> -2. Support shared storage.
> -3. Develop the heartbeat part.
> -4. Reduce checkpoint VM’s downtime while doing checkpoint.
> +1. Support shared storage.
> +2. Develop the heartbeat part.
> +3. Reduce checkpoint VM’s downtime while doing checkpoint.
> diff --git a/docs/block-replication.txt b/docs/block-replication.txt index
> 6bde6737fb..108e9166a8 100644
> --- a/docs/block-replication.txt
> +++ b/docs/block-replication.txt
> @@ -65,12 +65,12 @@ blocks that are already in QEMU.
>               ^            ||                            .----------
>               |            ||                            | Secondary
>          1 Quorum          ||                            '----------
> -         /      \         ||
> -        /        \        ||
> -   Primary    2 filter
> -     disk         ^                                                             virtio-blk
> -                  |                                                                  ^
> -                3 NBD  ------->  3 NBD                                               |
> +         /      \         ||                                                           virtio-blk
> +        /        \        ||                                                               ^
> +   Primary    2 filter                                                                     |
> +     disk         ^                                                                   7 Quorum
> +                  |                                                                    /
> +                3 NBD  ------->  3 NBD                                                /
>                  client    ||     server                                          2 filter
>                            ||        ^                                                ^
>  --------.                 ||        |                                                |
> @@ -106,6 +106,10 @@ any state that would otherwise be lost by the
> speculative write-through  of the NBD server into the secondary disk. So
> before block replication,  the primary disk and secondary disk should contain
> the same data.
> 
> +7) The secondary also has a quorum node, so after secondary failover it
> +can become the new primary and continue replication.
> +
> +
>  == Failure Handling ==
>  There are 7 internal errors when block replication is running:
>  1. I/O error on primary disk
> @@ -171,16 +175,18 @@ Primary:
>       leading whitespace.
>    5. The qmp command line must be run after running qmp command line in
>       secondary qemu.
> -  6. After failover we need remove children.1 (replication driver).
> +  6. After primary failover we need remove children.1 (replication driver).
> 
>  Secondary:
>    -drive if=none,driver=raw,file.filename=1.raw,id=colo1 \
> -  -drive if=xxx,id=topxxx,driver=replication,mode=secondary,top-
> id=topxxx\
> +  -drive
> + if=none,id=childs1,driver=replication,mode=secondary,top-id=childs1
>           file.file.filename=active_disk.qcow2,\
>           file.driver=qcow2,\
>           file.backing.file.filename=hidden_disk.qcow2,\
>           file.backing.driver=qcow2,\
>           file.backing.backing=colo1
> +  -drive if=xxx,driver=quorum,read-pattern=fifo,id=top-disk1,\
> +         vote-threshold=1,children.0=childs1
> 
>    Then run qmp command in secondary qemu:
>      { 'execute': 'nbd-server-start',
> @@ -234,6 +240,8 @@ Secondary:
>    The primary host is down, so we should do the following thing:
>    { 'execute': 'nbd-server-stop' }
> 
> +Promote Secondary to Primary:
> +  see COLO-FT.txt
> +
>  TODO:
> -1. Continuous block replication
> -2. Shared disk
> +1. Shared disk
> --
> 2.20.1

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 4/4] colo: Update Documentation for continuous replication
  2019-10-09  8:36   ` Zhang, Chen
@ 2019-10-09 15:16     ` Lukas Straub
  2019-10-10 10:34       ` Zhang, Chen
  0 siblings, 1 reply; 16+ messages in thread
From: Lukas Straub @ 2019-10-09 15:16 UTC (permalink / raw)
  To: Zhang, Chen
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, qemu-devel,
	Max Reitz, Xie Changlong

On Wed, 9 Oct 2019 08:36:52 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub <lukasstraub2@web.de>
> > Sent: Saturday, October 5, 2019 9:06 PM
> > To: qemu-devel <qemu-devel@nongnu.org>
> > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> > Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> > <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> > <qemu-block@nongnu.org>
> > Subject: [PATCH v6 4/4] colo: Update Documentation for continuous
> > replication
> > 
> > Document the qemu command-line and qmp commands for continuous
> > replication
> > 
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >  docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++----------
> >  docs/block-replication.txt |  28 +++--
> >  2 files changed, 174 insertions(+), 67 deletions(-)
> > 
> > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > ad24680d13..bc1a0ccb99 100644
> > --- a/docs/COLO-FT.txt
> > +++ b/docs/COLO-FT.txt
> > @@ -145,35 +145,65 @@ The diagram just shows the main qmp command,
> > you can get the detail  in test procedure.
> > 
> > ...
> >
> > +Note: Here we are running both instances on the same Host for testing,
> > +change the IP Addresses if you want to run it on two Hosts. Initally
> > +127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
> > +
> > +== Startup qemu ==
> > +1. Primary:
> > +Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all Hosts.
> > +# imagefolder="/mnt/vms/colo-test-primary"
> > +
> > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp
> > 1 -qmp stdio \
> > +   -device piix3-usb-uhci -device usb-tablet -name primary \
> > +   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > \
> > +   -device rtl8139,id=e0,netdev=hn0 \
> > +   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
> > +   -chardev socket,id=compare1,host=0.0.0.0,port=9004,server,wait \  
> 
> We should change the host=127.0.0.1 consistent with the expression below.

Hi,
This (and the IPs below in the QMP commands) needs to be this way, because 
it's a listening port and with 127.0.0.1 it would only listen on the loopback ip
and wouldn't be reachable from another node for example. With 0.0.0.0 it will
listen on all Interfaces.

> > +   -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait \
> > +   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
> > +   -chardev socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait
> > \
> > +   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
> > +   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
> > +   -object filter-
> > redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
> > +   -object filter-
> > redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
> > +   -object iothread,id=iothread1 \
> > +   -object
> > +colo-compare,id=comp0,primary_in=compare0-
> > 0,secondary_in=compare1,\
> > +outdev=compare_out0,iothread=iothread1 \
> > +   -drive
> > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
> > +children.0.file.filename=$imagefolder/primary.qcow2,children.0.driver=q
> > +cow2 -S
> > +
> > +2. Secondary:
> > +# imagefolder="/mnt/vms/colo-test-secondary"
> > +# primary_ip=127.0.0.1
> > +
> > +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G
> > +
> > +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2 10G
> > +  
> 
> The active disk and hidden disk just need create one time, we can note that here.

Ok, I will Note that. But I will wait until the block changes are reviewed before sending the next version.

Regards,
Lukas Straub

> > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp
> > 1 -qmp stdio \
> > +   -device piix3-usb-uhci -device usb-tablet -name secondary \
> > +   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > \
> > +   -device rtl8139,id=e0,netdev=hn0 \
> > +   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \
> > +   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \
> > +   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
> > +   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
> > +   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
> > +   -drive
> > if=none,id=parent0,file.filename=$imagefolder/primary.qcow2,driver=qcow
> > 2 \
> > +   -drive
> > +if=none,id=childs0,driver=replication,mode=secondary,file.driver=qcow2,
> > +\
> > +top-id=childs0,file.file.filename=$imagefolder/secondary-active.qcow2,\
> > +file.backing.driver=qcow2,file.backing.file.filename=$imagefolder/secon
> > +dary-hidden.qcow2,\
> > +file.backing.backing=parent0 \
> > +   -drive
> > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
> > +children.0=childs0 \
> > +   -incoming tcp:0.0.0.0:9998
> > +
> > +
> > +3. On Secondary VM's QEMU monitor, issue command
> >  {'execute':'qmp_capabilities'}
> > -{ 'execute': 'nbd-server-start',
> > -  'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx', 'port':
> > '8889'} } } -}
> > -{'execute': 'nbd-server-add', 'arguments': {'device': 'secondary-disk0',
> > 'writable': true } }
> > +{'execute': 'nbd-server-start', 'arguments': {'addr': {'type': 'inet',
> > +'data': {'host': '0.0.0.0', 'port': '9999'} } } }
> > +{'execute': 'nbd-server-add', 'arguments': {'device': 'parent0',
> > +'writable': true } }
> > 
> >  Note:
> >    a. The qmp command nbd-server-start and nbd-server-add must be run
> > @@ -182,44 +212,113 @@ Note:
> >       same.
> >    c. It is better to put active disk and hidden disk in ramdisk.
> > 
> > -3. On Primary VM's QEMU monitor, issue command:
> > +4. On Primary VM's QEMU monitor, issue command:
> >  {'execute':'qmp_capabilities'}
> > -{ 'execute': 'human-monitor-command',
> > -  'arguments': {'command-line': 'drive_add -n buddy
> > driver=replication,mode=primary,file.driver=nbd,file.host=xx.xx.xx.xx,file.p
> > ort=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
> > -{ 'execute':'x-blockdev-change', 'arguments':{'parent': 'primary-disk0',
> > 'node': 'nbd_client0' } } -{ 'execute': 'migrate-set-capabilities',
> > -      'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
> > -{ 'execute': 'migrate', 'arguments': {'uri': 'tcp:xx.xx.xx.xx:8888' } }
> > +{'execute': 'human-monitor-command', 'arguments': {'command-line':
> > +'drive_add -n buddy
> > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,fil
> > +e.port=9999,file.export=parent0,node-name=replication0'}}
> > +{'execute': 'x-blockdev-change', 'arguments':{'parent': 'colo-disk0',
> > +'node': 'replication0' } }
> > +{'execute': 'migrate-set-capabilities', 'arguments': {'capabilities': [
> > +{'capability': 'x-colo', 'state': true } ] } }
> > +{'execute': 'migrate', 'arguments': {'uri': 'tcp:127.0.0.2:9998' } }
> > 
> >    Note:
> >    a. There should be only one NBD Client for each primary disk.
> > -  b. xx.xx.xx.xx is the secondary physical machine's hostname or IP
> > -  c. The qmp command line must be run after running qmp command line in
> > +  b. The qmp command line must be run after running qmp command line in
> >       secondary qemu.
> > 
> > -4. After the above steps, you will see, whenever you make changes to PVM,
> > SVM will be synced.
> > +5. After the above steps, you will see, whenever you make changes to PVM,
> > SVM will be synced.
> >  You can issue command '{ "execute": "migrate-set-parameters" ,
> > "arguments":{ "x-checkpoint-delay": 2000 } }'
> > -to change the checkpoint period time
> > +to change the idle checkpoint period time
> > +
> > +6. Failover test
> > +You can kill one of the VMs and Failover on the surviving VM:
> > +
> > +If you killed the Secondary, then follow "Primary Failover". After
> > +that, if you want to resume the replication, follow "Primary resume
> > replication"
> > +
> > +If you killed the Primary, then follow "Secondary Failover". After
> > +that, if you want to resume the replication, follow "Secondary resume
> > replication"
> > +
> > +== Primary Failover ==
> > +The Secondary died, resume on the Primary
> > +
> > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
> > +'child': 'children.1'} }
> > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > +'drive_del replication0' } }
> > +{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
> > +{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
> > +{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
> > +{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
> > +{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
> > +{'execute': 'x-colo-lost-heartbeat' }
> > +
> > +== Secondary Failover ==
> > +The Primary died, resume on the Secondary and prepare to become the
> > new
> > +Primary
> > +
> > +{'execute': 'nbd-server-stop'}
> > +{'execute': 'x-colo-lost-heartbeat'}
> > +
> > +{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
> > +{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
> > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
> > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
> > +
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend':
> > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > +'0.0.0.0', 'port': '9003' } }, 'server': true } } } }  
> 
> Same like I said before.
> 
> Others statement looks good for me.
> 
> Thanks
> Zhang Chen
> 
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > +'0.0.0.0', 'port': '9004' } }, 'server': true } } } }
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend':
> > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > +'127.0.0.1', 'port': '9001' } }, 'server': true } } } }
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0-0', 'backend':
> > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > +'127.0.0.1', 'port': '9001' } }, 'server': false } } } }
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out',
> > +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': true } } }
> > +}
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out0',
> > +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': false } }
> > +} }
> > +
> > +== Primary resume replication ==
> > +Resume replication after new Secondary is up.
> > +
> > +Start the new Secondary (Steps 2 and 3 above), then on the Primary:
> > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > +'job-id': 'resync', 'target': 'nbd://127.0.0.2:9999/parent0', 'mode':
> > +'existing', 'format': 'raw', 'sync': 'full'} }
> > +
> > +Wait until disk is synced, then:
> > +{'execute': 'stop'}
> > +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync'} }
> > +
> > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > +'drive_add -n buddy
> > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2,fil
> > +e.port=9999,file.export=parent0,node-name=replication0'}}
> > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
> > +'node': 'replication0' } }
> > +
> > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror',
> > +'id': 'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev':
> > +'mirror0' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > +'filter-redirector', 'id': 'redire0', 'props': { 'netdev': 'hn0',
> > +'queue': 'rx', 'indev': 'compare_out' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > +'filter-redirector', 'id': 'redire1', 'props': { 'netdev': 'hn0',
> > +'queue': 'rx', 'outdev': 'compare0' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > +'iothread1' } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> > +
> > +{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [
> > +{'capability': 'x-colo', 'state': true } ] } }
> > +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.2:9998' } }
> > +
> > +Note:
> > +If this Primary previously was a Secondary, then we need to insert the
> > +filters before the filter-rewriter by using the
> > +"'insert': 'before', 'position': 'id=rew0'" Options. See below.
> > +
> > +== Secondary resume replication ==
> > +Become Primary and resume replication after new Secondary is up. Note
> > +that now 127.0.0.1 is the Secondary and 127.0.0.2 is the Primary.
> > +
> > +Start the new Secondary (Steps 2 and 3 above, but with
> > +primary_ip=127.0.0.2), then on the old Secondary:
> > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> > +'existing', 'format': 'raw', 'sync': 'full'} }
> > +
> > +Wait until disk is synced, then:
> > +{'execute': 'stop'}
> > +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync' } }
> > 
> > -5. Failover test
> > -You can kill Primary VM and run 'x_colo_lost_heartbeat' in Secondary VM's -
> > monitor at the same time, then SVM will failover and client will not detect
> > this -change.
> > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > +'drive_add -n buddy
> > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.1,fil
> > +e.port=9999,file.export=parent0,node-name=replication0'}}
> > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent': 'colo-disk0',
> > +'node': 'replication0' } }
> > 
> > -Before issuing '{ "execute": "x-colo-lost-heartbeat" }' command, we have to
> > -issue block related command to stop block replication.
> > -Primary:
> > -  Remove the nbd child from the quorum:
> > -  { 'execute': 'x-blockdev-change', 'arguments': {'parent': 'colo-disk0', 'child':
> > 'children.1'}}
> > -  { 'execute': 'human-monitor-command','arguments': {'command-line':
> > 'drive_del blk-buddy0'}}
> > -  Note: there is no qmp command to remove the blockdev now
> > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror',
> > +'id': 'm0', 'props': { 'insert': 'before', 'position': 'id=rew0',
> > +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > +'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before',
> > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > +'compare_out' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > +'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before',
> > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> > +'compare0' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > +'iothread1' } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> > 
> > -Secondary:
> > -  The primary host is down, so we should do the following thing:
> > -  { 'execute': 'nbd-server-stop' }
> > +{'execute': 'migrate-set-capabilities', 'arguments':{ 'capabilities': [
> > +{'capability': 'x-colo', 'state': true } ] } }
> > +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.1:9998' } }
> > 
> >  == TODO ==
> > -1. Support continuous VM replication.
> > -2. Support shared storage.
> > -3. Develop the heartbeat part.
> > -4. Reduce checkpoint VM’s downtime while doing checkpoint.
> > +1. Support shared storage.
> > +2. Develop the heartbeat part.
> > +3. Reduce checkpoint VM’s downtime while doing checkpoint.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication
  2019-10-09  6:03   ` Zhang, Chen
@ 2019-10-09 15:19     ` Lukas Straub
  0 siblings, 0 replies; 16+ messages in thread
From: Lukas Straub @ 2019-10-09 15:19 UTC (permalink / raw)
  To: Zhang, Chen
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, qemu-devel,
	Max Reitz, Xie Changlong

On Wed, 9 Oct 2019 06:03:03 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub <lukasstraub2@web.de>
> > Sent: Saturday, October 5, 2019 9:06 PM
> > To: qemu-devel <qemu-devel@nongnu.org>
> > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> > Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> > <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> > <qemu-block@nongnu.org>
> > Subject: [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary
> > node continuing replication
> >
> > This simulates the case that happens when we resume COLO after failover.
> >
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >  tests/test-replication.c | 52
> > ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/tests/test-replication.c b/tests/test-replication.c index
> > f085d1993a..8e18ecd998 100644
> > --- a/tests/test-replication.c
> > +++ b/tests/test-replication.c
> > @@ -489,6 +489,56 @@ static void test_secondary_stop(void)
> >      teardown_secondary();
> >  }
> >
> > +static void test_secondary_continuous_replication(void)
> > +{
> > +    BlockBackend *top_blk, *local_blk;
> > +    Error *local_err = NULL;
> > +
> > +    top_blk = start_secondary();
> > +    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
> > +    g_assert(!local_err);
> > +
> > +    /* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
> > +    local_blk = blk_by_name(S_LOCAL_DISK_ID);
> > +    test_blk_write(local_blk, 0x22, IMG_SIZE / 2, IMG_SIZE / 2, false);
> > +
> > +    /* replication will backup s_local_disk to s_hidden_disk */
> > +    test_blk_read(top_blk, 0x11, IMG_SIZE / 2,
> > +                  IMG_SIZE / 2, 0, IMG_SIZE, false);
> > +
> > +    /* write 0x33 to s_active_disk (0, IMG_SIZE / 2) */
> > +    test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
> > +
> > +    /* do failover (active commit) */
> > +    replication_stop_all(true, &local_err);
> > +    g_assert(!local_err);
> > +
> > +    /* it should ignore all requests from now on */
>
> Should we need add teardown_secondary() here?

I don't think so. It is used to remove the block node after each test and I'm doing that below.

Regards,
Lukas Straub

> Thanks
> Zhang Chen
>
> > +
> > +    /* start after failover */
> > +    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
> > +    g_assert(!local_err);
> > +
> > +    /* checkpoint */
> > +    replication_do_checkpoint_all(&local_err);
> > +    g_assert(!local_err);
> > +
> > +    /* stop */
> > +    replication_stop_all(true, &local_err);
> > +    g_assert(!local_err);
> > +
> > +    /* read from s_local_disk (0, IMG_SIZE / 2) */
> > +    test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
> > +                  0, IMG_SIZE / 2, false);
> > +
> > +
> > +    /* read from s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
> > +    test_blk_read(top_blk, 0x22, IMG_SIZE / 2,
> > +                  IMG_SIZE / 2, 0, IMG_SIZE, false);
> > +
> > +    teardown_secondary();
> > +}
> > +
> >  static void test_secondary_do_checkpoint(void)
> >  {
> >      BlockBackend *top_blk, *local_blk;
> > @@ -584,6 +634,8 @@ int main(int argc, char **argv)
> >      g_test_add_func("/replication/secondary/write", test_secondary_write);
> >      g_test_add_func("/replication/secondary/start", test_secondary_start);
> >      g_test_add_func("/replication/secondary/stop",  test_secondary_stop);
> > +    g_test_add_func("/replication/secondary/continuous_replication",
> > +                    test_secondary_continuous_replication);
> >      g_test_add_func("/replication/secondary/do_checkpoint",
> >                      test_secondary_do_checkpoint);
> >      g_test_add_func("/replication/secondary/get_error_all",
> > --
> > 2.20.1
>



^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 4/4] colo: Update Documentation for continuous replication
  2019-10-09 15:16     ` Lukas Straub
@ 2019-10-10 10:34       ` Zhang, Chen
  2019-10-11 16:00         ` Lukas Straub
  0 siblings, 1 reply; 16+ messages in thread
From: Zhang, Chen @ 2019-10-10 10:34 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, qemu-devel,
	Max Reitz, Xie Changlong


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Wednesday, October 9, 2019 11:17 PM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang
> <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> <qemu-block@nongnu.org>
> Subject: Re: [PATCH v6 4/4] colo: Update Documentation for continuous
> replication
> 
> On Wed, 9 Oct 2019 08:36:52 +0000
> "Zhang, Chen" <chen.zhang@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukas Straub <lukasstraub2@web.de>
> > > Sent: Saturday, October 5, 2019 9:06 PM
> > > To: qemu-devel <qemu-devel@nongnu.org>
> > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> > > <jasowang@redhat.com>; Wen Congyang
> <wencongyang2@huawei.com>; Xie
> > > Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>;
> > > Max Reitz <mreitz@redhat.com>; qemu-block <qemu-
> block@nongnu.org>
> > > Subject: [PATCH v6 4/4] colo: Update Documentation for continuous
> > > replication
> > >
> > > Document the qemu command-line and qmp commands for continuous
> > > replication
> > >
> > > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > > ---
> > >  docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++----------
> > >  docs/block-replication.txt |  28 +++--
> > >  2 files changed, 174 insertions(+), 67 deletions(-)
> > >
> > > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > > ad24680d13..bc1a0ccb99 100644
> > > --- a/docs/COLO-FT.txt
> > > +++ b/docs/COLO-FT.txt
> > > @@ -145,35 +145,65 @@ The diagram just shows the main qmp
> command,
> > > you can get the detail  in test procedure.
> > >
> > > ...
> > >
> > > +Note: Here we are running both instances on the same Host for
> > > +testing, change the IP Addresses if you want to run it on two
> > > +Hosts. Initally
> > > +127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
> > > +
> > > +== Startup qemu ==
> > > +1. Primary:
> > > +Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all
> Hosts.
> > > +# imagefolder="/mnt/vms/colo-test-primary"
> > > +
> > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -
> smp
> > > 1 -qmp stdio \
> > > +   -device piix3-usb-uhci -device usb-tablet -name primary \
> > > +   -netdev
> > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > \
> > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > +   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
> > > +   -chardev socket,id=compare1,host=0.0.0.0,port=9004,server,wait \
> >
> > We should change the host=127.0.0.1 consistent with the expression below.
> 
> Hi,
> This (and the IPs below in the QMP commands) needs to be this way,
> because it's a listening port and with 127.0.0.1 it would only listen on the
> loopback ip and wouldn't be reachable from another node for example. With
> 0.0.0.0 it will listen on all Interfaces.

Yes, I know.  For this command demo, maybe use 192.168.0.1/192.168.0.2 are more clear.

> 
> > > +   -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait
> \
> > > +   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
> > > +   -chardev
> > > + socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait
> > > \
> > > +   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
> > > +   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
> > > +   -object filter-
> > > redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
> > > +   -object filter-
> > > redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
> > > +   -object iothread,id=iothread1 \
> > > +   -object
> > > +colo-compare,id=comp0,primary_in=compare0-
> > > 0,secondary_in=compare1,\
> > > +outdev=compare_out0,iothread=iothread1 \
> > > +   -drive
> > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold
> > > +=1,\
> > > +children.0.file.filename=$imagefolder/primary.qcow2,children.0.driv
> > > +er=q
> > > +cow2 -S
> > > +
> > > +2. Secondary:
> > > +# imagefolder="/mnt/vms/colo-test-secondary"
> > > +# primary_ip=127.0.0.1
> > > +
> > > +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G
> > > +
> > > +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2
> 10G
> > > +
> >
> > The active disk and hidden disk just need create one time, we can note that
> here.
> 
> Ok, I will Note that. But I will wait until the block changes are reviewed
> before sending the next version.

That's fine for me.

Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub
> 
> > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -
> smp
> > > 1 -qmp stdio \
> > > +   -device piix3-usb-uhci -device usb-tablet -name secondary \
> > > +   -netdev
> > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > \
> > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > +   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \
> > > +   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \
> > > +   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
> > > +   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
> > > +   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
> > > +   -drive
> > > if=none,id=parent0,file.filename=$imagefolder/primary.qcow2,driver=q
> > > cow
> > > 2 \
> > > +   -drive
> > > +if=none,id=childs0,driver=replication,mode=secondary,file.driver=qc
> > > +ow2,
> > > +\
> > > +top-id=childs0,file.file.filename=$imagefolder/secondary-active.qco
> > > +w2,\
> > > +file.backing.driver=qcow2,file.backing.file.filename=$imagefolder/s
> > > +econ
> > > +dary-hidden.qcow2,\
> > > +file.backing.backing=parent0 \
> > > +   -drive
> > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold
> > > +=1,\
> > > +children.0=childs0 \
> > > +   -incoming tcp:0.0.0.0:9998
> > > +
> > > +
> > > +3. On Secondary VM's QEMU monitor, issue command
> > >  {'execute':'qmp_capabilities'}
> > > -{ 'execute': 'nbd-server-start',
> > > -  'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx', 'port':
> > > '8889'} } } -}
> > > -{'execute': 'nbd-server-add', 'arguments': {'device':
> > > 'secondary-disk0',
> > > 'writable': true } }
> > > +{'execute': 'nbd-server-start', 'arguments': {'addr': {'type':
> > > +'inet',
> > > +'data': {'host': '0.0.0.0', 'port': '9999'} } } }
> > > +{'execute': 'nbd-server-add', 'arguments': {'device': 'parent0',
> > > +'writable': true } }
> > >
> > >  Note:
> > >    a. The qmp command nbd-server-start and nbd-server-add must be
> > > run @@ -182,44 +212,113 @@ Note:
> > >       same.
> > >    c. It is better to put active disk and hidden disk in ramdisk.
> > >
> > > -3. On Primary VM's QEMU monitor, issue command:
> > > +4. On Primary VM's QEMU monitor, issue command:
> > >  {'execute':'qmp_capabilities'}
> > > -{ 'execute': 'human-monitor-command',
> > > -  'arguments': {'command-line': 'drive_add -n buddy
> > > driver=replication,mode=primary,file.driver=nbd,file.host=xx.xx.xx.x
> > > x,file.p
> > > ort=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
> > > -{ 'execute':'x-blockdev-change', 'arguments':{'parent':
> > > 'primary-disk0',
> > > 'node': 'nbd_client0' } } -{ 'execute': 'migrate-set-capabilities',
> > > -      'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
> > > -{ 'execute': 'migrate', 'arguments': {'uri': 'tcp:xx.xx.xx.xx:8888'
> > > } }
> > > +{'execute': 'human-monitor-command', 'arguments': {'command-line':
> > > +'drive_add -n buddy
> > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2
> > > +,fil e.port=9999,file.export=parent0,node-name=replication0'}}
> > > +{'execute': 'x-blockdev-change', 'arguments':{'parent':
> > > +'colo-disk0',
> > > +'node': 'replication0' } }
> > > +{'execute': 'migrate-set-capabilities', 'arguments':
> > > +{'capabilities': [
> > > +{'capability': 'x-colo', 'state': true } ] } }
> > > +{'execute': 'migrate', 'arguments': {'uri': 'tcp:127.0.0.2:9998' }
> > > +}
> > >
> > >    Note:
> > >    a. There should be only one NBD Client for each primary disk.
> > > -  b. xx.xx.xx.xx is the secondary physical machine's hostname or IP
> > > -  c. The qmp command line must be run after running qmp command
> > > line in
> > > +  b. The qmp command line must be run after running qmp command
> > > + line in
> > >       secondary qemu.
> > >
> > > -4. After the above steps, you will see, whenever you make changes
> > > to PVM, SVM will be synced.
> > > +5. After the above steps, you will see, whenever you make changes
> > > +to PVM,
> > > SVM will be synced.
> > >  You can issue command '{ "execute": "migrate-set-parameters" ,
> > > "arguments":{ "x-checkpoint-delay": 2000 } }'
> > > -to change the checkpoint period time
> > > +to change the idle checkpoint period time
> > > +
> > > +6. Failover test
> > > +You can kill one of the VMs and Failover on the surviving VM:
> > > +
> > > +If you killed the Secondary, then follow "Primary Failover". After
> > > +that, if you want to resume the replication, follow "Primary resume
> > > replication"
> > > +
> > > +If you killed the Primary, then follow "Secondary Failover". After
> > > +that, if you want to resume the replication, follow "Secondary
> > > +resume
> > > replication"
> > > +
> > > +== Primary Failover ==
> > > +The Secondary died, resume on the Primary
> > > +
> > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > +'colo-disk0',
> > > +'child': 'children.1'} }
> > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > > +'drive_del replication0' } }
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
> > > +{'execute': 'x-colo-lost-heartbeat' }
> > > +
> > > +== Secondary Failover ==
> > > +The Primary died, resume on the Secondary and prepare to become the
> > > new
> > > +Primary
> > > +
> > > +{'execute': 'nbd-server-stop'}
> > > +{'execute': 'x-colo-lost-heartbeat'}
> > > +
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
> > > +{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
> > > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
> > > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
> > > +
> > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend':
> > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > +'0.0.0.0', 'port': '9003' } }, 'server': true } } } }
> >
> > Same like I said before.
> >
> > Others statement looks good for me.
> >
> > Thanks
> > Zhang Chen
> >
> > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > +'0.0.0.0', 'port': '9004' } }, 'server': true } } } }
> > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend':
> > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > +'127.0.0.1', 'port': '9001' } }, 'server': true } } } }
> > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0-0', 'backend':
> > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > +'127.0.0.1', 'port': '9001' } }, 'server': false } } } }
> > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out',
> > > +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> > > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': true }
> > > +} } }
> > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out0',
> > > +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> > > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': false
> > > +} } } }
> > > +
> > > +== Primary resume replication ==
> > > +Resume replication after new Secondary is up.
> > > +
> > > +Start the new Secondary (Steps 2 and 3 above), then on the Primary:
> > > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > > +'job-id': 'resync', 'target': 'nbd://127.0.0.2:9999/parent0', 'mode':
> > > +'existing', 'format': 'raw', 'sync': 'full'} }
> > > +
> > > +Wait until disk is synced, then:
> > > +{'execute': 'stop'}
> > > +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync'} }
> > > +
> > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > > +'drive_add -n buddy
> > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2
> > > +,fil e.port=9999,file.export=parent0,node-name=replication0'}}
> > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > +'colo-disk0',
> > > +'node': 'replication0' } }
> > > +
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > +'filter-mirror',
> > > +'id': 'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev':
> > > +'mirror0' } } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > +'filter-redirector', 'id': 'redire0', 'props': { 'netdev': 'hn0',
> > > +'queue': 'rx', 'indev': 'compare_out' } } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > +'filter-redirector', 'id': 'redire1', 'props': { 'netdev': 'hn0',
> > > +'queue': 'rx', 'outdev': 'compare0' } } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > > +'iothread1' } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> > > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> > > +
> > > +{'execute': 'migrate-set-capabilities', 'arguments':{
> > > +'capabilities': [
> > > +{'capability': 'x-colo', 'state': true } ] } }
> > > +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.2:9998' }
> > > +}
> > > +
> > > +Note:
> > > +If this Primary previously was a Secondary, then we need to insert
> > > +the filters before the filter-rewriter by using the
> > > +"'insert': 'before', 'position': 'id=rew0'" Options. See below.
> > > +
> > > +== Secondary resume replication ==
> > > +Become Primary and resume replication after new Secondary is up.
> > > +Note that now 127.0.0.1 is the Secondary and 127.0.0.2 is the Primary.
> > > +
> > > +Start the new Secondary (Steps 2 and 3 above, but with
> > > +primary_ip=127.0.0.2), then on the old Secondary:
> > > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > > +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> > > +'existing', 'format': 'raw', 'sync': 'full'} }
> > > +
> > > +Wait until disk is synced, then:
> > > +{'execute': 'stop'}
> > > +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync' }
> > > +}
> > >
> > > -5. Failover test
> > > -You can kill Primary VM and run 'x_colo_lost_heartbeat' in
> > > Secondary VM's - monitor at the same time, then SVM will failover
> > > and client will not detect this -change.
> > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > > +'drive_add -n buddy
> > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.1
> > > +,fil e.port=9999,file.export=parent0,node-name=replication0'}}
> > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > +'colo-disk0',
> > > +'node': 'replication0' } }
> > >
> > > -Before issuing '{ "execute": "x-colo-lost-heartbeat" }' command, we
> > > have to -issue block related command to stop block replication.
> > > -Primary:
> > > -  Remove the nbd child from the quorum:
> > > -  { 'execute': 'x-blockdev-change', 'arguments': {'parent': 'colo-disk0',
> 'child':
> > > 'children.1'}}
> > > -  { 'execute': 'human-monitor-command','arguments': {'command-line':
> > > 'drive_del blk-buddy0'}}
> > > -  Note: there is no qmp command to remove the blockdev now
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > +'filter-mirror',
> > > +'id': 'm0', 'props': { 'insert': 'before', 'position': 'id=rew0',
> > > +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > +'filter-redirector', 'id': 'redire0', 'props': { 'insert':
> > > +'before',
> > > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > > +'compare_out' } } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > +'filter-redirector', 'id': 'redire1', 'props': { 'insert':
> > > +'before',
> > > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> > > +'compare0' } } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > > +'iothread1' } }
> > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> > > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> > >
> > > -Secondary:
> > > -  The primary host is down, so we should do the following thing:
> > > -  { 'execute': 'nbd-server-stop' }
> > > +{'execute': 'migrate-set-capabilities', 'arguments':{
> > > +'capabilities': [
> > > +{'capability': 'x-colo', 'state': true } ] } }
> > > +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.1:9998' }
> > > +}
> > >
> > >  == TODO ==
> > > -1. Support continuous VM replication.
> > > -2. Support shared storage.
> > > -3. Develop the heartbeat part.
> > > -4. Reduce checkpoint VM’s downtime while doing checkpoint.
> > > +1. Support shared storage.
> > > +2. Develop the heartbeat part.
> > > +3. Reduce checkpoint VM’s downtime while doing checkpoint.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 4/4] colo: Update Documentation for continuous replication
  2019-10-10 10:34       ` Zhang, Chen
@ 2019-10-11 16:00         ` Lukas Straub
  2019-10-11 17:39           ` Zhang, Chen
  0 siblings, 1 reply; 16+ messages in thread
From: Lukas Straub @ 2019-10-11 16:00 UTC (permalink / raw)
  To: Zhang, Chen
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, qemu-devel,
	Max Reitz, Xie Changlong

On Thu, 10 Oct 2019 10:34:15 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub <lukasstraub2@web.de>
> > Sent: Wednesday, October 9, 2019 11:17 PM
> > To: Zhang, Chen <chen.zhang@intel.com>
> > Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang
> > <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> > Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> > <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> > <qemu-block@nongnu.org>
> > Subject: Re: [PATCH v6 4/4] colo: Update Documentation for continuous
> > replication
> > 
> > On Wed, 9 Oct 2019 08:36:52 +0000
> > "Zhang, Chen" <chen.zhang@intel.com> wrote:
> >   
> > > > -----Original Message-----
> > > > From: Lukas Straub <lukasstraub2@web.de>
> > > > Sent: Saturday, October 5, 2019 9:06 PM
> > > > To: qemu-devel <qemu-devel@nongnu.org>
> > > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> > > > <jasowang@redhat.com>; Wen Congyang  
> > <wencongyang2@huawei.com>; Xie  
> > > > Changlong <xiechanglong.d@gmail.com>; Kevin Wolf  
> > <kwolf@redhat.com>;  
> > > > Max Reitz <mreitz@redhat.com>; qemu-block <qemu-  
> > block@nongnu.org>  
> > > > Subject: [PATCH v6 4/4] colo: Update Documentation for continuous
> > > > replication
> > > >
> > > > Document the qemu command-line and qmp commands for continuous
> > > > replication
> > > >
> > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > > > ---
> > > >  docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++----------
> > > >  docs/block-replication.txt |  28 +++--
> > > >  2 files changed, 174 insertions(+), 67 deletions(-)
> > > >
> > > > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > > > ad24680d13..bc1a0ccb99 100644
> > > > --- a/docs/COLO-FT.txt
> > > > +++ b/docs/COLO-FT.txt
> > > > @@ -145,35 +145,65 @@ The diagram just shows the main qmp  
> > command,  
> > > > you can get the detail  in test procedure.
> > > >
> > > > ...
> > > >
> > > > +Note: Here we are running both instances on the same Host for
> > > > +testing, change the IP Addresses if you want to run it on two
> > > > +Hosts. Initally
> > > > +127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
> > > > +
> > > > +== Startup qemu ==
> > > > +1. Primary:
> > > > +Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all  
> > Hosts.  
> > > > +# imagefolder="/mnt/vms/colo-test-primary"
> > > > +
> > > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -  
> > smp  
> > > > 1 -qmp stdio \
> > > > +   -device piix3-usb-uhci -device usb-tablet -name primary \
> > > > +   -netdev
> > > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > > \
> > > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > > +   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
> > > > +   -chardev socket,id=compare1,host=0.0.0.0,port=9004,server,wait \  
> > >
> > > We should change the host=127.0.0.1 consistent with the expression below.  
> > 
> > Hi,
> > This (and the IPs below in the QMP commands) needs to be this way,
> > because it's a listening port and with 127.0.0.1 it would only listen on the
> > loopback ip and wouldn't be reachable from another node for example. With
> > 0.0.0.0 it will listen on all Interfaces.  
> 
> Yes, I know.  For this command demo, maybe use 192.168.0.1/192.168.0.2 are more clear.

Hmm,
the compare0 and compare_out actually can be replaced by unix sockets. 
So what do you think about the following?

   -chardev socket,id=mirror0,host=127.0.0.1,port=9003,server,nowait \
   -chardev socket,id=compare1,host=127.0.0.1,port=9004,server,wait \
   -chardev socket,id=compare0,path=/tmp/compare0.sock,server,nowait \
   -chardev socket,id=compare0-0,path=/tmp/compare0.sock \
   -chardev socket,id=compare_out,path=/tmp/compare_out.sock,server,nowait \
   -chardev socket,id=compare_out0,path=/tmp/compare_out.sock \

> >   
> > > > +   -chardev socket,id=compare0,host=127.0.0.1,port=9001,server,nowait  
> > \  
> > > > +   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
> > > > +   -chardev
> > > > + socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait
> > > > \
> > > > +   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
> > > > +   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0 \
> > > > +   -object filter-
> > > > redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
> > > > +   -object filter-
> > > > redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
> > > > +   -object iothread,id=iothread1 \
> > > > +   -object
> > > > +colo-compare,id=comp0,primary_in=compare0-
> > > > 0,secondary_in=compare1,\
> > > > +outdev=compare_out0,iothread=iothread1 \
> > > > +   -drive
> > > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold
> > > > +=1,\
> > > > +children.0.file.filename=$imagefolder/primary.qcow2,children.0.driv
> > > > +er=q
> > > > +cow2 -S
> > > > +
> > > > +2. Secondary:
> > > > +# imagefolder="/mnt/vms/colo-test-secondary"
> > > > +# primary_ip=127.0.0.1
> > > > +
> > > > +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2 10G
> > > > +
> > > > +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2  
> > 10G  
> > > > +  
> > >
> > > The active disk and hidden disk just need create one time, we can note that  
> > here.
> > 
> > Ok, I will Note that. But I will wait until the block changes are reviewed
> > before sending the next version.  
> 
> That's fine for me.
> 
> Thanks
> Zhang Chen
> 
> > 
> > Regards,
> > Lukas Straub
> >   
> > > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -  
> > smp  
> > > > 1 -qmp stdio \
> > > > +   -device piix3-usb-uhci -device usb-tablet -name secondary \
> > > > +   -netdev
> > > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > > \
> > > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > > +   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1 \
> > > > +   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1 \
> > > > +   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
> > > > +   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1 \
> > > > +   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
> > > > +   -drive
> > > > if=none,id=parent0,file.filename=$imagefolder/primary.qcow2,driver=q
> > > > cow
> > > > 2 \
> > > > +   -drive
> > > > +if=none,id=childs0,driver=replication,mode=secondary,file.driver=qc
> > > > +ow2,
> > > > +\
> > > > +top-id=childs0,file.file.filename=$imagefolder/secondary-active.qco
> > > > +w2,\
> > > > +file.backing.driver=qcow2,file.backing.file.filename=$imagefolder/s
> > > > +econ
> > > > +dary-hidden.qcow2,\
> > > > +file.backing.backing=parent0 \
> > > > +   -drive
> > > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold
> > > > +=1,\
> > > > +children.0=childs0 \
> > > > +   -incoming tcp:0.0.0.0:9998
> > > > +
> > > > +
> > > > +3. On Secondary VM's QEMU monitor, issue command
> > > >  {'execute':'qmp_capabilities'}
> > > > -{ 'execute': 'nbd-server-start',
> > > > -  'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx', 'port':
> > > > '8889'} } } -}
> > > > -{'execute': 'nbd-server-add', 'arguments': {'device':
> > > > 'secondary-disk0',
> > > > 'writable': true } }
> > > > +{'execute': 'nbd-server-start', 'arguments': {'addr': {'type':
> > > > +'inet',
> > > > +'data': {'host': '0.0.0.0', 'port': '9999'} } } }
> > > > +{'execute': 'nbd-server-add', 'arguments': {'device': 'parent0',
> > > > +'writable': true } }
> > > >
> > > >  Note:
> > > >    a. The qmp command nbd-server-start and nbd-server-add must be
> > > > run @@ -182,44 +212,113 @@ Note:
> > > >       same.
> > > >    c. It is better to put active disk and hidden disk in ramdisk.
> > > >
> > > > -3. On Primary VM's QEMU monitor, issue command:
> > > > +4. On Primary VM's QEMU monitor, issue command:
> > > >  {'execute':'qmp_capabilities'}
> > > > -{ 'execute': 'human-monitor-command',
> > > > -  'arguments': {'command-line': 'drive_add -n buddy
> > > > driver=replication,mode=primary,file.driver=nbd,file.host=xx.xx.xx.x
> > > > x,file.p
> > > > ort=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
> > > > -{ 'execute':'x-blockdev-change', 'arguments':{'parent':
> > > > 'primary-disk0',
> > > > 'node': 'nbd_client0' } } -{ 'execute': 'migrate-set-capabilities',
> > > > -      'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
> > > > -{ 'execute': 'migrate', 'arguments': {'uri': 'tcp:xx.xx.xx.xx:8888'
> > > > } }
> > > > +{'execute': 'human-monitor-command', 'arguments': {'command-line':
> > > > +'drive_add -n buddy
> > > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2
> > > > +,fil e.port=9999,file.export=parent0,node-name=replication0'}}
> > > > +{'execute': 'x-blockdev-change', 'arguments':{'parent':
> > > > +'colo-disk0',
> > > > +'node': 'replication0' } }
> > > > +{'execute': 'migrate-set-capabilities', 'arguments':
> > > > +{'capabilities': [
> > > > +{'capability': 'x-colo', 'state': true } ] } }
> > > > +{'execute': 'migrate', 'arguments': {'uri': 'tcp:127.0.0.2:9998' }
> > > > +}
> > > >
> > > >    Note:
> > > >    a. There should be only one NBD Client for each primary disk.
> > > > -  b. xx.xx.xx.xx is the secondary physical machine's hostname or IP
> > > > -  c. The qmp command line must be run after running qmp command
> > > > line in
> > > > +  b. The qmp command line must be run after running qmp command
> > > > + line in
> > > >       secondary qemu.
> > > >
> > > > -4. After the above steps, you will see, whenever you make changes
> > > > to PVM, SVM will be synced.
> > > > +5. After the above steps, you will see, whenever you make changes
> > > > +to PVM,
> > > > SVM will be synced.
> > > >  You can issue command '{ "execute": "migrate-set-parameters" ,
> > > > "arguments":{ "x-checkpoint-delay": 2000 } }'
> > > > -to change the checkpoint period time
> > > > +to change the idle checkpoint period time
> > > > +
> > > > +6. Failover test
> > > > +You can kill one of the VMs and Failover on the surviving VM:
> > > > +
> > > > +If you killed the Secondary, then follow "Primary Failover". After
> > > > +that, if you want to resume the replication, follow "Primary resume
> > > > replication"
> > > > +
> > > > +If you killed the Primary, then follow "Secondary Failover". After
> > > > +that, if you want to resume the replication, follow "Secondary
> > > > +resume
> > > > replication"
> > > > +
> > > > +== Primary Failover ==
> > > > +The Secondary died, resume on the Primary
> > > > +
> > > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > > +'colo-disk0',
> > > > +'child': 'children.1'} }
> > > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > > > +'drive_del replication0' } }
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
> > > > +{'execute': 'x-colo-lost-heartbeat' }
> > > > +
> > > > +== Secondary Failover ==
> > > > +The Primary died, resume on the Secondary and prepare to become the
> > > > new
> > > > +Primary
> > > > +
> > > > +{'execute': 'nbd-server-stop'}
> > > > +{'execute': 'x-colo-lost-heartbeat'}
> > > > +
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
> > > > +{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
> > > > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
> > > > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
> > > > +
> > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend':
> > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > +'0.0.0.0', 'port': '9003' } }, 'server': true } } } }  
> > >
> > > Same like I said before.
> > >
> > > Others statement looks good for me.
> > >
> > > Thanks
> > > Zhang Chen
> > >  
> > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > +'0.0.0.0', 'port': '9004' } }, 'server': true } } } }
> > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend':
> > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > +'127.0.0.1', 'port': '9001' } }, 'server': true } } } }
> > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0-0', 'backend':
> > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > +'127.0.0.1', 'port': '9001' } }, 'server': false } } } }
> > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out',
> > > > +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> > > > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': true }
> > > > +} } }
> > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out0',
> > > > +'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet',
> > > > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server': false
> > > > +} } } }
> > > > +
> > > > +== Primary resume replication ==
> > > > +Resume replication after new Secondary is up.
> > > > +
> > > > +Start the new Secondary (Steps 2 and 3 above), then on the Primary:
> > > > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > > > +'job-id': 'resync', 'target': 'nbd://127.0.0.2:9999/parent0', 'mode':
> > > > +'existing', 'format': 'raw', 'sync': 'full'} }
> > > > +
> > > > +Wait until disk is synced, then:
> > > > +{'execute': 'stop'}
> > > > +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync'} }
> > > > +
> > > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > > > +'drive_add -n buddy
> > > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.2
> > > > +,fil e.port=9999,file.export=parent0,node-name=replication0'}}
> > > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > > +'colo-disk0',
> > > > +'node': 'replication0' } }
> > > > +
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > +'filter-mirror',
> > > > +'id': 'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev':
> > > > +'mirror0' } } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > +'filter-redirector', 'id': 'redire0', 'props': { 'netdev': 'hn0',
> > > > +'queue': 'rx', 'indev': 'compare_out' } } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > +'filter-redirector', 'id': 'redire1', 'props': { 'netdev': 'hn0',
> > > > +'queue': 'rx', 'outdev': 'compare0' } } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > > > +'iothread1' } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> > > > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > > > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> > > > +
> > > > +{'execute': 'migrate-set-capabilities', 'arguments':{
> > > > +'capabilities': [
> > > > +{'capability': 'x-colo', 'state': true } ] } }
> > > > +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.2:9998' }
> > > > +}
> > > > +
> > > > +Note:
> > > > +If this Primary previously was a Secondary, then we need to insert
> > > > +the filters before the filter-rewriter by using the
> > > > +"'insert': 'before', 'position': 'id=rew0'" Options. See below.
> > > > +
> > > > +== Secondary resume replication ==
> > > > +Become Primary and resume replication after new Secondary is up.
> > > > +Note that now 127.0.0.1 is the Secondary and 127.0.0.2 is the Primary.
> > > > +
> > > > +Start the new Secondary (Steps 2 and 3 above, but with
> > > > +primary_ip=127.0.0.2), then on the old Secondary:
> > > > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > > > +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> > > > +'existing', 'format': 'raw', 'sync': 'full'} }
> > > > +
> > > > +Wait until disk is synced, then:
> > > > +{'execute': 'stop'}
> > > > +{'execute': 'block-job-cancel', 'arguments':{ 'device': 'resync' }
> > > > +}
> > > >
> > > > -5. Failover test
> > > > -You can kill Primary VM and run 'x_colo_lost_heartbeat' in
> > > > Secondary VM's - monitor at the same time, then SVM will failover
> > > > and client will not detect this -change.
> > > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-line':
> > > > +'drive_add -n buddy
> > > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0.0.1
> > > > +,fil e.port=9999,file.export=parent0,node-name=replication0'}}
> > > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > > +'colo-disk0',
> > > > +'node': 'replication0' } }
> > > >
> > > > -Before issuing '{ "execute": "x-colo-lost-heartbeat" }' command, we
> > > > have to -issue block related command to stop block replication.
> > > > -Primary:
> > > > -  Remove the nbd child from the quorum:
> > > > -  { 'execute': 'x-blockdev-change', 'arguments': {'parent': 'colo-disk0',  
> > 'child':  
> > > > 'children.1'}}
> > > > -  { 'execute': 'human-monitor-command','arguments': {'command-line':
> > > > 'drive_del blk-buddy0'}}
> > > > -  Note: there is no qmp command to remove the blockdev now
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > +'filter-mirror',
> > > > +'id': 'm0', 'props': { 'insert': 'before', 'position': 'id=rew0',
> > > > +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > +'filter-redirector', 'id': 'redire0', 'props': { 'insert':
> > > > +'before',
> > > > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > > > +'compare_out' } } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > +'filter-redirector', 'id': 'redire1', 'props': { 'insert':
> > > > +'before',
> > > > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> > > > +'compare0' } } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > > > +'iothread1' } }
> > > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'colo-compare',
> > > > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > > > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' } } }
> > > >
> > > > -Secondary:
> > > > -  The primary host is down, so we should do the following thing:
> > > > -  { 'execute': 'nbd-server-stop' }
> > > > +{'execute': 'migrate-set-capabilities', 'arguments':{
> > > > +'capabilities': [
> > > > +{'capability': 'x-colo', 'state': true } ] } }
> > > > +{'execute': 'migrate', 'arguments':{ 'uri': 'tcp:127.0.0.1:9998' }
> > > > +}
> > > >
> > > >  == TODO ==
> > > > -1. Support continuous VM replication.
> > > > -2. Support shared storage.
> > > > -3. Develop the heartbeat part.
> > > > -4. Reduce checkpoint VM’s downtime while doing checkpoint.
> > > > +1. Support shared storage.
> > > > +2. Develop the heartbeat part.
> > > > +3. Reduce checkpoint VM’s downtime while doing checkpoint.  



^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 4/4] colo: Update Documentation for continuous replication
  2019-10-11 16:00         ` Lukas Straub
@ 2019-10-11 17:39           ` Zhang, Chen
  0 siblings, 0 replies; 16+ messages in thread
From: Zhang, Chen @ 2019-10-11 17:39 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, qemu-devel,
	Max Reitz, Xie Changlong


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Saturday, October 12, 2019 12:01 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang
> <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> <qemu-block@nongnu.org>
> Subject: Re: [PATCH v6 4/4] colo: Update Documentation for continuous
> replication
> 
> On Thu, 10 Oct 2019 10:34:15 +0000
> "Zhang, Chen" <chen.zhang@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukas Straub <lukasstraub2@web.de>
> > > Sent: Wednesday, October 9, 2019 11:17 PM
> > > To: Zhang, Chen <chen.zhang@intel.com>
> > > Cc: qemu-devel <qemu-devel@nongnu.org>; Jason Wang
> > > <jasowang@redhat.com>; Wen Congyang
> <wencongyang2@huawei.com>; Xie
> > > Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>;
> > > Max Reitz <mreitz@redhat.com>; qemu-block <qemu-
> block@nongnu.org>
> > > Subject: Re: [PATCH v6 4/4] colo: Update Documentation for
> > > continuous replication
> > >
> > > On Wed, 9 Oct 2019 08:36:52 +0000
> > > "Zhang, Chen" <chen.zhang@intel.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Lukas Straub <lukasstraub2@web.de>
> > > > > Sent: Saturday, October 5, 2019 9:06 PM
> > > > > To: qemu-devel <qemu-devel@nongnu.org>
> > > > > Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> > > > > <jasowang@redhat.com>; Wen Congyang
> > > <wencongyang2@huawei.com>; Xie
> > > > > Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> > > <kwolf@redhat.com>;
> > > > > Max Reitz <mreitz@redhat.com>; qemu-block <qemu-
> > > block@nongnu.org>
> > > > > Subject: [PATCH v6 4/4] colo: Update Documentation for
> > > > > continuous replication
> > > > >
> > > > > Document the qemu command-line and qmp commands for
> continuous
> > > > > replication
> > > > >
> > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > > > > ---
> > > > >  docs/COLO-FT.txt           | 213 +++++++++++++++++++++++++++------
> ----
> > > > >  docs/block-replication.txt |  28 +++--
> > > > >  2 files changed, 174 insertions(+), 67 deletions(-)
> > > > >
> > > > > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > > > > ad24680d13..bc1a0ccb99 100644
> > > > > --- a/docs/COLO-FT.txt
> > > > > +++ b/docs/COLO-FT.txt
> > > > > @@ -145,35 +145,65 @@ The diagram just shows the main qmp
> > > command,
> > > > > you can get the detail  in test procedure.
> > > > >
> > > > > ...
> > > > >
> > > > > +Note: Here we are running both instances on the same Host for
> > > > > +testing, change the IP Addresses if you want to run it on two
> > > > > +Hosts. Initally
> > > > > +127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.
> > > > > +
> > > > > +== Startup qemu ==
> > > > > +1. Primary:
> > > > > +Note: Initally, $imagefolder/primary.qcow2 needs to be copied
> > > > > +to all
> > > Hosts.
> > > > > +# imagefolder="/mnt/vms/colo-test-primary"
> > > > > +
> > > > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m
> 512 -
> > > smp
> > > > > 1 -qmp stdio \
> > > > > +   -device piix3-usb-uhci -device usb-tablet -name primary \
> > > > > +   -netdev
> > > > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > > > \
> > > > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > > > +   -chardev socket,id=mirror0,host=0.0.0.0,port=9003,server,nowait \
> > > > > +   -chardev
> > > > > + socket,id=compare1,host=0.0.0.0,port=9004,server,wait \
> > > >
> > > > We should change the host=127.0.0.1 consistent with the expression
> below.
> > >
> > > Hi,
> > > This (and the IPs below in the QMP commands) needs to be this way,
> > > because it's a listening port and with 127.0.0.1 it would only
> > > listen on the loopback ip and wouldn't be reachable from another
> > > node for example. With
> > > 0.0.0.0 it will listen on all Interfaces.
> >
> > Yes, I know.  For this command demo, maybe use 192.168.0.1/192.168.0.2
> are more clear.
> 
> Hmm,
> the compare0 and compare_out actually can be replaced by unix sockets.
> So what do you think about the following?
> 
>    -chardev socket,id=mirror0,host=127.0.0.1,port=9003,server,nowait \
>    -chardev socket,id=compare1,host=127.0.0.1,port=9004,server,wait \
>    -chardev socket,id=compare0,path=/tmp/compare0.sock,server,nowait \
>    -chardev socket,id=compare0-0,path=/tmp/compare0.sock \
>    -chardev
> socket,id=compare_out,path=/tmp/compare_out.sock,server,nowait \
>    -chardev socket,id=compare_out0,path=/tmp/compare_out.sock \

In this way, user must create the unix socket node before start COLO, it is very hard to use.
I re-considered the issue here, looks keep the 0.0.0.0 is a relatively reasonably choice.
We can add some note here like you said before, to make sure user know the means of 0.0.0.0. 

Thanks
Zhang Chen

> 
> > >
> > > > > +   -chardev
> > > > > + socket,id=compare0,host=127.0.0.1,port=9001,server,nowait
> > > \
> > > > > +   -chardev socket,id=compare0-0,host=127.0.0.1,port=9001 \
> > > > > +   -chardev
> > > > > + socket,id=compare_out,host=127.0.0.1,port=9005,server,nowait
> > > > > \
> > > > > +   -chardev socket,id=compare_out0,host=127.0.0.1,port=9005 \
> > > > > +   -object filter-mirror,id=m0,netdev=hn0,queue=tx,outdev=mirror0
> \
> > > > > +   -object filter-
> > > > > redirector,netdev=hn0,id=redire0,queue=rx,indev=compare_out \
> > > > > +   -object filter-
> > > > > redirector,netdev=hn0,id=redire1,queue=rx,outdev=compare0 \
> > > > > +   -object iothread,id=iothread1 \
> > > > > +   -object
> > > > > +colo-compare,id=comp0,primary_in=compare0-
> > > > > 0,secondary_in=compare1,\
> > > > > +outdev=compare_out0,iothread=iothread1 \
> > > > > +   -drive
> > > > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-thres
> > > > > +hold
> > > > > +=1,\
> > > > > +children.0.file.filename=$imagefolder/primary.qcow2,children.0.
> > > > > +driv
> > > > > +er=q
> > > > > +cow2 -S
> > > > > +
> > > > > +2. Secondary:
> > > > > +# imagefolder="/mnt/vms/colo-test-secondary"
> > > > > +# primary_ip=127.0.0.1
> > > > > +
> > > > > +# qemu-img create -f qcow2 $imagefolder/secondary-active.qcow2
> > > > > +10G
> > > > > +
> > > > > +# qemu-img create -f qcow2 $imagefolder/secondary-hidden.qcow2
> > > 10G
> > > > > +
> > > >
> > > > The active disk and hidden disk just need create one time, we can
> > > > note that
> > > here.
> > >
> > > Ok, I will Note that. But I will wait until the block changes are
> > > reviewed before sending the next version.
> >
> > That's fine for me.
> >
> > Thanks
> > Zhang Chen
> >
> > >
> > > Regards,
> > > Lukas Straub
> > >
> > > > > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m
> 512 -
> > > smp
> > > > > 1 -qmp stdio \
> > > > > +   -device piix3-usb-uhci -device usb-tablet -name secondary \
> > > > > +   -netdev
> > > > > + tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > > > > \
> > > > > +   -device rtl8139,id=e0,netdev=hn0 \
> > > > > +   -chardev socket,id=red0,host=$primary_ip,port=9003,reconnect=1
> \
> > > > > +   -chardev socket,id=red1,host=$primary_ip,port=9004,reconnect=1
> \
> > > > > +   -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0 \
> > > > > +   -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
> \
> > > > > +   -object filter-rewriter,id=rew0,netdev=hn0,queue=all \
> > > > > +   -drive
> > > > > if=none,id=parent0,file.filename=$imagefolder/primary.qcow2,driv
> > > > > er=q
> > > > > cow
> > > > > 2 \
> > > > > +   -drive
> > > > > +if=none,id=childs0,driver=replication,mode=secondary,file.drive
> > > > > +r=qc
> > > > > +ow2,
> > > > > +\
> > > > > +top-id=childs0,file.file.filename=$imagefolder/secondary-active
> > > > > +.qco
> > > > > +w2,\
> > > > > +file.backing.driver=qcow2,file.backing.file.filename=$imagefold
> > > > > +er/s
> > > > > +econ
> > > > > +dary-hidden.qcow2,\
> > > > > +file.backing.backing=parent0 \
> > > > > +   -drive
> > > > > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-thres
> > > > > +hold
> > > > > +=1,\
> > > > > +children.0=childs0 \
> > > > > +   -incoming tcp:0.0.0.0:9998
> > > > > +
> > > > > +
> > > > > +3. On Secondary VM's QEMU monitor, issue command
> > > > >  {'execute':'qmp_capabilities'}
> > > > > -{ 'execute': 'nbd-server-start',
> > > > > -  'arguments': {'addr': {'type': 'inet', 'data': {'host': 'xx.xx.xx.xx', 'port':
> > > > > '8889'} } } -}
> > > > > -{'execute': 'nbd-server-add', 'arguments': {'device':
> > > > > 'secondary-disk0',
> > > > > 'writable': true } }
> > > > > +{'execute': 'nbd-server-start', 'arguments': {'addr': {'type':
> > > > > +'inet',
> > > > > +'data': {'host': '0.0.0.0', 'port': '9999'} } } }
> > > > > +{'execute': 'nbd-server-add', 'arguments': {'device':
> > > > > +'parent0',
> > > > > +'writable': true } }
> > > > >
> > > > >  Note:
> > > > >    a. The qmp command nbd-server-start and nbd-server-add must
> > > > > be run @@ -182,44 +212,113 @@ Note:
> > > > >       same.
> > > > >    c. It is better to put active disk and hidden disk in ramdisk.
> > > > >
> > > > > -3. On Primary VM's QEMU monitor, issue command:
> > > > > +4. On Primary VM's QEMU monitor, issue command:
> > > > >  {'execute':'qmp_capabilities'}
> > > > > -{ 'execute': 'human-monitor-command',
> > > > > -  'arguments': {'command-line': 'drive_add -n buddy
> > > > > driver=replication,mode=primary,file.driver=nbd,file.host=xx.xx.
> > > > > xx.x
> > > > > x,file.p
> > > > > ort=8889,file.export=secondary-disk0,node-name=nbd_client0'}}
> > > > > -{ 'execute':'x-blockdev-change', 'arguments':{'parent':
> > > > > 'primary-disk0',
> > > > > 'node': 'nbd_client0' } } -{ 'execute': 'migrate-set-capabilities',
> > > > > -      'arguments': {'capabilities': [ {'capability': 'x-colo', 'state': true } ] } }
> > > > > -{ 'execute': 'migrate', 'arguments': {'uri': 'tcp:xx.xx.xx.xx:8888'
> > > > > } }
> > > > > +{'execute': 'human-monitor-command', 'arguments': {'command-
> line':
> > > > > +'drive_add -n buddy
> > > > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0
> > > > > +.0.2 ,fil
> > > > > +e.port=9999,file.export=parent0,node-name=replication0'}}
> > > > > +{'execute': 'x-blockdev-change', 'arguments':{'parent':
> > > > > +'colo-disk0',
> > > > > +'node': 'replication0' } }
> > > > > +{'execute': 'migrate-set-capabilities', 'arguments':
> > > > > +{'capabilities': [
> > > > > +{'capability': 'x-colo', 'state': true } ] } }
> > > > > +{'execute': 'migrate', 'arguments': {'uri':
> > > > > +'tcp:127.0.0.2:9998' } }
> > > > >
> > > > >    Note:
> > > > >    a. There should be only one NBD Client for each primary disk.
> > > > > -  b. xx.xx.xx.xx is the secondary physical machine's hostname
> > > > > or IP
> > > > > -  c. The qmp command line must be run after running qmp command
> > > > > line in
> > > > > +  b. The qmp command line must be run after running qmp
> command
> > > > > + line in
> > > > >       secondary qemu.
> > > > >
> > > > > -4. After the above steps, you will see, whenever you make
> > > > > changes to PVM, SVM will be synced.
> > > > > +5. After the above steps, you will see, whenever you make
> > > > > +changes to PVM,
> > > > > SVM will be synced.
> > > > >  You can issue command '{ "execute": "migrate-set-parameters" ,
> > > > > "arguments":{ "x-checkpoint-delay": 2000 } }'
> > > > > -to change the checkpoint period time
> > > > > +to change the idle checkpoint period time
> > > > > +
> > > > > +6. Failover test
> > > > > +You can kill one of the VMs and Failover on the surviving VM:
> > > > > +
> > > > > +If you killed the Secondary, then follow "Primary Failover".
> > > > > +After that, if you want to resume the replication, follow
> > > > > +"Primary resume
> > > > > replication"
> > > > > +
> > > > > +If you killed the Primary, then follow "Secondary Failover".
> > > > > +After that, if you want to resume the replication, follow
> > > > > +"Secondary resume
> > > > > replication"
> > > > > +
> > > > > +== Primary Failover ==
> > > > > +The Secondary died, resume on the Primary
> > > > > +
> > > > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > > > +'colo-disk0',
> > > > > +'child': 'children.1'} }
> > > > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-
> line':
> > > > > +'drive_del replication0' } }
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'comp0' } }
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'iothread1' } }
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'm0' } }
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'redire0' } }
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'redire1' } }
> > > > > +{'execute': 'x-colo-lost-heartbeat' }
> > > > > +
> > > > > +== Secondary Failover ==
> > > > > +The Primary died, resume on the Secondary and prepare to become
> > > > > +the
> > > > > new
> > > > > +Primary
> > > > > +
> > > > > +{'execute': 'nbd-server-stop'}
> > > > > +{'execute': 'x-colo-lost-heartbeat'}
> > > > > +
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'f2' } }
> > > > > +{'execute': 'object-del', 'arguments':{ 'id': 'f1' } }
> > > > > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red1' } }
> > > > > +{'execute': 'chardev-remove', 'arguments':{ 'id': 'red0' } }
> > > > > +
> > > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'mirror0', 'backend':
> > > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > > +'0.0.0.0', 'port': '9003' } }, 'server': true } } } }
> > > >
> > > > Same like I said before.
> > > >
> > > > Others statement looks good for me.
> > > >
> > > > Thanks
> > > > Zhang Chen
> > > >
> > > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> > > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > > +'0.0.0.0', 'port': '9004' } }, 'server': true } } } }
> > > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0', 'backend':
> > > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > > +'127.0.0.1', 'port': '9001' } }, 'server': true } } } }
> > > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare0-0', 'backend':
> > > > > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > > > > +'127.0.0.1', 'port': '9001' } }, 'server': false } } } }
> > > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out',
> > > > > +'backend': {'type': 'socket', 'data': {'addr': { 'type':
> > > > > +'inet',
> > > > > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server':
> > > > > +true } } } }
> > > > > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare_out0',
> > > > > +'backend': {'type': 'socket', 'data': {'addr': { 'type':
> > > > > +'inet',
> > > > > +'data': { 'host': '127.0.0.1', 'port': '9005' } }, 'server':
> > > > > +false } } } }
> > > > > +
> > > > > +== Primary resume replication == Resume replication after new
> > > > > +Secondary is up.
> > > > > +
> > > > > +Start the new Secondary (Steps 2 and 3 above), then on the Primary:
> > > > > +{'execute': 'drive-mirror', 'arguments':{ 'device':
> > > > > +'colo-disk0',
> > > > > +'job-id': 'resync', 'target': 'nbd://127.0.0.2:9999/parent0', 'mode':
> > > > > +'existing', 'format': 'raw', 'sync': 'full'} }
> > > > > +
> > > > > +Wait until disk is synced, then:
> > > > > +{'execute': 'stop'}
> > > > > +{'execute': 'block-job-cancel', 'arguments':{ 'device':
> > > > > +'resync'} }
> > > > > +
> > > > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-
> line':
> > > > > +'drive_add -n buddy
> > > > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0
> > > > > +.0.2 ,fil
> > > > > +e.port=9999,file.export=parent0,node-name=replication0'}}
> > > > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > > > +'colo-disk0',
> > > > > +'node': 'replication0' } }
> > > > > +
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'filter-mirror',
> > > > > +'id': 'm0', 'props': { 'netdev': 'hn0', 'queue': 'tx', 'outdev':
> > > > > +'mirror0' } } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'filter-redirector', 'id': 'redire0', 'props': { 'netdev':
> > > > > +'hn0',
> > > > > +'queue': 'rx', 'indev': 'compare_out' } } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'filter-redirector', 'id': 'redire1', 'props': { 'netdev':
> > > > > +'hn0',
> > > > > +'queue': 'rx', 'outdev': 'compare0' } } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > > > > +'iothread1' } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'colo-compare',
> > > > > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > > > > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' }
> > > > > +} }
> > > > > +
> > > > > +{'execute': 'migrate-set-capabilities', 'arguments':{
> > > > > +'capabilities': [
> > > > > +{'capability': 'x-colo', 'state': true } ] } }
> > > > > +{'execute': 'migrate', 'arguments':{ 'uri':
> > > > > +'tcp:127.0.0.2:9998' } }
> > > > > +
> > > > > +Note:
> > > > > +If this Primary previously was a Secondary, then we need to
> > > > > +insert the filters before the filter-rewriter by using the
> > > > > +"'insert': 'before', 'position': 'id=rew0'" Options. See below.
> > > > > +
> > > > > +== Secondary resume replication == Become Primary and resume
> > > > > +replication after new Secondary is up.
> > > > > +Note that now 127.0.0.1 is the Secondary and 127.0.0.2 is the Primary.
> > > > > +
> > > > > +Start the new Secondary (Steps 2 and 3 above, but with
> > > > > +primary_ip=127.0.0.2), then on the old Secondary:
> > > > > +{'execute': 'drive-mirror', 'arguments':{ 'device':
> > > > > +'colo-disk0',
> > > > > +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> > > > > +'existing', 'format': 'raw', 'sync': 'full'} }
> > > > > +
> > > > > +Wait until disk is synced, then:
> > > > > +{'execute': 'stop'}
> > > > > +{'execute': 'block-job-cancel', 'arguments':{ 'device':
> > > > > +'resync' } }
> > > > >
> > > > > -5. Failover test
> > > > > -You can kill Primary VM and run 'x_colo_lost_heartbeat' in
> > > > > Secondary VM's - monitor at the same time, then SVM will
> > > > > failover and client will not detect this -change.
> > > > > +{'execute': 'human-monitor-command', 'arguments':{ 'command-
> line':
> > > > > +'drive_add -n buddy
> > > > > +driver=replication,mode=primary,file.driver=nbd,file.host=127.0
> > > > > +.0.1 ,fil
> > > > > +e.port=9999,file.export=parent0,node-name=replication0'}}
> > > > > +{'execute': 'x-blockdev-change', 'arguments':{ 'parent':
> > > > > +'colo-disk0',
> > > > > +'node': 'replication0' } }
> > > > >
> > > > > -Before issuing '{ "execute": "x-colo-lost-heartbeat" }'
> > > > > command, we have to -issue block related command to stop block
> replication.
> > > > > -Primary:
> > > > > -  Remove the nbd child from the quorum:
> > > > > -  { 'execute': 'x-blockdev-change', 'arguments': {'parent':
> > > > > 'colo-disk0',
> > > 'child':
> > > > > 'children.1'}}
> > > > > -  { 'execute': 'human-monitor-command','arguments': {'command-
> line':
> > > > > 'drive_del blk-buddy0'}}
> > > > > -  Note: there is no qmp command to remove the blockdev now
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'filter-mirror',
> > > > > +'id': 'm0', 'props': { 'insert': 'before', 'position':
> > > > > +'id=rew0',
> > > > > +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'filter-redirector', 'id': 'redire0', 'props': { 'insert':
> > > > > +'before',
> > > > > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > > > > +'compare_out' } } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'filter-redirector', 'id': 'redire1', 'props': { 'insert':
> > > > > +'before',
> > > > > +'position': 'id=rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> > > > > +'compare0' } } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type': 'iothread', 'id':
> > > > > +'iothread1' } }
> > > > > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > > > > +'colo-compare',
> > > > > +'id': 'comp0', 'props': { 'primary_in': 'compare0-0', 'secondary_in':
> > > > > +'compare1', 'outdev': 'compare_out0', 'iothread': 'iothread1' }
> > > > > +} }
> > > > >
> > > > > -Secondary:
> > > > > -  The primary host is down, so we should do the following thing:
> > > > > -  { 'execute': 'nbd-server-stop' }
> > > > > +{'execute': 'migrate-set-capabilities', 'arguments':{
> > > > > +'capabilities': [
> > > > > +{'capability': 'x-colo', 'state': true } ] } }
> > > > > +{'execute': 'migrate', 'arguments':{ 'uri':
> > > > > +'tcp:127.0.0.1:9998' } }
> > > > >
> > > > >  == TODO ==
> > > > > -1. Support continuous VM replication.
> > > > > -2. Support shared storage.
> > > > > -3. Develop the heartbeat part.
> > > > > -4. Reduce checkpoint VM’s downtime while doing checkpoint.
> > > > > +1. Support shared storage.
> > > > > +2. Develop the heartbeat part.
> > > > > +3. Reduce checkpoint VM’s downtime while doing checkpoint.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 1/4] block/replication.c: Ignore requests after failover
  2019-10-05 13:05 ` [PATCH v6 1/4] block/replication.c: Ignore requests after failover Lukas Straub
@ 2019-10-18 18:46   ` Lukas Straub
  2019-10-23  8:13     ` Zhang, Chen
  2019-10-23 12:49   ` Max Reitz
  1 sibling, 1 reply; 16+ messages in thread
From: Lukas Straub @ 2019-10-18 18:46 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Zhang Chen, Xie Changlong

On Sat, 5 Oct 2019 15:05:23 +0200
Lukas Straub <lukasstraub2@web.de> wrote:

> After failover the Secondary side of replication shouldn't change state, because
> it now functions as our primary disk.
>
> In replication_start, replication_do_checkpoint, replication_stop, ignore
> the request if current state is BLOCK_REPLICATION_DONE (sucessful failover) or
> BLOCK_REPLICATION_FAILOVER (failover in progres i.e. currently merging active
> and hidden images into the base image).
>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> Reviewed-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  block/replication.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/block/replication.c b/block/replication.c
> index 3d4dedddfc..97cc65c0cf 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -454,6 +454,17 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>      aio_context_acquire(aio_context);
>      s = bs->opaque;
>
> +    if (s->stage == BLOCK_REPLICATION_DONE ||
> +        s->stage == BLOCK_REPLICATION_FAILOVER) {
> +        /*
> +         * This case happens when a secondary is promoted to primary.
> +         * Ignore the request because the secondary side of replication
> +         * doesn't have to do anything anymore.
> +         */
> +        aio_context_release(aio_context);
> +        return;
> +    }
> +
>      if (s->stage != BLOCK_REPLICATION_NONE) {
>          error_setg(errp, "Block replication is running or done");
>          aio_context_release(aio_context);
> @@ -529,8 +540,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>                     "Block device is in use by internal backup job");
>
>          top_bs = bdrv_lookup_bs(s->top_id, s->top_id, NULL);
> -        if (!top_bs || !bdrv_is_root_node(top_bs) ||
> -            !check_top_bs(top_bs, bs)) {
> +        if (!top_bs || !check_top_bs(top_bs, bs)) {
>              error_setg(errp, "No top_bs or it is invalid");
>              reopen_backing_file(bs, false, NULL);
>              aio_context_release(aio_context);
> @@ -577,6 +587,17 @@ static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
>      aio_context_acquire(aio_context);
>      s = bs->opaque;
>
> +    if (s->stage == BLOCK_REPLICATION_DONE ||
> +        s->stage == BLOCK_REPLICATION_FAILOVER) {
> +        /*
> +         * This case happens when a secondary was promoted to primary.
> +         * Ignore the request because the secondary side of replication
> +         * doesn't have to do anything anymore.
> +         */
> +        aio_context_release(aio_context);
> +        return;
> +    }
> +
>      if (s->mode == REPLICATION_MODE_SECONDARY) {
>          secondary_do_checkpoint(s, errp);
>      }
> @@ -593,7 +614,7 @@ static void replication_get_error(ReplicationState *rs, Error **errp)
>      aio_context_acquire(aio_context);
>      s = bs->opaque;
>
> -    if (s->stage != BLOCK_REPLICATION_RUNNING) {
> +    if (s->stage == BLOCK_REPLICATION_NONE) {
>          error_setg(errp, "Block replication is not running");
>          aio_context_release(aio_context);
>          return;
> @@ -635,6 +656,17 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp)
>      aio_context_acquire(aio_context);
>      s = bs->opaque;
>
> +    if (s->stage == BLOCK_REPLICATION_DONE ||
> +        s->stage == BLOCK_REPLICATION_FAILOVER) {
> +        /*
> +         * This case happens when a secondary was promoted to primary.
> +         * Ignore the request because the secondary side of replication
> +         * doesn't have to do anything anymore.
> +         */
> +        aio_context_release(aio_context);
> +        return;
> +    }
> +
>      if (s->stage != BLOCK_REPLICATION_RUNNING) {
>          error_setg(errp, "Block replication is not running");
>          aio_context_release(aio_context);

Hello Everyone,
Could the block people have a look at this patch?

Regards,
Lukas Straub


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH v6 1/4] block/replication.c: Ignore requests after failover
  2019-10-18 18:46   ` Lukas Straub
@ 2019-10-23  8:13     ` Zhang, Chen
  0 siblings, 0 replies; 16+ messages in thread
From: Zhang, Chen @ 2019-10-23  8:13 UTC (permalink / raw)
  To: Lukas Straub, qemu-devel, Dr. David Alan Gilbert
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Max Reitz,
	Xie Changlong


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Saturday, October 19, 2019 2:46 AM
> To: qemu-devel <qemu-devel@nongnu.org>
> Cc: Zhang, Chen <chen.zhang@intel.com>; Jason Wang
> <jasowang@redhat.com>; Wen Congyang <wencongyang2@huawei.com>;
> Xie Changlong <xiechanglong.d@gmail.com>; Kevin Wolf
> <kwolf@redhat.com>; Max Reitz <mreitz@redhat.com>; qemu-block
> <qemu-block@nongnu.org>
> Subject: Re: [PATCH v6 1/4] block/replication.c: Ignore requests after failover
> 
> On Sat, 5 Oct 2019 15:05:23 +0200
> Lukas Straub <lukasstraub2@web.de> wrote:
> 
> > After failover the Secondary side of replication shouldn't change
> > state, because it now functions as our primary disk.
> >
> > In replication_start, replication_do_checkpoint, replication_stop,
> > ignore the request if current state is BLOCK_REPLICATION_DONE
> > (sucessful failover) or BLOCK_REPLICATION_FAILOVER (failover in
> > progres i.e. currently merging active and hidden images into the base
> image).
> >
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > Reviewed-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  block/replication.c | 38 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 35 insertions(+), 3 deletions(-)
> >
> > diff --git a/block/replication.c b/block/replication.c index
> > 3d4dedddfc..97cc65c0cf 100644
> > --- a/block/replication.c
> > +++ b/block/replication.c
> > @@ -454,6 +454,17 @@ static void replication_start(ReplicationState *rs,
> ReplicationMode mode,
> >      aio_context_acquire(aio_context);
> >      s = bs->opaque;
> >
> > +    if (s->stage == BLOCK_REPLICATION_DONE ||
> > +        s->stage == BLOCK_REPLICATION_FAILOVER) {
> > +        /*
> > +         * This case happens when a secondary is promoted to primary.
> > +         * Ignore the request because the secondary side of replication
> > +         * doesn't have to do anything anymore.
> > +         */
> > +        aio_context_release(aio_context);
> > +        return;
> > +    }
> > +
> >      if (s->stage != BLOCK_REPLICATION_NONE) {
> >          error_setg(errp, "Block replication is running or done");
> >          aio_context_release(aio_context); @@ -529,8 +540,7 @@ static
> > void replication_start(ReplicationState *rs, ReplicationMode mode,
> >                     "Block device is in use by internal backup job");
> >
> >          top_bs = bdrv_lookup_bs(s->top_id, s->top_id, NULL);
> > -        if (!top_bs || !bdrv_is_root_node(top_bs) ||
> > -            !check_top_bs(top_bs, bs)) {
> > +        if (!top_bs || !check_top_bs(top_bs, bs)) {
> >              error_setg(errp, "No top_bs or it is invalid");
> >              reopen_backing_file(bs, false, NULL);
> >              aio_context_release(aio_context); @@ -577,6 +587,17 @@
> > static void replication_do_checkpoint(ReplicationState *rs, Error **errp)
> >      aio_context_acquire(aio_context);
> >      s = bs->opaque;
> >
> > +    if (s->stage == BLOCK_REPLICATION_DONE ||
> > +        s->stage == BLOCK_REPLICATION_FAILOVER) {
> > +        /*
> > +         * This case happens when a secondary was promoted to primary.
> > +         * Ignore the request because the secondary side of replication
> > +         * doesn't have to do anything anymore.
> > +         */
> > +        aio_context_release(aio_context);
> > +        return;
> > +    }
> > +
> >      if (s->mode == REPLICATION_MODE_SECONDARY) {
> >          secondary_do_checkpoint(s, errp);
> >      }
> > @@ -593,7 +614,7 @@ static void replication_get_error(ReplicationState
> *rs, Error **errp)
> >      aio_context_acquire(aio_context);
> >      s = bs->opaque;
> >
> > -    if (s->stage != BLOCK_REPLICATION_RUNNING) {
> > +    if (s->stage == BLOCK_REPLICATION_NONE) {
> >          error_setg(errp, "Block replication is not running");
> >          aio_context_release(aio_context);
> >          return;
> > @@ -635,6 +656,17 @@ static void replication_stop(ReplicationState *rs,
> bool failover, Error **errp)
> >      aio_context_acquire(aio_context);
> >      s = bs->opaque;
> >
> > +    if (s->stage == BLOCK_REPLICATION_DONE ||
> > +        s->stage == BLOCK_REPLICATION_FAILOVER) {
> > +        /*
> > +         * This case happens when a secondary was promoted to primary.
> > +         * Ignore the request because the secondary side of replication
> > +         * doesn't have to do anything anymore.
> > +         */
> > +        aio_context_release(aio_context);
> > +        return;
> > +    }
> > +
> >      if (s->stage != BLOCK_REPLICATION_RUNNING) {
> >          error_setg(errp, "Block replication is not running");
> >          aio_context_release(aio_context);
> 
> Hello Everyone,
> Could the block people have a look at this patch?

Add Dave, do you have time to review this series?

Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 1/4] block/replication.c: Ignore requests after failover
  2019-10-05 13:05 ` [PATCH v6 1/4] block/replication.c: Ignore requests after failover Lukas Straub
  2019-10-18 18:46   ` Lukas Straub
@ 2019-10-23 12:49   ` Max Reitz
  2019-10-23 18:07     ` Lukas Straub
  1 sibling, 1 reply; 16+ messages in thread
From: Max Reitz @ 2019-10-23 12:49 UTC (permalink / raw)
  To: Lukas Straub, qemu-devel
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, Zhang Chen,
	Xie Changlong


[-- Attachment #1.1: Type: text/plain, Size: 2583 bytes --]

On 05.10.19 15:05, Lukas Straub wrote:
> After failover the Secondary side of replication shouldn't change state, because
> it now functions as our primary disk.
> 
> In replication_start, replication_do_checkpoint, replication_stop, ignore
> the request if current state is BLOCK_REPLICATION_DONE (sucessful failover) or
> BLOCK_REPLICATION_FAILOVER (failover in progres i.e. currently merging active
> and hidden images into the base image).
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> Reviewed-by: Zhang Chen <chen.zhang@intel.com>
> ---
>  block/replication.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)

Disclaimer: I don’t know anything about the replication block driver.

> diff --git a/block/replication.c b/block/replication.c
> index 3d4dedddfc..97cc65c0cf 100644
> --- a/block/replication.c
> +++ b/block/replication.c

[...]

> @@ -529,8 +540,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
>                     "Block device is in use by internal backup job");
> 
>          top_bs = bdrv_lookup_bs(s->top_id, s->top_id, NULL);
> -        if (!top_bs || !bdrv_is_root_node(top_bs) ||
> -            !check_top_bs(top_bs, bs)) {
> +        if (!top_bs || !check_top_bs(top_bs, bs)) {

It appears to me that top_bs is only used to install op blockers.  It
seems reasonable to require a root node to be able to do so (because op
blockers are really only checked on a root node).
(And the commit message doesn’t tell why we’d want to drop the
is_root_node check here.)

Now OTOH I don’t know whether the replication driver needs an op blocker
at all or whether the permission system suffices...

I suppose the rest of this patch is not really about the block layer, so
I can’t really comment on it.  (It looks OK to me from a generic and
naïve standpoint, though.)

>              error_setg(errp, "No top_bs or it is invalid");
>              reopen_backing_file(bs, false, NULL);
>              aio_context_release(aio_context);

[...]

> @@ -593,7 +614,7 @@ static void replication_get_error(ReplicationState *rs, Error **errp)
>      aio_context_acquire(aio_context);
>      s = bs->opaque;
> 
> -    if (s->stage != BLOCK_REPLICATION_RUNNING) {
> +    if (s->stage == BLOCK_REPLICATION_NONE) {

Just one question out of curiosity, though: Is this a bug fix?

Max

>          error_setg(errp, "Block replication is not running");
>          aio_context_release(aio_context);
>          return;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v6 1/4] block/replication.c: Ignore requests after failover
  2019-10-23 12:49   ` Max Reitz
@ 2019-10-23 18:07     ` Lukas Straub
  0 siblings, 0 replies; 16+ messages in thread
From: Lukas Straub @ 2019-10-23 18:07 UTC (permalink / raw)
  To: Max Reitz
  Cc: Kevin Wolf, qemu-block, Wen Congyang, Jason Wang, qemu-devel,
	Zhang Chen, Xie Changlong

On Wed, 23 Oct 2019 14:49:29 +0200
Max Reitz <mreitz@redhat.com> wrote:

> On 05.10.19 15:05, Lukas Straub wrote:
> > After failover the Secondary side of replication shouldn't change state, because
> > it now functions as our primary disk.
> > 
> > In replication_start, replication_do_checkpoint, replication_stop, ignore
> > the request if current state is BLOCK_REPLICATION_DONE (sucessful failover) or
> > BLOCK_REPLICATION_FAILOVER (failover in progres i.e. currently merging active
> > and hidden images into the base image).
> > 
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > Reviewed-by: Zhang Chen <chen.zhang@intel.com>
> > ---
> >  block/replication.c | 38 +++++++++++++++++++++++++++++++++++---
> >  1 file changed, 35 insertions(+), 3 deletions(-)  
> 
> Disclaimer: I don’t know anything about the replication block driver.
> 
> > diff --git a/block/replication.c b/block/replication.c
> > index 3d4dedddfc..97cc65c0cf 100644
> > --- a/block/replication.c
> > +++ b/block/replication.c  
> 
> [...]
> 
> > @@ -529,8 +540,7 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
> >                     "Block device is in use by internal backup job");
> > 
> >          top_bs = bdrv_lookup_bs(s->top_id, s->top_id, NULL);
> > -        if (!top_bs || !bdrv_is_root_node(top_bs) ||
> > -            !check_top_bs(top_bs, bs)) {
> > +        if (!top_bs || !check_top_bs(top_bs, bs)) {  
> 
> It appears to me that top_bs is only used to install op blockers.  It
> seems reasonable to require a root node to be able to do so (because op
> blockers are really only checked on a root node).
> (And the commit message doesn’t tell why we’d want to drop the
> is_root_node check here.)
> 
> Now OTOH I don’t know whether the replication driver needs an op blocker
> at all or whether the permission system suffices...

Hi,
Now that I look at again, it actually works without this change, by passing
a correct top-id= parameter to the driver (I somehow overlooked that parameter). 
So I will revert this change in the next version.

> 
> I suppose the rest of this patch is not really about the block layer, so
> I can’t really comment on it.  (It looks OK to me from a generic and
> naïve standpoint, though.)
> 
> >              error_setg(errp, "No top_bs or it is invalid");
> >              reopen_backing_file(bs, false, NULL);
> >              aio_context_release(aio_context);  
> 
> [...]
> 
> > @@ -593,7 +614,7 @@ static void replication_get_error(ReplicationState *rs, Error **errp)
> >      aio_context_acquire(aio_context);
> >      s = bs->opaque;
> > 
> > -    if (s->stage != BLOCK_REPLICATION_RUNNING) {
> > +    if (s->stage == BLOCK_REPLICATION_NONE) {  
> 
> Just one question out of curiosity, though: Is this a bug fix?

No, It only applies to continuous replication, because colo will
check all replication nodes for errors before checkpointing. So
a secondary continuing replication would error out here, because
it is either in state BLOCK_REPLICATION_DONE or 
BLOCK_REPLICATION_FAILOVER.

> Max
> 
> >          error_setg(errp, "Block replication is not running");
> >          aio_context_release(aio_context);
> >          return;  
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-10-23 18:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05 13:05 [PATCH v6 0/4] colo: Add support for continuous replication Lukas Straub
2019-10-05 13:05 ` [PATCH v6 1/4] block/replication.c: Ignore requests after failover Lukas Straub
2019-10-18 18:46   ` Lukas Straub
2019-10-23  8:13     ` Zhang, Chen
2019-10-23 12:49   ` Max Reitz
2019-10-23 18:07     ` Lukas Straub
2019-10-05 13:05 ` [PATCH v6 2/4] tests/test-replication.c: Add test for for secondary node continuing replication Lukas Straub
2019-10-09  6:03   ` Zhang, Chen
2019-10-09 15:19     ` Lukas Straub
2019-10-05 13:05 ` [PATCH v6 3/4] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
2019-10-05 13:05 ` [PATCH v6 4/4] colo: Update Documentation for continuous replication Lukas Straub
2019-10-09  8:36   ` Zhang, Chen
2019-10-09 15:16     ` Lukas Straub
2019-10-10 10:34       ` Zhang, Chen
2019-10-11 16:00         ` Lukas Straub
2019-10-11 17:39           ` Zhang, Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).