qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
@ 2019-08-15 18:48 Lukas Straub
  2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 1/3] Replication: Ignore requests after failover Lukas Straub
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Lukas Straub @ 2019-08-15 18:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Zhang Chen, Jason Wang, Xie Changlong, Wen Congyang

Hello Everyone,
These Patches add support for continious replication to colo.
Please review.

Regards,
Lukas Straub

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

Lukas Straub (3):
  Replication: Ignore requests after failover
  net/filter.c: Add Options to insert filters anywhere in the filter
    list
  Update Documentation

 block/replication.c  |  38 ++++++++-
 docs/COLO-FT.txt     | 185 ++++++++++++++++++++++++++++++++-----------
 include/net/filter.h |   2 +
 net/filter.c         |  71 ++++++++++++++++-
 qemu-options.hx      |  10 +--
 5 files changed, 250 insertions(+), 56 deletions(-)

--
2.20.1


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

* [Qemu-devel] [PATCH v2 1/3] Replication: Ignore requests after failover
  2019-08-15 18:48 [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication Lukas Straub
@ 2019-08-15 18:48 ` Lukas Straub
  2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
  2019-08-15 18:49 ` [Qemu-devel] [PATCH v2 3/3] Update Documentation Lukas Straub
  2 siblings, 0 replies; 19+ messages in thread
From: Lukas Straub @ 2019-08-15 18:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Zhang Chen, Jason Wang, Xie Changlong, Wen Congyang

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>
---
 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] 19+ messages in thread

* [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-08-15 18:48 [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication Lukas Straub
  2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 1/3] Replication: Ignore requests after failover Lukas Straub
@ 2019-08-15 18:48 ` Lukas Straub
  2019-08-23  3:24   ` Zhang, Chen
  2019-08-15 18:49 ` [Qemu-devel] [PATCH v2 3/3] Update Documentation Lukas Straub
  2 siblings, 1 reply; 19+ messages in thread
From: Lukas Straub @ 2019-08-15 18:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Zhang Chen, Jason Wang, Xie Changlong, Wen Congyang

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 either "head", "tail" or the id of another filter.
insert should be either "before" or "after" to specify where to
insert the new filter relative to the one specified with position.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 include/net/filter.h |  2 ++
 net/filter.c         | 71 +++++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx      | 10 +++----
 3 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/include/net/filter.h b/include/net/filter.h
index 49da666ac0..355c178f75 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;
     QTAILQ_ENTRY(NetFilterState) next;
 };

diff --git a/net/filter.c b/net/filter.c
index 28d1930db7..309fd778df 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 ? g_strdup("before") : g_strdup("after");
+}
+
+static void netfilter_set_insert(Object *obj, const char *str, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    if (strcmp(str, "before") && strcmp(str, "after")) {
+        error_setg(errp, "Invalid value for netfilter insert, "
+                         "should be 'head' or 'tail'");
+        return;
+    }
+
+    nf->insert_before = !strcmp(str, "before");
+}
+
 static void netfilter_init(Object *obj)
 {
     NetFilterState *nf = NETFILTER(obj);

     nf->on = true;
+    nf->insert_before = 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,20 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
         return;
     }

+    if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) {
+        /* Search for the position to insert before/after */
+        Object *container;
+        Object *obj;
+
+        container = object_get_objects_root();
+        obj = object_resolve_path_component(container, nf->position);
+        if (!obj) {
+            error_setg(errp, "filter '%s' not found", nf->position);
+            return;
+        }
+        position = NETFILTER(obj);
+    }
+
     nf->netdev = ncs[0];

     if (nfc->setup) {
@@ -228,7 +285,18 @@ static void netfilter_complete(UserCreatable *uc, Error **errp)
             return;
         }
     }
-    QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
+
+    if (position) {
+        if (nf->insert_before) {
+            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 +313,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..f0a47a0746 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}][,insert=@var{after|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,11 @@ 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]
+@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}][,insert=@var{after|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}][,insert=@var{after|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 +4400,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}][,insert=@var{after|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 +4413,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}][,insert=@var{after|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] 19+ messages in thread

* [Qemu-devel] [PATCH v2 3/3] Update Documentation
  2019-08-15 18:48 [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication Lukas Straub
  2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 1/3] Replication: Ignore requests after failover Lukas Straub
  2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
@ 2019-08-15 18:49 ` Lukas Straub
  2019-08-16  6:14   ` Markus Armbruster
  2019-09-02 12:17   ` Zhang, Chen
  2 siblings, 2 replies; 19+ messages in thread
From: Lukas Straub @ 2019-08-15 18:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Zhang Chen, Jason Wang, Xie Changlong, Wen Congyang

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

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 docs/COLO-FT.txt | 185 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 138 insertions(+), 47 deletions(-)

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index ad24680d13..c08bfbd3a8 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -145,35 +145,64 @@ The diagram just shows the main qmp command, you can get the detail
 in test procedure.
 
 == Test procedure ==
+Note: Here we are running both instances on the same Machine for testing, 
+change the IP Addresses if you want to run it on two Hosts
+
 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
+# imagefolder="/mnt/vms/colo-test"
+
+# cp --reflink=auto $imagefolder/primary.qcow2 $imagefolder/primary-copy.qcow2
+
+# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp 1 -qmp stdio \
+   -vnc :0 -k de -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=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,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
+
 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
+# imagefolder="/mnt/vms/colo-test"
+
+# 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 \
+   -vnc :1 -k de -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=127.0.0.1,port=9003,reconnect=1 \
+   -chardev socket,id=red1,host=127.0.0.1,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-copy.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.file=childs0,children.0.driver=raw \
+   -incoming tcp:0:9998
+
 
 2. 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': '127.0.0.1', '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
@@ -184,12 +213,10 @@ Note:
 
 3. 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.1,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.1:9998' } }
 
   Note:
   a. There should be only one NBD Client for each primary disk.
@@ -199,27 +226,91 @@ Note:
 
 4. 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
 
 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.
+You can kill one of the VMs and Failover on the surviving VM: 
 
-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
+== Primary Failover ==
+The Secondary died, resume on the Primary
 
-Secondary:
-  The primary host is down, so we should do the following thing:
-  { 'execute': 'nbd-server-stop' }
+{'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': '127.0.0.1', 'port': '9003' } }, 'server': true } } } }
+{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend': {'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host': '127.0.0.1', '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, then:
+{'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'} }
+
+{'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' } }
+
+{'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.1: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': 'rew0'" Options. See below.
+
+== Secondary resume replication ==
+Become Primary and resume replication after new Secondary is up.
+
+Start the new Secondary, then:
+{'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' } }
+
+{'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' } }
+
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } } }
+{'execute': 'object-add', 'arguments':{ 'qom-type': 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before', 'position': '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' } } }
+
+{'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.
-- 
2.20.1


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

* Re: [Qemu-devel] [PATCH v2 3/3] Update Documentation
  2019-08-15 18:49 ` [Qemu-devel] [PATCH v2 3/3] Update Documentation Lukas Straub
@ 2019-08-16  6:14   ` Markus Armbruster
  2019-09-02 12:17   ` Zhang, Chen
  1 sibling, 0 replies; 19+ messages in thread
From: Markus Armbruster @ 2019-08-16  6:14 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Zhang Chen, Jason Wang, Xie Changlong, qemu-devel, Wen Congyang

Subject "Update Documentation" is too vague, because (1) it lacks a
subsystem prefix, and (2) it fails to hint at what it is updated for.
Suggest something like "colo: Update documentation for FIXME", or maybe
"docs/COLO-FT.txt: Update for FIXME", with a suitable replacement for
FIXME.


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

* Re: [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
@ 2019-08-23  3:24   ` Zhang, Chen
  2019-08-23  6:21     ` Lukas Straub
  0 siblings, 1 reply; 19+ messages in thread
From: Zhang, Chen @ 2019-08-23  3:24 UTC (permalink / raw)
  To: Lukas Straub, qemu-devel; +Cc: Wen Congyang, Jason Wang, Xie Changlong



> -----Original Message-----
> From: Lukas Straub [mailto:lukasstraub2@web.de]
> Sent: Friday, August 16, 2019 2:49 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>
> Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in
> the filter list
> 
> 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 either "head", "tail" or the id of another filter.
> insert should be either "before" or "after" to specify where to insert the new
> filter relative to the one specified with position.
> 

Hi Lukas,

It looks no need to add the "insert = xxx" for this operation.
For example:

We have 3 net-filters, the running order like that:

Fiter1   ---------->   Filter2 ------------> Filter3

If we want to add another filter between filter1 and filter2.
The "Position = head, insert = after" always seam with "position = filter2 id, insert = before". It seems the "insert" is a redundant args.
So I think it is enough with the "position", we can make the "insert" always equal "after" except the "head".


Thanks
Zhang Chen

> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>  include/net/filter.h |  2 ++
>  net/filter.c         | 71 +++++++++++++++++++++++++++++++++++++++++++-
>  qemu-options.hx      | 10 +++----
>  3 files changed, 77 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/filter.h b/include/net/filter.h index
> 49da666ac0..355c178f75 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;
>      QTAILQ_ENTRY(NetFilterState) next;
>  };
> 
> diff --git a/net/filter.c b/net/filter.c index 28d1930db7..309fd778df 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 ? g_strdup("before") : g_strdup("after");
> +}
> +
> +static void netfilter_set_insert(Object *obj, const char *str, Error
> +**errp) {
> +    NetFilterState *nf = NETFILTER(obj);
> +
> +    if (strcmp(str, "before") && strcmp(str, "after")) {
> +        error_setg(errp, "Invalid value for netfilter insert, "
> +                         "should be 'head' or 'tail'");
> +        return;
> +    }
> +
> +    nf->insert_before = !strcmp(str, "before"); }
> +
>  static void netfilter_init(Object *obj)  {
>      NetFilterState *nf = NETFILTER(obj);
> 
>      nf->on = true;
> +    nf->insert_before = 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,20 @@ static void netfilter_complete(UserCreatable *uc,
> Error **errp)
>          return;
>      }
> 
> +    if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) {
> +        /* Search for the position to insert before/after */
> +        Object *container;
> +        Object *obj;
> +
> +        container = object_get_objects_root();
> +        obj = object_resolve_path_component(container, nf->position);
> +        if (!obj) {
> +            error_setg(errp, "filter '%s' not found", nf->position);
> +            return;
> +        }
> +        position = NETFILTER(obj);
> +    }
> +
>      nf->netdev = ncs[0];
> 
>      if (nfc->setup) {
> @@ -228,7 +285,18 @@ static void netfilter_complete(UserCreatable *uc,
> Error **errp)
>              return;
>          }
>      }
> -    QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
> +
> +    if (position) {
> +        if (nf->insert_before) {
> +            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 +313,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..f0a47a0746
> 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|r
> x|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}][,
> +insert=@var{after|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,11 @@ 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=@v
> ar{all|rx|tx}[,vnet_hdr_support]
> +@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}][
> +,insert=@var{after|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{chardevi
> +d},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,pos
> +ition=@var{head|tail|id}][,insert=@var{after|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 +4400,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}][,insert=@var{after|be
> +fore}]
> 
>  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
> +4413,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}][,insert=@var{after|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	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-08-23  3:24   ` Zhang, Chen
@ 2019-08-23  6:21     ` Lukas Straub
  2019-09-02 11:43       ` Zhang, Chen
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Straub @ 2019-08-23  6:21 UTC (permalink / raw)
  To: Zhang, Chen; +Cc: Wen Congyang, Jason Wang, Xie Changlong, qemu-devel

On Fri, 23 Aug 2019 03:24:02 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > Sent: Friday, August 16, 2019 2:49 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>
> > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in
> > the filter list
> >
> > 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 either "head", "tail" or the id of another filter.
> > insert should be either "before" or "after" to specify where to insert the new
> > filter relative to the one specified with position.
> >
>
> Hi Lukas,
>
> It looks no need to add the "insert = xxx" for this operation.
> For example:
>
> We have 3 net-filters, the running order like that:
>
> Fiter1   ---------->   Filter2 ------------> Filter3
>
> If we want to add another filter between filter1 and filter2.
> The "Position = head, insert = after" always seam with "position = filter2 id, insert = before".

Hi Zhang,
The insert= parameter is ignored if position=head or tail. It always Inserts at the head (before Filter1) or the tail (after Filter3) of the List in these cases.

> It seems the "insert" is a redundant args.
> So I think it is enough with the "position", we can make the "insert" always equal "after" except the "head".

Yes, we still could do it without it, but its more convenient with the insert= parameter. For example our Case with inserting before the rewriter:

'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' }
'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' }
'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' }

You see directly that here 3 Filters are inserted before the rewriter.

would have to become:

'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', 'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' }
'filter-redirector', 'id': 'redire0', 'props': { 'position': 'm0', 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' }
'filter-redirector', 'id': 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' }

Which is less obvious.

Regards,
Lukas Straub

>
> Thanks
> Zhang Chen
>
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >  include/net/filter.h |  2 ++
> >  net/filter.c         | 71 +++++++++++++++++++++++++++++++++++++++++++-
> >  qemu-options.hx      | 10 +++----
> >  3 files changed, 77 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/filter.h b/include/net/filter.h index
> > 49da666ac0..355c178f75 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;
> >      QTAILQ_ENTRY(NetFilterState) next;
> >  };
> >
> > diff --git a/net/filter.c b/net/filter.c index 28d1930db7..309fd778df 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 ? g_strdup("before") : g_strdup("after");
> > +}
> > +
> > +static void netfilter_set_insert(Object *obj, const char *str, Error
> > +**errp) {
> > +    NetFilterState *nf = NETFILTER(obj);
> > +
> > +    if (strcmp(str, "before") && strcmp(str, "after")) {
> > +        error_setg(errp, "Invalid value for netfilter insert, "
> > +                         "should be 'head' or 'tail'");
> > +        return;
> > +    }
> > +
> > +    nf->insert_before = !strcmp(str, "before"); }
> > +
> >  static void netfilter_init(Object *obj)  {
> >      NetFilterState *nf = NETFILTER(obj);
> >
> >      nf->on = true;
> > +    nf->insert_before = 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,20 @@ static void netfilter_complete(UserCreatable *uc,
> > Error **errp)
> >          return;
> >      }
> >
> > +    if (strcmp(nf->position, "head") && strcmp(nf->position, "tail")) {
> > +        /* Search for the position to insert before/after */
> > +        Object *container;
> > +        Object *obj;
> > +
> > +        container = object_get_objects_root();
> > +        obj = object_resolve_path_component(container, nf->position);
> > +        if (!obj) {
> > +            error_setg(errp, "filter '%s' not found", nf->position);
> > +            return;
> > +        }
> > +        position = NETFILTER(obj);
> > +    }
> > +
> >      nf->netdev = ncs[0];
> >
> >      if (nfc->setup) {
> > @@ -228,7 +285,18 @@ static void netfilter_complete(UserCreatable *uc,
> > Error **errp)
> >              return;
> >          }
> >      }
> > -    QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
> > +
> > +    if (position) {
> > +        if (nf->insert_before) {
> > +            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 +313,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..f0a47a0746
> > 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|r
> > x|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}][,
> > +insert=@var{after|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,11 @@ 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=@v
> > ar{all|rx|tx}[,vnet_hdr_support]
> > +@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}][
> > +,insert=@var{after|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{chardevi
> > +d},outdev=@var{chardevid},queue=@var{all|rx|tx}[,vnet_hdr_support][,pos
> > +ition=@var{head|tail|id}][,insert=@var{after|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 +4400,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}][,insert=@var{after|be
> > +fore}]
> >
> >  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
> > +4413,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}][,insert=@var{after|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	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-08-23  6:21     ` Lukas Straub
@ 2019-09-02 11:43       ` Zhang, Chen
  2019-09-02 18:51         ` Lukas Straub
  0 siblings, 1 reply; 19+ messages in thread
From: Zhang, Chen @ 2019-09-02 11:43 UTC (permalink / raw)
  To: Lukas Straub; +Cc: Wen Congyang, Jason Wang, Xie Changlong, qemu-devel


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Friday, August 23, 2019 2:21 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>
> Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert filters
> anywhere in the filter list
> 
> On Fri, 23 Aug 2019 03:24:02 +0000
> "Zhang, Chen" <chen.zhang@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > > Sent: Friday, August 16, 2019 2:49 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>
> > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters
> > > anywhere in the filter list
> > >
> > > 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 either "head", "tail" or the id of another filter.
> > > insert should be either "before" or "after" to specify where to
> > > insert the new filter relative to the one specified with position.
> > >
> >
> > Hi Lukas,
> >
> > It looks no need to add the "insert = xxx" for this operation.
> > For example:
> >
> > We have 3 net-filters, the running order like that:
> >
> > Fiter1   ---------->   Filter2 ------------> Filter3
> >
> > If we want to add another filter between filter1 and filter2.
> > The "Position = head, insert = after" always seam with "position = filter2 id,
> insert = before".
> 
> Hi Zhang,
> The insert= parameter is ignored if position=head or tail. It always Inserts at
> the head (before Filter1) or the tail (after Filter3) of the List in these cases.
> 
> > It seems the "insert" is a redundant args.
> > So I think it is enough with the "position", we can make the "insert" always
> equal "after" except the "head".
> 
> Yes, we still could do it without it, but its more convenient with the insert=
> parameter. For example our Case with inserting before the rewriter:
> 
> 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev':
> 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props':
> { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before',
> 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' }
> 
> You see directly that here 3 Filters are inserted before the rewriter.
> 
> would have to become:
> 
> 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', 'netdev': 'hn0', 'queue': 'tx',
> 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': { 'position': 'm0',
> 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id':
> 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> 'compare0' }
> 
> Which is less obvious.

OK, It is fine for me.
But in the code have some other issues like that:

+
+static void netfilter_set_insert(Object *obj, const char *str, Error **errp)
+{
+    NetFilterState *nf = NETFILTER(obj);
+
+    if (strcmp(str, "before") && strcmp(str, "after")) {                                
+        error_setg(errp, "Invalid value for netfilter insert, " 
+                         "should be 'head' or 'tail'");                                  -------------------------------->>> I think you should change the "head/tail"  to "before/after".
+        return;
+    }
+
+    nf->insert_before = !strcmp(str, "before");
+}


And I think the "front/behind" is better than "before/after" in this status.
At the same time the name of the "insert_before" change to "front_flag" is better.


Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub
> 
> >
> > Thanks
> > Zhang Chen
> >
> > > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > > ---
> > >  include/net/filter.h |  2 ++
> > >  net/filter.c         | 71
> +++++++++++++++++++++++++++++++++++++++++++-
> > >  qemu-options.hx      | 10 +++----
> > >  3 files changed, 77 insertions(+), 6 deletions(-)



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

* Re: [Qemu-devel] [PATCH v2 3/3] Update Documentation
  2019-08-15 18:49 ` [Qemu-devel] [PATCH v2 3/3] Update Documentation Lukas Straub
  2019-08-16  6:14   ` Markus Armbruster
@ 2019-09-02 12:17   ` Zhang, Chen
  2019-09-02 21:24     ` Lukas Straub
  1 sibling, 1 reply; 19+ messages in thread
From: Zhang, Chen @ 2019-09-02 12:17 UTC (permalink / raw)
  To: Lukas Straub, qemu-devel; +Cc: Wen Congyang, Jason Wang, Xie Changlong

Hi Lukas,

Please address Markus's comments change this patch's name in next version.
And I wrote some comments behind, please check it.
Firstly, Please remove all the trailing whitespace in this patch.


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Friday, August 16, 2019 2:49 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>
> Subject: [PATCH v2 3/3] Update Documentation
> 
> Document the qemu command-line and qmp commands for continious
> replication
> 
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>  docs/COLO-FT.txt | 185 +++++++++++++++++++++++++++++++++++--------
> ----
>  1 file changed, 138 insertions(+), 47 deletions(-)
> 
> diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> ad24680d13..c08bfbd3a8 100644
> --- a/docs/COLO-FT.txt
> +++ b/docs/COLO-FT.txt
> @@ -145,35 +145,64 @@ The diagram just shows the main qmp command,
> you can get the detail  in test procedure.
> 
>  == Test procedure ==
> +Note: Here we are running both instances on the same Machine for
> +testing, change the IP Addresses if you want to run it on two Hosts
> +
>  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
> +# imagefolder="/mnt/vms/colo-test"
> +
> +# cp --reflink=auto $imagefolder/primary.qcow2
> +$imagefolder/primary-copy.qcow2

I think you can tell other people here that we need two same disk image before COLO startup.
The name "primary and primary-copy" will make reader very confused.

> +
> +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp
> 1 -qmp stdio \
> +   -vnc :0 -k de -device piix3-usb-uhci -device usb-tablet -name primary \

What's mean of the "-k de" ?

> +   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> \
> +   -device rtl8139,id=e0,netdev=hn0 \
> +   -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,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 \

Please keep the space style.

> +   -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
> +
>  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
> +# imagefolder="/mnt/vms/colo-test"
> +
> +# 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 \
> +   -vnc :1 -k de -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=127.0.0.1,port=9003,reconnect=1 \
> +   -chardev socket,id=red1,host=127.0.0.1,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-
> copy.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/secon
> +dary-hidden.qcow2,\
> +file.backing.backing=parent0 \

Please keep the space style.

> +   -drive
> +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
> +children.0.file=childs0,children.0.driver=raw \
> +   -incoming tcp:0:9998
> +

Here, you changed the original block design, can you explain the reasons why we need to do this?
Please draw a ASCII diagram to describe COLO block detail.

> 
>  2. 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': '127.0.0.1', '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
> @@ -184,12 +213,10 @@ Note:
> 
>  3. 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.1,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.1:9998' } }
> 
>    Note:
>    a. There should be only one NBD Client for each primary disk.
> @@ -199,27 +226,91 @@ Note:
> 
>  4. 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
> 
>  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.
> +You can kill one of the VMs and Failover on the surviving VM:
> 
> -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
> +== Primary Failover ==
> +The Secondary died, resume on the Primary
> 
> -Secondary:
> -  The primary host is down, so we should do the following thing:
> -  { 'execute': 'nbd-server-stop' }
> +{'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':
> +'127.0.0.1', 'port': '9003' } }, 'server': true } } } }
> +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> +'127.0.0.1', '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, then:
> +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> +'existing', 'format': 'raw', 'sync': 'full'} }

I think the original secondary node has stopped the nbd server in failover:
> +{'execute': 'nbd-server-stop'}
> +{'execute': 'x-colo-lost-heartbeat'}

Why you can connect to it?

And please add new Secondary node start up script here,
I think it is different with original secondary node script.
same issue in resume secondary.

> +
> +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.1,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.1: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': 'rew0'" Options. See below.
> +
> +== Secondary resume replication ==
> +Become Primary and resume replication after new Secondary is up.
> +
> +Start the new Secondary, then:
> +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> +'existing', 'format': 'raw', 'sync': 'full'} }

Same issue as primary resume.

I want to re-tested the resume function but got stuck here.

Thanks
Zhang Chen

> +
> +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.1,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': { 'insert': 'before', 'position': 'rew0',
> +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type':
> +'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before',
> +'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> +'compare_out' } } }
> +{'execute': 'object-add', 'arguments':{ 'qom-type':
> +'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before',
> +'position': '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' } } }
> +
> +{'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.
> --
> 2.20.1

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

* Re: [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-09-02 11:43       ` Zhang, Chen
@ 2019-09-02 18:51         ` Lukas Straub
  2019-09-03  3:32           ` Zhang, Chen
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Straub @ 2019-09-02 18:51 UTC (permalink / raw)
  To: Zhang, Chen; +Cc: Wen Congyang, Jason Wang, Xie Changlong, qemu-devel

On Mon, 2 Sep 2019 11:43:57 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub <lukasstraub2@web.de>
> > Sent: Friday, August 23, 2019 2:21 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>
> > Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert filters
> > anywhere in the filter list
> >
> > On Fri, 23 Aug 2019 03:24:02 +0000
> > "Zhang, Chen" <chen.zhang@intel.com> wrote:
> >
> > > > -----Original Message-----
> > > > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > > > Sent: Friday, August 16, 2019 2:49 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>
> > > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert filters
> > > > anywhere in the filter list
> > > >
> > > > 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 either "head", "tail" or the id of another filter.
> > > > insert should be either "before" or "after" to specify where to
> > > > insert the new filter relative to the one specified with position.
> > > >
> > >
> > > Hi Lukas,
> > >
> > > It looks no need to add the "insert = xxx" for this operation.
> > > For example:
> > >
> > > We have 3 net-filters, the running order like that:
> > >
> > > Fiter1   ---------->   Filter2 ------------> Filter3
> > >
> > > If we want to add another filter between filter1 and filter2.
> > > The "Position = head, insert = after" always seam with "position = filter2 id,
> > insert = before".
> >
> > Hi Zhang,
> > The insert= parameter is ignored if position=head or tail. It always Inserts at
> > the head (before Filter1) or the tail (after Filter3) of the List in these cases.
> >
> > > It seems the "insert" is a redundant args.
> > > So I think it is enough with the "position", we can make the "insert" always
> > equal "after" except the "head".
> >
> > Yes, we still could do it without it, but its more convenient with the insert=
> > parameter. For example our Case with inserting before the rewriter:
> >
> > 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0', 'netdev':
> > 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props':
> > { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before',
> > 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev': 'compare0' }
> >
> > You see directly that here 3 Filters are inserted before the rewriter.
> >
> > would have to become:
> >
> > 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head', 'netdev': 'hn0', 'queue': 'tx',
> > 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props': { 'position': 'm0',
> > 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id':
> > 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> > 'compare0' }
> >
> > Which is less obvious.
>
> OK, It is fine for me.
> But in the code have some other issues like that:
>
> +
> +static void netfilter_set_insert(Object *obj, const char *str, Error **errp)
> +{
> +    NetFilterState *nf = NETFILTER(obj);
> +
> +    if (strcmp(str, "before") && strcmp(str, "after")) {
> +        error_setg(errp, "Invalid value for netfilter insert, "
> +                         "should be 'head' or 'tail'");                                  -------------------------------->>> I think you should change the "head/tail"  to "before/after".

Oops, that was a typo.

> +        return;
> +    }
> +
> +    nf->insert_before = !strcmp(str, "before");
> +}
>
>
> And I think the "front/behind" is better than "before/after" in this status.
> At the same time the name of the "insert_before" change to "front_flag" is better.

What I like about the "before" is that it sounds more like an English sentence.
insert: before, position: rew0
vs.
insert: front, position: rew0

But I agree, "behind" is more clear than "after".

What do you think about "first/last" instead of "head/tail"?

Regards,
Lukas Straub

>
> Thanks
> Zhang Chen
>
> >
> > Regards,
> > Lukas Straub
> >
> > >
> > > Thanks
> > > Zhang Chen
> > >
> > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > > > ---
> > > >  include/net/filter.h |  2 ++
> > > >  net/filter.c         | 71
> > +++++++++++++++++++++++++++++++++++++++++++-
> > > >  qemu-options.hx      | 10 +++----
> > > >  3 files changed, 77 insertions(+), 6 deletions(-)
>



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

* Re: [Qemu-devel] [PATCH v2 3/3] Update Documentation
  2019-09-02 12:17   ` Zhang, Chen
@ 2019-09-02 21:24     ` Lukas Straub
  0 siblings, 0 replies; 19+ messages in thread
From: Lukas Straub @ 2019-09-02 21:24 UTC (permalink / raw)
  To: Zhang, Chen; +Cc: Wen Congyang, Jason Wang, Xie Changlong, qemu-devel

On Mon, 2 Sep 2019 12:17:43 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> Hi Lukas,
> 
> Please address Markus's comments change this patch's name in next version.
> And I wrote some comments behind, please check it.
> Firstly, Please remove all the trailing whitespace in this patch.
> 
> 
> > -----Original Message-----
> > From: Lukas Straub <lukasstraub2@web.de>
> > Sent: Friday, August 16, 2019 2:49 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>
> > Subject: [PATCH v2 3/3] Update Documentation
> > 
> > Document the qemu command-line and qmp commands for continious
> > replication
> > 
> > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > ---
> >  docs/COLO-FT.txt | 185 +++++++++++++++++++++++++++++++++++--------
> > ----
> >  1 file changed, 138 insertions(+), 47 deletions(-)
> > 
> > diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt index
> > ad24680d13..c08bfbd3a8 100644
> > --- a/docs/COLO-FT.txt
> > +++ b/docs/COLO-FT.txt
> > @@ -145,35 +145,64 @@ The diagram just shows the main qmp command,
> > you can get the detail  in test procedure.
> > 
> >  == Test procedure ==
> > +Note: Here we are running both instances on the same Machine for
> > +testing, change the IP Addresses if you want to run it on two Hosts
> > +
> >  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
> > +# imagefolder="/mnt/vms/colo-test"
> > +
> > +# cp --reflink=auto $imagefolder/primary.qcow2
> > +$imagefolder/primary-copy.qcow2  
> 
> I think you can tell other people here that we need two same disk image before COLO startup.
> The name "primary and primary-copy" will make reader very confused.
> 
> > +
> > +# qemu-system-x86_64 -enable-kvm -cpu qemu64,+kvmclock -m 512 -smp
> > 1 -qmp stdio \
> > +   -vnc :0 -k de -device piix3-usb-uhci -device usb-tablet -name primary \  
> 
> What's mean of the "-k de" ?
Hi,

Oops, thats for German Keyboard layout, will remove.

> > +   -netdev tap,id=hn0,vhost=off,helper=/usr/lib/qemu/qemu-bridge-helper
> > \
> > +   -device rtl8139,id=e0,netdev=hn0 \
> > +   -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,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 \  
> 
> Please keep the space style.
> 
> > +   -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
> > +
> >  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
> > +# imagefolder="/mnt/vms/colo-test"
> > +
> > +# 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 \
> > +   -vnc :1 -k de -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=127.0.0.1,port=9003,reconnect=1 \
> > +   -chardev socket,id=red1,host=127.0.0.1,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-
> > copy.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/secon
> > +dary-hidden.qcow2,\
> > +file.backing.backing=parent0 \  
> 
> Please keep the space style.

The Idea here is that this can simply be copy-pasted into a Terminal and it will just 
work without having to edit it manually.

> > +   -drive
> > +if=ide,id=colo-disk0,driver=quorum,read-pattern=fifo,vote-threshold=1,\
> > +children.0.file=childs0,children.0.driver=raw \
> > +   -incoming tcp:0:9998
> > +  
> 
> Here, you changed the original block design, can you explain the reasons why we need to do this?
> Please draw a ASCII diagram to describe COLO block detail.

will do.

> > 
> >  2. 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': '127.0.0.1', '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
> > @@ -184,12 +213,10 @@ Note:
> > 
> >  3. 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.1,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.1:9998' } }
> > 
> >    Note:
> >    a. There should be only one NBD Client for each primary disk.
> > @@ -199,27 +226,91 @@ Note:
> > 
> >  4. 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
> > 
> >  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.
> > +You can kill one of the VMs and Failover on the surviving VM:
> > 
> > -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
> > +== Primary Failover ==
> > +The Secondary died, resume on the Primary
> > 
> > -Secondary:
> > -  The primary host is down, so we should do the following thing:
> > -  { 'execute': 'nbd-server-stop' }
> > +{'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':
> > +'127.0.0.1', 'port': '9003' } }, 'server': true } } } }
> > +{'execute': 'chardev-add', 'arguments':{ 'id': 'compare1', 'backend':
> > +{'type': 'socket', 'data': {'addr': { 'type': 'inet', 'data': { 'host':
> > +'127.0.0.1', '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, then:
> > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> > +'existing', 'format': 'raw', 'sync': 'full'} }  
> 
> I think the original secondary node has stopped the nbd server in failover:
> > +{'execute': 'nbd-server-stop'}
> > +{'execute': 'x-colo-lost-heartbeat'}  
> 
> Why you can connect to it?

Oh, I meant it differently. You execute this only if you killed the Secondary.

Thats what I meant with "You can kill one of the VMs and Failover on 
the surviving VM" above.

If you killed the Secondary, then you start a new Secondary (using the Secondary
commandline above, start nbd server and add the disk to the nbd server ) 
then execute the commands here under "Primary resume replication". After that, 
if you want to resume the replication, follow "Primary resume replication"

I will try to clarify this.

> And please add new Secondary node start up script here,
> I think it is different with original secondary node script.
> same issue in resume secondary.

No, Just the initial Primary cmdline is different. The Secondary cmdline is
always the same as above. Except if you run both on the same node, then you
need to choose a different Image than primary-copy.qcow2, I will clarify this.

Regards,
Lukas Straub

> > +
> > +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.1,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.1: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': 'rew0'" Options. See below.
> > +
> > +== Secondary resume replication ==
> > +Become Primary and resume replication after new Secondary is up.
> > +
> > +Start the new Secondary, then:
> > +{'execute': 'drive-mirror', 'arguments':{ 'device': 'colo-disk0',
> > +'job-id': 'resync', 'target': 'nbd://127.0.0.1:9999/parent0', 'mode':
> > +'existing', 'format': 'raw', 'sync': 'full'} }  
> 
> Same issue as primary resume.
> 
> I want to re-tested the resume function but got stuck here.
> 
> Thanks
> Zhang Chen
> 
> > +
> > +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.1,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': { 'insert': 'before', 'position': 'rew0',
> > +'netdev': 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > +'filter-redirector', 'id': 'redire0', 'props': { 'insert': 'before',
> > +'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > +'compare_out' } } }
> > +{'execute': 'object-add', 'arguments':{ 'qom-type':
> > +'filter-redirector', 'id': 'redire1', 'props': { 'insert': 'before',
> > +'position': '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' } } }
> > +
> > +{'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.
> > --
> > 2.20.1  



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

* Re: [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list
  2019-09-02 18:51         ` Lukas Straub
@ 2019-09-03  3:32           ` Zhang, Chen
  0 siblings, 0 replies; 19+ messages in thread
From: Zhang, Chen @ 2019-09-03  3:32 UTC (permalink / raw)
  To: Lukas Straub; +Cc: Wen Congyang, Jason Wang, Xie Changlong, qemu-devel


> -----Original Message-----
> From: Lukas Straub <lukasstraub2@web.de>
> Sent: Tuesday, September 3, 2019 2:51 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>
> Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert filters
> anywhere in the filter list
> 
> On Mon, 2 Sep 2019 11:43:57 +0000
> "Zhang, Chen" <chen.zhang@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukas Straub <lukasstraub2@web.de>
> > > Sent: Friday, August 23, 2019 2:21 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>
> > > Subject: Re: [PATCH v2 2/3] net/filter.c: Add Options to insert
> > > filters anywhere in the filter list
> > >
> > > On Fri, 23 Aug 2019 03:24:02 +0000
> > > "Zhang, Chen" <chen.zhang@intel.com> wrote:
> > >
> > > > > -----Original Message-----
> > > > > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > > > > Sent: Friday, August 16, 2019 2:49 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>
> > > > > Subject: [PATCH v2 2/3] net/filter.c: Add Options to insert
> > > > > filters anywhere in the filter list
> > > > >
> > > > > 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 either "head", "tail" or the id of another filter.
> > > > > insert should be either "before" or "after" to specify where to
> > > > > insert the new filter relative to the one specified with position.
> > > > >
> > > >
> > > > Hi Lukas,
> > > >
> > > > It looks no need to add the "insert = xxx" for this operation.
> > > > For example:
> > > >
> > > > We have 3 net-filters, the running order like that:
> > > >
> > > > Fiter1   ---------->   Filter2 ------------> Filter3
> > > >
> > > > If we want to add another filter between filter1 and filter2.
> > > > The "Position = head, insert = after" always seam with "position =
> > > > filter2 id,
> > > insert = before".
> > >
> > > Hi Zhang,
> > > The insert= parameter is ignored if position=head or tail. It always
> > > Inserts at the head (before Filter1) or the tail (after Filter3) of the List in
> these cases.
> > >
> > > > It seems the "insert" is a redundant args.
> > > > So I think it is enough with the "position", we can make the
> > > > "insert" always
> > > equal "after" except the "head".
> > >
> > > Yes, we still could do it without it, but its more convenient with
> > > the insert= parameter. For example our Case with inserting before the
> rewriter:
> > >
> > > 'filter-mirror', 'id': 'm0', 'props': { 'insert': 'before', 'position': 'rew0',
> 'netdev':
> > > 'hn0', 'queue': 'tx', 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0',
> 'props':
> > > { 'insert': 'before', 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'indev':
> > > 'compare_out' } 'filter-redirector', 'id': 'redire1', 'props': {
> > > 'insert': 'before',
> > > 'position': 'rew0', 'netdev': 'hn0', 'queue': 'rx', 'outdev':
> > > 'compare0' }
> > >
> > > You see directly that here 3 Filters are inserted before the rewriter.
> > >
> > > would have to become:
> > >
> > > 'filter-mirror', 'id': 'm0', 'props': { 'position': 'head',
> > > 'netdev': 'hn0', 'queue': 'tx',
> > > 'outdev': 'mirror0' } 'filter-redirector', 'id': 'redire0', 'props':
> > > { 'position': 'm0',
> > > 'netdev': 'hn0', 'queue': 'rx', 'indev': 'compare_out' } 'filter-redirector', 'id':
> > > 'redire1', 'props': { 'position': 'redire0', 'netdev': 'hn0', 'queue': 'rx',
> 'outdev':
> > > 'compare0' }
> > >
> > > Which is less obvious.
> >
> > OK, It is fine for me.
> > But in the code have some other issues like that:
> >
> > +
> > +static void netfilter_set_insert(Object *obj, const char *str, Error
> > +**errp) {
> > +    NetFilterState *nf = NETFILTER(obj);
> > +
> > +    if (strcmp(str, "before") && strcmp(str, "after")) {
> > +        error_setg(errp, "Invalid value for netfilter insert, "
> > +                         "should be 'head' or 'tail'");                                  ----------------------
> ---------->>> I think you should change the "head/tail"  to "before/after".
> 
> Oops, that was a typo.
> 
> > +        return;
> > +    }
> > +
> > +    nf->insert_before = !strcmp(str, "before"); }
> >
> >
> > And I think the "front/behind" is better than "before/after" in this status.
> > At the same time the name of the "insert_before" change to "front_flag" is
> better.
> 
> What I like about the "before" is that it sounds more like an English sentence.
> insert: before, position: rew0
> vs.
> insert: front, position: rew0
> 
> But I agree, "behind" is more clear than "after".
> 
> What do you think about "first/last" instead of "head/tail"?

I'm not a English native speaker, but I think head/tail is enough same as the QTAILQ.

Thanks
Zhang Chen

> 
> Regards,
> Lukas Straub
> 
> >
> > Thanks
> > Zhang Chen
> >
> > >
> > > Regards,
> > > Lukas Straub
> > >
> > > >
> > > > Thanks
> > > > Zhang Chen
> > > >
> > > > > Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> > > > > ---
> > > > >  include/net/filter.h |  2 ++
> > > > >  net/filter.c         | 71
> > > +++++++++++++++++++++++++++++++++++++++++++-
> > > > >  qemu-options.hx      | 10 +++----
> > > > >  3 files changed, 77 insertions(+), 6 deletions(-)
> >



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

* Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
  2019-08-16 18:20       ` Lukas Straub
  2019-08-21  5:23         ` Zhang, Chen
@ 2019-08-21 17:34         ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-21 17:34 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Zhang, Chen, Jason Wang, Xie Changlong, qemu-devel, Wen Congyang

* Lukas Straub (lukasstraub2@web.de) wrote:
> On Fri, 16 Aug 2019 01:51:20 +0000
> "Zhang, Chen" <chen.zhang@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > > Sent: Friday, August 16, 2019 3:48 AM
> > > To: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > Cc: qemu-devel <qemu-devel@nongnu.org>; Zhang, Chen
> > > <chen.zhang@intel.com>; Jason Wang <jasowang@redhat.com>; Xie
> > > Changlong <xiechanglong.d@gmail.com>; Wen Congyang
> > > <wencongyang2@huawei.com>
> > > Subject: Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious
> > > replication
> > >
> > > On Thu, 15 Aug 2019 19:57:37 +0100
> > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > >
> > > > * Lukas Straub (lukasstraub2@web.de) wrote:
> > > > > Hello Everyone,
> > > > > These Patches add support for continious replication to colo.
> > > > > Please review.
> > > >
> > > >
> > > > OK, for those who haven't followed COLO for so long; 'continuous
> > > > replication' is when after the first primary fails, you can promote
> > > > the original secondary to a new primary and start replicating again;
> > > >
> > > > i.e. current COLO gives you
> > > >
> > > > p<->s
> > > >     <primary fails>
> > > >     s
> > > >
> > > > with your patches you can do
> > > >
> > > >     s becomes p2
> > > >     p2<->s2
> > > >
> > > > and you're back to being resilient again.
> > > >
> > > > Which is great; because that was always an important missing piece.
> > > >
> > > > Do you have some test scripts/setup for this - it would be great to
> > > > automate some testing.
> > >
> > > My Plan is to write a Pacemaker Resource Agent[1] for qemu-colo and then do
> > > some long-term testing in my small cluster here. Writing standalone tests using
> > > that Resource Agent should be easy, it just needs to be provided with the right
> > > arguments and environment Variables.

Could you update tests/test-replication.c to test the extra steps?

Dave

> > Thanks Dave's explanation.
> > It looks good for me and I will test this series in my side.
> >
> > Another question: Is "Pacemaker Resource Agent[1] "  like a heartbeat module?
> 
> It's a bit more than that. Pacemaker itself is an Cluster Resource Manager, you can think of it like sysvinit but for clusters. It controls where in the cluster Resources run, what state (master/slave) and what to do in case of a Node or Resource failure. Now Resources can be anything like SQL-Server, Webserver, VM, etc. and Pacemaker itself doesn't directly control them, that's the Job of the Resource Agents. So a Resource Agent is like an init-script, but cluster-aware with more actions like start, stop, monitor, promote (to master) or migrate-to.
> 
> > I have wrote an internal heartbeat module running on Qemu, it make COLO can detect fail and trigger failover automatically, no need external APP to call the QMP command "x-colo-lost-heartbeat". If you need it, I can send a RFC version recently.
> 
> Cool, this should be faster to failover than with Pacemaker.
> What is the plan with cases like Primary-failover, which need to issue multiple commands?
> 
> > Thanks
> > Zhang Chen
> > >
> > > Regards,
> > > Lukas Straub
> > >
> > > [1] https://github.com/ClusterLabs/resource-agents/blob/master/doc/dev-guides/ra-dev-guide.asc#what-is-a-resource-agent
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


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

* Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
  2019-08-16 18:20       ` Lukas Straub
@ 2019-08-21  5:23         ` Zhang, Chen
  2019-08-21 17:34         ` Dr. David Alan Gilbert
  1 sibling, 0 replies; 19+ messages in thread
From: Zhang, Chen @ 2019-08-21  5:23 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Wen Congyang, Jason Wang, Xie Changlong, Dr. David Alan Gilbert,
	qemu-devel



> -----Original Message-----
> From: Lukas Straub [mailto:lukasstraub2@web.de]
> Sent: Saturday, August 17, 2019 2:20 AM
> To: Zhang, Chen <chen.zhang@intel.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>; qemu-devel <qemu-
> devel@nongnu.org>; Jason Wang <jasowang@redhat.com>; Xie Changlong
> <xiechanglong.d@gmail.com>; Wen Congyang <wencongyang2@huawei.com>
> Subject: Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious
> replication
> 
> On Fri, 16 Aug 2019 01:51:20 +0000
> "Zhang, Chen" <chen.zhang@intel.com> wrote:
> 
> > > -----Original Message-----
> > > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > > Sent: Friday, August 16, 2019 3:48 AM
> > > To: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > > Cc: qemu-devel <qemu-devel@nongnu.org>; Zhang, Chen
> > > <chen.zhang@intel.com>; Jason Wang <jasowang@redhat.com>; Xie
> > > Changlong <xiechanglong.d@gmail.com>; Wen Congyang
> > > <wencongyang2@huawei.com>
> > > Subject: Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for
> > > continious replication
> > >
> > > On Thu, 15 Aug 2019 19:57:37 +0100
> > > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> > >
> > > > * Lukas Straub (lukasstraub2@web.de) wrote:
> > > > > Hello Everyone,
> > > > > These Patches add support for continious replication to colo.
> > > > > Please review.
> > > >
> > > >
> > > > OK, for those who haven't followed COLO for so long; 'continuous
> > > > replication' is when after the first primary fails, you can
> > > > promote the original secondary to a new primary and start
> > > > replicating again;
> > > >
> > > > i.e. current COLO gives you
> > > >
> > > > p<->s
> > > >     <primary fails>
> > > >     s
> > > >
> > > > with your patches you can do
> > > >
> > > >     s becomes p2
> > > >     p2<->s2
> > > >
> > > > and you're back to being resilient again.
> > > >
> > > > Which is great; because that was always an important missing piece.
> > > >
> > > > Do you have some test scripts/setup for this - it would be great
> > > > to automate some testing.
> > >
> > > My Plan is to write a Pacemaker Resource Agent[1] for qemu-colo and
> > > then do some long-term testing in my small cluster here. Writing
> > > standalone tests using that Resource Agent should be easy, it just
> > > needs to be provided with the right arguments and environment Variables.
> >
> > Thanks Dave's explanation.
> > It looks good for me and I will test this series in my side.
> >
> > Another question: Is "Pacemaker Resource Agent[1] "  like a heartbeat
> module?
> 
> It's a bit more than that. Pacemaker itself is an Cluster Resource Manager, you
> can think of it like sysvinit but for clusters. It controls where in the cluster
> Resources run, what state (master/slave) and what to do in case of a Node or
> Resource failure. Now Resources can be anything like SQL-Server, Webserver,
> VM, etc. and Pacemaker itself doesn't directly control them, that's the Job of
> the Resource Agents. So a Resource Agent is like an init-script, but cluster-
> aware with more actions like start, stop, monitor, promote (to master) or
> migrate-to.
> 
> > I have wrote an internal heartbeat module running on Qemu, it make COLO
> can detect fail and trigger failover automatically, no need external APP to call
> the QMP command "x-colo-lost-heartbeat". If you need it, I can send a RFC
> version recently.
> 
> Cool, this should be faster to failover than with Pacemaker.
> What is the plan with cases like Primary-failover, which need to issue multiple
> commands?

Yes, currently we need input some net filter delete command after primary-failover.
We need make a way to remove related net-filter and chardev automatically.
But for Pacemaker it isn't a problem, you can send related qmp command after the "x-lost-heart-beat". 

Thanks
Zhang Chen

> 
> > Thanks
> > Zhang Chen
> > >
> > > Regards,
> > > Lukas Straub
> > >
> > > [1]
> > > https://github.com/ClusterLabs/resource-agents/blob/master/doc/dev-g
> > > uides/ra-dev-guide.asc#what-is-a-resource-agent



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

* Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
  2019-08-16  1:51     ` Zhang, Chen
@ 2019-08-16 18:20       ` Lukas Straub
  2019-08-21  5:23         ` Zhang, Chen
  2019-08-21 17:34         ` Dr. David Alan Gilbert
  0 siblings, 2 replies; 19+ messages in thread
From: Lukas Straub @ 2019-08-16 18:20 UTC (permalink / raw)
  To: Zhang, Chen
  Cc: Wen Congyang, Jason Wang, Xie Changlong, Dr. David Alan Gilbert,
	qemu-devel

On Fri, 16 Aug 2019 01:51:20 +0000
"Zhang, Chen" <chen.zhang@intel.com> wrote:

> > -----Original Message-----
> > From: Lukas Straub [mailto:lukasstraub2@web.de]
> > Sent: Friday, August 16, 2019 3:48 AM
> > To: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Cc: qemu-devel <qemu-devel@nongnu.org>; Zhang, Chen
> > <chen.zhang@intel.com>; Jason Wang <jasowang@redhat.com>; Xie
> > Changlong <xiechanglong.d@gmail.com>; Wen Congyang
> > <wencongyang2@huawei.com>
> > Subject: Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious
> > replication
> >
> > On Thu, 15 Aug 2019 19:57:37 +0100
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> >
> > > * Lukas Straub (lukasstraub2@web.de) wrote:
> > > > Hello Everyone,
> > > > These Patches add support for continious replication to colo.
> > > > Please review.
> > >
> > >
> > > OK, for those who haven't followed COLO for so long; 'continuous
> > > replication' is when after the first primary fails, you can promote
> > > the original secondary to a new primary and start replicating again;
> > >
> > > i.e. current COLO gives you
> > >
> > > p<->s
> > >     <primary fails>
> > >     s
> > >
> > > with your patches you can do
> > >
> > >     s becomes p2
> > >     p2<->s2
> > >
> > > and you're back to being resilient again.
> > >
> > > Which is great; because that was always an important missing piece.
> > >
> > > Do you have some test scripts/setup for this - it would be great to
> > > automate some testing.
> >
> > My Plan is to write a Pacemaker Resource Agent[1] for qemu-colo and then do
> > some long-term testing in my small cluster here. Writing standalone tests using
> > that Resource Agent should be easy, it just needs to be provided with the right
> > arguments and environment Variables.
>
> Thanks Dave's explanation.
> It looks good for me and I will test this series in my side.
>
> Another question: Is "Pacemaker Resource Agent[1] "  like a heartbeat module?

It's a bit more than that. Pacemaker itself is an Cluster Resource Manager, you can think of it like sysvinit but for clusters. It controls where in the cluster Resources run, what state (master/slave) and what to do in case of a Node or Resource failure. Now Resources can be anything like SQL-Server, Webserver, VM, etc. and Pacemaker itself doesn't directly control them, that's the Job of the Resource Agents. So a Resource Agent is like an init-script, but cluster-aware with more actions like start, stop, monitor, promote (to master) or migrate-to.

> I have wrote an internal heartbeat module running on Qemu, it make COLO can detect fail and trigger failover automatically, no need external APP to call the QMP command "x-colo-lost-heartbeat". If you need it, I can send a RFC version recently.

Cool, this should be faster to failover than with Pacemaker.
What is the plan with cases like Primary-failover, which need to issue multiple commands?

> Thanks
> Zhang Chen
> >
> > Regards,
> > Lukas Straub
> >
> > [1] https://github.com/ClusterLabs/resource-agents/blob/master/doc/dev-guides/ra-dev-guide.asc#what-is-a-resource-agent



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

* Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
  2019-08-15 19:48   ` Lukas Straub
@ 2019-08-16  1:51     ` Zhang, Chen
  2019-08-16 18:20       ` Lukas Straub
  0 siblings, 1 reply; 19+ messages in thread
From: Zhang, Chen @ 2019-08-16  1:51 UTC (permalink / raw)
  To: Lukas Straub, Dr. David Alan Gilbert
  Cc: Wen Congyang, Jason Wang, Xie Changlong, qemu-devel



> -----Original Message-----
> From: Lukas Straub [mailto:lukasstraub2@web.de]
> Sent: Friday, August 16, 2019 3:48 AM
> To: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: qemu-devel <qemu-devel@nongnu.org>; Zhang, Chen
> <chen.zhang@intel.com>; Jason Wang <jasowang@redhat.com>; Xie
> Changlong <xiechanglong.d@gmail.com>; Wen Congyang
> <wencongyang2@huawei.com>
> Subject: Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious
> replication
> 
> On Thu, 15 Aug 2019 19:57:37 +0100
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> 
> > * Lukas Straub (lukasstraub2@web.de) wrote:
> > > Hello Everyone,
> > > These Patches add support for continious replication to colo.
> > > Please review.
> >
> >
> > OK, for those who haven't followed COLO for so long; 'continuous
> > replication' is when after the first primary fails, you can promote
> > the original secondary to a new primary and start replicating again;
> >
> > i.e. current COLO gives you
> >
> > p<->s
> >     <primary fails>
> >     s
> >
> > with your patches you can do
> >
> >     s becomes p2
> >     p2<->s2
> >
> > and you're back to being resilient again.
> >
> > Which is great; because that was always an important missing piece.
> >
> > Do you have some test scripts/setup for this - it would be great to
> > automate some testing.
> 
> My Plan is to write a Pacemaker Resource Agent[1] for qemu-colo and then do
> some long-term testing in my small cluster here. Writing standalone tests using
> that Resource Agent should be easy, it just needs to be provided with the right
> arguments and environment Variables.

Thanks Dave's explanation.
It looks good for me and I will test this series in my side.

Another question: Is "Pacemaker Resource Agent[1] "  like a heartbeat module?   I have wrote an internal heartbeat module running on Qemu, it make COLO can detect fail and trigger failover automatically, no need external APP to call the QMP command "x-colo-lost-heartbeat". If you need it, I can send a RFC version recently.

Thanks
Zhang Chen
> 
> Regards,
> Lukas Straub
> 
> [1] https://github.com/ClusterLabs/resource-agents/blob/master/doc/dev-
> guides/ra-dev-guide.asc#what-is-a-resource-agent


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

* Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
  2019-08-15 18:57 ` Dr. David Alan Gilbert
@ 2019-08-15 19:48   ` Lukas Straub
  2019-08-16  1:51     ` Zhang, Chen
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Straub @ 2019-08-15 19:48 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: Zhang Chen, Jason Wang, Xie Changlong, qemu-devel, Wen Congyang

On Thu, 15 Aug 2019 19:57:37 +0100
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:

> * Lukas Straub (lukasstraub2@web.de) wrote:
> > Hello Everyone,
> > These Patches add support for continious replication to colo.
> > Please review.
>
>
> OK, for those who haven't followed COLO for so long; 'continuous
> replication' is when after the first primary fails, you can promote the
> original secondary to a new primary and start replicating again;
>
> i.e. current COLO gives you
>
> p<->s
>     <primary fails>
>     s
>
> with your patches you can do
>
>     s becomes p2
>     p2<->s2
>
> and you're back to being resilient again.
>
> Which is great; because that was always an important missing piece.
>
> Do you have some test scripts/setup for this - it would be great
> to automate some testing.

My Plan is to write a Pacemaker Resource Agent[1] for qemu-colo and
then do some long-term testing in my small cluster here. Writing
standalone tests using that Resource Agent should be easy, it just needs
to be provided with the right arguments and environment Variables.

Regards,
Lukas Straub

[1] https://github.com/ClusterLabs/resource-agents/blob/master/doc/dev-guides/ra-dev-guide.asc#what-is-a-resource-agent


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

* Re: [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
  2019-08-15 18:08 [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication Lukas Straub
@ 2019-08-15 18:57 ` Dr. David Alan Gilbert
  2019-08-15 19:48   ` Lukas Straub
  0 siblings, 1 reply; 19+ messages in thread
From: Dr. David Alan Gilbert @ 2019-08-15 18:57 UTC (permalink / raw)
  To: Lukas Straub
  Cc: Zhang Chen, Jason Wang, Xie Changlong, qemu-devel, Wen Congyang

* Lukas Straub (lukasstraub2@web.de) wrote:
> Hello Everyone,
> These Patches add support for continious replication to colo.
> Please review.


OK, for those who haven't followed COLO for so long; 'continuous
replication' is when after the first primary fails, you can promote the 
original secondary to a new primary and start replicating again;

i.e. current COLO gives you

p<->s
    <primary fails>
    s

with your patches you can do

    s becomes p2
    p2<->s2

and you're back to being resilient again.

Which is great; because that was always an important missing piece.

Do you have some test scripts/setup for this - it would be great
to automate some testing.

Dave

> Regards,
> Lukas Straub
> 
> v2:
>  - fix email formating
>  - fix checkpatch.pl warnings
>  - fix patchew error
>  - clearer commit messages
> 
> Lukas Straub (3):
>   Replication: Ignore requests after failover
>   net/filter.c: Add Options to insert filters anywhere in the filter
>     list
>   Update Documentation
> 
>  block/replication.c  |  38 ++++++++-
>  docs/COLO-FT.txt     | 185 ++++++++++++++++++++++++++++++++-----------
>  include/net/filter.h |   2 +
>  net/filter.c         |  71 ++++++++++++++++-
>  qemu-options.hx      |  10 +--
>  5 files changed, 250 insertions(+), 56 deletions(-)
> 
> --
> 2.20.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


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

* [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication
@ 2019-08-15 18:08 Lukas Straub
  2019-08-15 18:57 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 19+ messages in thread
From: Lukas Straub @ 2019-08-15 18:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: Zhang Chen, Jason Wang, Xie Changlong, Wen Congyang

Hello Everyone,
These Patches add support for continious replication to colo.
Please review.

Regards,
Lukas Straub

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

Lukas Straub (3):
  Replication: Ignore requests after failover
  net/filter.c: Add Options to insert filters anywhere in the filter
    list
  Update Documentation

 block/replication.c  |  38 ++++++++-
 docs/COLO-FT.txt     | 185 ++++++++++++++++++++++++++++++++-----------
 include/net/filter.h |   2 +
 net/filter.c         |  71 ++++++++++++++++-
 qemu-options.hx      |  10 +--
 5 files changed, 250 insertions(+), 56 deletions(-)

--
2.20.1


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

end of thread, other threads:[~2019-09-03  3:33 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-15 18:48 [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication Lukas Straub
2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 1/3] Replication: Ignore requests after failover Lukas Straub
2019-08-15 18:48 ` [Qemu-devel] [PATCH v2 2/3] net/filter.c: Add Options to insert filters anywhere in the filter list Lukas Straub
2019-08-23  3:24   ` Zhang, Chen
2019-08-23  6:21     ` Lukas Straub
2019-09-02 11:43       ` Zhang, Chen
2019-09-02 18:51         ` Lukas Straub
2019-09-03  3:32           ` Zhang, Chen
2019-08-15 18:49 ` [Qemu-devel] [PATCH v2 3/3] Update Documentation Lukas Straub
2019-08-16  6:14   ` Markus Armbruster
2019-09-02 12:17   ` Zhang, Chen
2019-09-02 21:24     ` Lukas Straub
  -- strict thread matches above, loose matches on Subject: below --
2019-08-15 18:08 [Qemu-devel] [PATCH v2 0/3] colo: Add support for continious replication Lukas Straub
2019-08-15 18:57 ` Dr. David Alan Gilbert
2019-08-15 19:48   ` Lukas Straub
2019-08-16  1:51     ` Zhang, Chen
2019-08-16 18:20       ` Lukas Straub
2019-08-21  5:23         ` Zhang, Chen
2019-08-21 17:34         ` Dr. David Alan Gilbert

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).