[Qemu-devel] [PULL 00/17] Net patches

[Qemu-devel] [PULL 00/17] Net patches

[Jason Wang]

The following changes since commit 7d0e02405fc02a181319b1ab8681d2f72246b7c6:

  Merge remote-tracking branch 'remotes/vivier2/tags/trivial-patches-pull-request' into staging (2019-07-01 17:40:32 +0100)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to 0e8818f023616677416840d6ddc880db8de3c967:

  migration/colo.c: Add missed filter notify for Xen COLO. (2019-07-02 10:21:07 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Cédric Le Goater (1):
      ftgmac100: do not link to netdev

Dr. David Alan Gilbert (5):
      net/announce: Allow optional list of interfaces
      net/announce: Add HMP optional interface list
      net/announce: Add optional ID
      net/announce: Add HMP optional ID
      net/announce: Expand test for stopping self announce

Markus Armbruster (2):
      MAINTAINERS: Add qemu-bridge-helper.c to "Network device backends"
      qemu-bridge-helper: Document known shortcomings

Stefano Garzarella (4):
      net: fix assertion failure when ipv6-prefixlen is not a number
      net: avoid using variable length array in net_client_init()
      net: use g_strsplit() for parsing host address and port
      net: remove unused get_str_sep() function

Zhang Chen (5):
      COLO-compare: Add new parameter to communicate with remote colo-frame
      COLO-compare: Add remote notification chardev handler frame
      COLO-compare: Make the compare_chr_send() can send notification message.
      COLO-compare: Add colo-compare remote notify support
      migration/colo.c: Add missed filter notify for Xen COLO.

 MAINTAINERS             |   1 +
 hmp-commands.hx         |   7 ++-
 hw/net/ftgmac100.c      |   2 -
 hw/net/virtio-net.c     |   4 +-
 include/net/announce.h  |   8 ++-
 migration/colo.c        |   2 +
 monitor/hmp-cmds.c      |  41 ++++++++++++-
 net/announce.c          |  89 +++++++++++++++++++++++----
 net/colo-compare.c      | 155 ++++++++++++++++++++++++++++++++++++++++++------
 net/net.c               |  99 +++++++++++++++----------------
 net/trace-events        |   3 +-
 qapi/net.json           |  16 ++++-
 qemu-bridge-helper.c    |  12 +++-
 qemu-options.hx         |  33 ++++++++++-
 tests/virtio-net-test.c |  57 +++++++++++++++++-
 15 files changed, 430 insertions(+), 99 deletions(-)

[Qemu-devel] [PULL 01/17] MAINTAINERS: Add qemu-bridge-helper.c to "Network device backends"

[Jason Wang]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8206fc5..c7b0c2c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1934,6 +1934,7 @@ M: Jason Wang <jasowang@redhat.com>
 S: Maintained
 F: net/
 F: include/net/
+F: qemu-bridge-helper.c
 T: git https://github.com/jasowang/qemu.git net
 F: qapi/net.json
 
-- 
2.5.0

[Qemu-devel] [PULL 02/17] qemu-bridge-helper: Document known shortcomings

[Jason Wang]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 qemu-bridge-helper.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index f9940de..95624bc 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -10,7 +10,17 @@
  *
  * This work is licensed under the terms of the GNU GPL, version 2.  See
  * the COPYING file in the top-level directory.
- *
+ */
+
+/*
+ * Known shortcomings:
+ * - There is no manual page
+ * - The syntax of the ACL file is not documented anywhere
+ * - parse_acl_file() doesn't report fopen() failure properly, fails
+ *   to check ferror() after fgets() failure, arbitrarily truncates
+ *   long lines, handles whitespace inconsistently, error messages
+ *   don't point to the offending file and line, errors in included
+ *   files are reported, but otherwise ignored, ...
  */
 
 #include "qemu/osdep.h"
-- 
2.5.0

[Qemu-devel] [PULL 03/17] ftgmac100: do not link to netdev

[Jason Wang]

From: Cédric Le Goater <clg@kaod.org>

qdev_set_nic_properties() is already used in the Aspeed SoC level to
bind the ftgmac100 device to the netdev.

This is fixing support for multiple net devices.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/ftgmac100.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index eb76047..d2cded5 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -1017,8 +1017,6 @@ static void ftgmac100_realize(DeviceState *dev, Error **errp)
     sysbus_init_irq(sbd, &s->irq);
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
 
-    s->conf.peers.ncs[0] = nd_table[0].netdev;
-
     s->nic = qemu_new_nic(&net_ftgmac100_info, &s->conf,
                           object_get_typename(OBJECT(dev)), DEVICE(dev)->id,
                           s);
-- 
2.5.0

[Qemu-devel] [PULL 04/17] net: fix assertion failure when ipv6-prefixlen is not a number

[Jason Wang]

From: Stefano Garzarella <sgarzare@redhat.com>

If 'ipv6-prefixlen' is not a number, the current behaviour
produces an assertion failure:
    $ qemu-system-x86_64 -net user,ipv6-net=feca::0/a
    qemu-system-x86_64: qemu/util/qemu-option.c:1175: qemu_opts_foreach:
    Assertion `!errp || !*errp' failed.
    Aborted (core dumped)

This patch fixes it, jumping to the end of the function when
'ipv6-prefixlen' is not a number, and printing the more friendly
message:
    $ qemu-system-x86_64 -net user,ipv6-net=feca::0/a
    qemu-system-x86_64: Parameter 'ipv6-prefixlen' expects a number

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/net.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/net.c b/net/net.c
index 3e65c93..d21c2c7 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1136,11 +1136,11 @@ static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
 
                 if (err) {
                     error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                              "ipv6-prefix", "a number");
-                } else {
-                    qemu_opt_set_number(opts, "ipv6-prefixlen", len,
-                                        &error_abort);
+                               "ipv6-prefixlen", "a number");
+                    goto out;
                 }
+
+                qemu_opt_set_number(opts, "ipv6-prefixlen", len, &error_abort);
             }
             qemu_opt_unset(opts, "ipv6-net");
         }
@@ -1162,6 +1162,7 @@ static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
         qapi_free_NetLegacy(object);
     }
 
+out:
     error_propagate(errp, err);
     visit_free(v);
     return ret;
-- 
2.5.0

[Qemu-devel] [PULL 05/17] net: avoid using variable length array in net_client_init()

[Jason Wang]

From: Stefano Garzarella <sgarzare@redhat.com>

net_client_init() uses a variable length array to store the prefix
of 'ipv6-net' parameter (e.g. if ipv6-net=fec0::0/64, the prefix
is 'fec0::0').
This patch introduces g_strsplit() to split the 'ipv6-net' parameter,
so we can remove the variable length array.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/net.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/net/net.c b/net/net.c
index d21c2c7..76ba0b7 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1105,6 +1105,7 @@ static void show_netdevs(void)
 
 static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
 {
+    gchar **substrings = NULL;
     void *object = NULL;
     Error *err = NULL;
     int ret = -1;
@@ -1120,28 +1121,33 @@ static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
         const char *ip6_net = qemu_opt_get(opts, "ipv6-net");
 
         if (ip6_net) {
-            char buf[strlen(ip6_net) + 1];
+            char *prefix_addr;
+            unsigned long prefix_len = 64; /* Default 64bit prefix length. */
+
+            substrings = g_strsplit(ip6_net, "/", 2);
+            if (!substrings || !substrings[0]) {
+                error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "ipv6-net",
+                           "a valid IPv6 prefix");
+                goto out;
+            }
 
-            if (get_str_sep(buf, sizeof(buf), &ip6_net, '/') < 0) {
-                /* Default 64bit prefix length.  */
-                qemu_opt_set(opts, "ipv6-prefix", ip6_net, &error_abort);
-                qemu_opt_set_number(opts, "ipv6-prefixlen", 64, &error_abort);
-            } else {
+            prefix_addr = substrings[0];
+
+            if (substrings[1]) {
                 /* User-specified prefix length.  */
-                unsigned long len;
                 int err;
 
-                qemu_opt_set(opts, "ipv6-prefix", buf, &error_abort);
-                err = qemu_strtoul(ip6_net, NULL, 10, &len);
-
+                err = qemu_strtoul(substrings[1], NULL, 10, &prefix_len);
                 if (err) {
                     error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
                                "ipv6-prefixlen", "a number");
                     goto out;
                 }
-
-                qemu_opt_set_number(opts, "ipv6-prefixlen", len, &error_abort);
             }
+
+            qemu_opt_set(opts, "ipv6-prefix", prefix_addr, &error_abort);
+            qemu_opt_set_number(opts, "ipv6-prefixlen", prefix_len,
+                                &error_abort);
             qemu_opt_unset(opts, "ipv6-net");
         }
     }
@@ -1164,6 +1170,7 @@ static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
 
 out:
     error_propagate(errp, err);
+    g_strfreev(substrings);
     visit_free(v);
     return ret;
 }
-- 
2.5.0

[Qemu-devel] [PULL 06/17] net: use g_strsplit() for parsing host address and port

[Jason Wang]

From: Stefano Garzarella <sgarzare@redhat.com>

Use the glib function to split host address and port in
the parse_host_port() function.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/net.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/net/net.c b/net/net.c
index 76ba0b7..5ce3996 100644
--- a/net/net.c
+++ b/net/net.c
@@ -87,32 +87,39 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
 int parse_host_port(struct sockaddr_in *saddr, const char *str,
                     Error **errp)
 {
-    char buf[512];
+    gchar **substrings;
     struct hostent *he;
-    const char *p, *r;
-    int port;
+    const char *addr, *p, *r;
+    int port, ret = 0;
 
-    p = str;
-    if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) {
+    substrings = g_strsplit(str, ":", 2);
+    if (!substrings || !substrings[0] || !substrings[1]) {
         error_setg(errp, "host address '%s' doesn't contain ':' "
                    "separating host from port", str);
-        return -1;
+        ret = -1;
+        goto out;
     }
+
+    addr = substrings[0];
+    p = substrings[1];
+
     saddr->sin_family = AF_INET;
-    if (buf[0] == '\0') {
+    if (addr[0] == '\0') {
         saddr->sin_addr.s_addr = 0;
     } else {
-        if (qemu_isdigit(buf[0])) {
-            if (!inet_aton(buf, &saddr->sin_addr)) {
+        if (qemu_isdigit(addr[0])) {
+            if (!inet_aton(addr, &saddr->sin_addr)) {
                 error_setg(errp, "host address '%s' is not a valid "
-                           "IPv4 address", buf);
-                return -1;
+                           "IPv4 address", addr);
+                ret = -1;
+                goto out;
             }
         } else {
-            he = gethostbyname(buf);
+            he = gethostbyname(addr);
             if (he == NULL) {
-                error_setg(errp, "can't resolve host address '%s'", buf);
-                return - 1;
+                error_setg(errp, "can't resolve host address '%s'", addr);
+                ret = -1;
+                goto out;
             }
             saddr->sin_addr = *(struct in_addr *)he->h_addr;
         }
@@ -120,10 +127,14 @@ int parse_host_port(struct sockaddr_in *saddr, const char *str,
     port = strtol(p, (char **)&r, 0);
     if (r == p) {
         error_setg(errp, "port number '%s' is invalid", p);
-        return -1;
+        ret = -1;
+        goto out;
     }
     saddr->sin_port = htons(port);
-    return 0;
+
+out:
+    g_strfreev(substrings);
+    return ret;
 }
 
 char *qemu_mac_strdup_printf(const uint8_t *macaddr)
-- 
2.5.0

[Qemu-devel] [PULL 07/17] net: remove unused get_str_sep() function

[Jason Wang]

From: Stefano Garzarella <sgarzare@redhat.com>

Since the get_str_sep() function is no longer used in
net/net.c, we can remove it.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/net.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/net/net.c b/net/net.c
index 5ce3996..7d40982 100644
--- a/net/net.c
+++ b/net/net.c
@@ -64,26 +64,6 @@ static QTAILQ_HEAD(, NetClientState) net_clients;
 /***********************************************************/
 /* network device redirectors */
 
-static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
-{
-    const char *p, *p1;
-    int len;
-    p = *pp;
-    p1 = strchr(p, sep);
-    if (!p1)
-        return -1;
-    len = p1 - p;
-    p1++;
-    if (buf_size > 0) {
-        if (len > buf_size - 1)
-            len = buf_size - 1;
-        memcpy(buf, p, len);
-        buf[len] = '\0';
-    }
-    *pp = p1;
-    return 0;
-}
-
 int parse_host_port(struct sockaddr_in *saddr, const char *str,
                     Error **errp)
 {
-- 
2.5.0

[Qemu-devel] [PULL 08/17] net/announce: Allow optional list of interfaces

[Jason Wang]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Allow the caller to restrict the set of interfaces that announces are
sent on.  The default is still to send on all interfaces.

e.g.

  { "execute": "announce-self", "arguments": { "initial": 50, "max": 550, "rounds": 5, "step": 50, "interfaces": ["vn2", "vn1"] } }

This doesn't affect the behaviour of migraiton announcments.

Note: There's still only one timer for the qmp command, so that
performing an 'announce-self' on one list of interfaces followed
by another 'announce-self' on another list will stop the announces
on the existing set.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/net/announce.h |  2 +-
 net/announce.c         | 39 ++++++++++++++++++++++++++++++++-------
 net/trace-events       |  2 +-
 qapi/net.json          | 11 ++++++++---
 4 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/include/net/announce.h b/include/net/announce.h
index 04a035f..7734704 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -22,7 +22,7 @@ struct AnnounceTimer {
 /* Returns: update the timer to the next time point */
 int64_t qemu_announce_timer_step(AnnounceTimer *timer);
 
-/* Delete the underlying timer */
+/* Delete the underlying timer and other data */
 void qemu_announce_timer_del(AnnounceTimer *timer);
 
 /*
diff --git a/net/announce.c b/net/announce.c
index 91e9a6e..1ce42b5 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -38,6 +38,8 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
         timer_free(timer->tm);
         timer->tm = NULL;
     }
+    qapi_free_strList(timer->params.interfaces);
+    timer->params.interfaces = NULL;
 }
 
 /*
@@ -96,24 +98,47 @@ static int announce_self_create(uint8_t *buf,
 
 static void qemu_announce_self_iter(NICState *nic, void *opaque)
 {
+    AnnounceTimer *timer = opaque;
     uint8_t buf[60];
     int len;
+    bool skip;
+
+    if (timer->params.has_interfaces) {
+        strList *entry = timer->params.interfaces;
+        /* Skip unless we find our name in the requested list */
+        skip = true;
+
+        while (entry) {
+            if (!strcmp(entry->value, nic->ncs->name)) {
+                /* Found us */
+                skip = false;
+                break;
+            }
+            entry = entry->next;
+        }
+    } else {
+        skip = false;
+    }
+
+    trace_qemu_announce_self_iter(nic->ncs->name,
+                                  qemu_ether_ntoa(&nic->conf->macaddr), skip);
 
-    trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
-    len = announce_self_create(buf, nic->conf->macaddr.a);
+    if (!skip) {
+        len = announce_self_create(buf, nic->conf->macaddr.a);
 
-    qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
+        qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
 
-    /* if the NIC provides it's own announcement support, use it as well */
-    if (nic->ncs->info->announce) {
-        nic->ncs->info->announce(nic->ncs);
+        /* if the NIC provides it's own announcement support, use it as well */
+        if (nic->ncs->info->announce) {
+            nic->ncs->info->announce(nic->ncs);
+        }
     }
 }
 static void qemu_announce_self_once(void *opaque)
 {
     AnnounceTimer *timer = (AnnounceTimer *)opaque;
 
-    qemu_foreach_nic(qemu_announce_self_iter, NULL);
+    qemu_foreach_nic(qemu_announce_self_iter, timer);
 
     if (--timer->round) {
         qemu_announce_timer_step(timer);
diff --git a/net/trace-events b/net/trace-events
index a7937f3..875ef2a 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,7 +1,7 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
 # announce.c
-qemu_announce_self_iter(const char *mac) "%s"
+qemu_announce_self_iter(const char *name, const char *mac, int skip) "%s:%s skip: %d"
 
 # vhost-user.c
 vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
diff --git a/qapi/net.json b/qapi/net.json
index 5f7bff1..6f2cd4f 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -699,6 +699,9 @@
 #
 # @step: Delay increase (in ms) after each self-announcement attempt
 #
+# @interfaces: An optional list of interface names, which restricts the
+#        announcement to the listed interfaces. (Since 4.1)
+#
 # Since: 4.0
 ##
 
@@ -706,7 +709,8 @@
   'data': { 'initial': 'int',
             'max': 'int',
             'rounds': 'int',
-            'step': 'int' } }
+            'step': 'int',
+            '*interfaces': ['str'] } }
 
 ##
 # @announce-self:
@@ -718,9 +722,10 @@
 #
 # Example:
 #
-# -> { "execute": "announce-self"
+# -> { "execute": "announce-self",
 #      "arguments": {
-#          "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
+#          "initial": 50, "max": 550, "rounds": 10, "step": 50,
+#          "interfaces": ["vn2", "vn3"] } }
 # <- { "return": {} }
 #
 # Since: 4.0
-- 
2.5.0

[Qemu-devel] [PULL 09/17] net/announce: Add HMP optional interface list

[Jason Wang]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add the optional interface list to the HMP command.

i.e.

   All interfaces
        announce_self

   Just the named interfaces:
        announce_self vn1,vn2

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hmp-commands.hx    |  6 ++++--
 monitor/hmp-cmds.c | 38 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8b7aec3..d42c09f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -955,8 +955,8 @@ ETEXI
 
     {
         .name       = "announce_self",
-        .args_type  = "",
-        .params     = "",
+        .args_type  = "interfaces:s?",
+        .params     = "[interfaces]",
         .help       = "Trigger GARP/RARP announcements",
         .cmd        = hmp_announce_self,
     },
@@ -967,6 +967,8 @@ STEXI
 Trigger a round of GARP/RARP broadcasts; this is useful for explicitly updating the
 network infrastructure after a reconfiguration or some forms of migration.
 The timings of the round are set by the migration announce parameters.
+An optional comma separated @var{interfaces} list restricts the announce to the
+named set of interfaces.
 ETEXI
 
     {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index c283dde..0fce179 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -27,6 +27,7 @@
 #include "monitor/monitor-internal.h"
 #include "monitor/qdev.h"
 #include "qapi/error.h"
+#include "qapi/clone-visitor.h"
 #include "qapi/opts-visitor.h"
 #include "qapi/qapi-builtin-visit.h"
 #include "qapi/qapi-commands-block.h"
@@ -38,6 +39,7 @@
 #include "qapi/qapi-commands-run-state.h"
 #include "qapi/qapi-commands-tpm.h"
 #include "qapi/qapi-commands-ui.h"
+#include "qapi/qapi-visit-net.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/string-input-visitor.h"
@@ -67,6 +69,32 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
     }
 }
 
+/*
+ * Produce a strList from a comma separated list.
+ * A NULL or empty input string return NULL.
+ */
+static strList *strList_from_comma_list(const char *in)
+{
+    strList *res = NULL;
+    strList **hook = &res;
+
+    while (in && in[0]) {
+        char *comma = strchr(in, ',');
+        *hook = g_new0(strList, 1);
+
+        if (comma) {
+            (*hook)->value = g_strndup(in, comma - in);
+            in = comma + 1; /* skip the , */
+        } else {
+            (*hook)->value = g_strdup(in);
+            in = NULL;
+        }
+        hook = &(*hook)->next;
+    }
+
+    return res;
+}
+
 void hmp_info_name(Monitor *mon, const QDict *qdict)
 {
     NameInfo *info;
@@ -1631,7 +1659,15 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 
 void hmp_announce_self(Monitor *mon, const QDict *qdict)
 {
-    qmp_announce_self(migrate_announce_params(), NULL);
+    const char *interfaces_str = qdict_get_try_str(qdict, "interfaces");
+    AnnounceParameters *params = QAPI_CLONE(AnnounceParameters,
+                                            migrate_announce_params());
+
+    qapi_free_strList(params->interfaces);
+    params->interfaces = strList_from_comma_list(interfaces_str);
+    params->has_interfaces = params->interfaces != NULL;
+    qmp_announce_self(params, NULL);
+    qapi_free_AnnounceParameters(params);
 }
 
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
-- 
2.5.0

[Qemu-devel] [PULL 10/17] net/announce: Add optional ID

[Jason Wang]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Previously there was a single instance of the timer used by
monitor triggered announces, that's OK, but when combined with the
previous change that lets you have announces for subsets of interfaces
it's a bit restrictive if you want to do different things to different
interfaces.

Add an 'id' field to the announce, and maintain a list of the
timers based on id.

This allows you to for example:
    a) Start an announce going on interface eth0 for a long time
    b) Start an announce going on interface eth1 for a long time
    c) Kill the announce on eth0 while leaving eth1 going.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c    |  4 ++--
 include/net/announce.h |  8 ++++++--
 net/announce.c         | 52 +++++++++++++++++++++++++++++++++++++++++++-------
 net/trace-events       |  3 ++-
 qapi/net.json          |  9 +++++++--
 5 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c3f5fcc..b9e1cd7 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2360,7 +2360,7 @@ static int virtio_net_post_load_device(void *opaque, int version_id)
             timer_mod(n->announce_timer.tm,
                       qemu_clock_get_ms(n->announce_timer.type));
         } else {
-            qemu_announce_timer_del(&n->announce_timer);
+            qemu_announce_timer_del(&n->announce_timer, false);
         }
     }
 
@@ -2784,7 +2784,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
         virtio_net_del_queue(n, i);
     }
 
-    qemu_announce_timer_del(&n->announce_timer);
+    qemu_announce_timer_del(&n->announce_timer, false);
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_net_rsc_cleanup(n);
diff --git a/include/net/announce.h b/include/net/announce.h
index 7734704..3d90c83 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -22,8 +22,12 @@ struct AnnounceTimer {
 /* Returns: update the timer to the next time point */
 int64_t qemu_announce_timer_step(AnnounceTimer *timer);
 
-/* Delete the underlying timer and other data */
-void qemu_announce_timer_del(AnnounceTimer *timer);
+/*
+ * Delete the underlying timer and other data
+ * If 'free_named' true and the timer is a named timer, then remove
+ * it from the list of named timers and free the AnnounceTimer itself.
+ */
+void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named);
 
 /*
  * Under BQL/main thread
diff --git a/net/announce.c b/net/announce.c
index 1ce42b5..db90d3b 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -15,6 +15,8 @@
 #include "qapi/qapi-commands-net.h"
 #include "trace.h"
 
+static GData *named_timers;
+
 int64_t qemu_announce_timer_step(AnnounceTimer *timer)
 {
     int64_t step;
@@ -31,8 +33,13 @@ int64_t qemu_announce_timer_step(AnnounceTimer *timer)
     return step;
 }
 
-void qemu_announce_timer_del(AnnounceTimer *timer)
+/*
+ * If 'free_named' is true, then remove the timer from the list
+ * and free the timer itself.
+ */
+void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named)
 {
+    bool free_timer = false;
     if (timer->tm) {
         timer_del(timer->tm);
         timer_free(timer->tm);
@@ -40,6 +47,24 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
     }
     qapi_free_strList(timer->params.interfaces);
     timer->params.interfaces = NULL;
+    if (free_named && timer->params.has_id) {
+        AnnounceTimer *list_timer;
+        /*
+         * Sanity check: There should only be one timer on the list with
+         * the id.
+         */
+        list_timer = g_datalist_get_data(&named_timers, timer->params.id);
+        assert(timer == list_timer);
+        free_timer = true;
+        g_datalist_remove_data(&named_timers, timer->params.id);
+    }
+    trace_qemu_announce_timer_del(free_named, free_timer, timer->params.id);
+    g_free(timer->params.id);
+    timer->params.id = NULL;
+
+    if (free_timer) {
+        g_free(timer);
+    }
 }
 
 /*
@@ -56,7 +81,7 @@ void qemu_announce_timer_reset(AnnounceTimer *timer,
      * We're under the BQL, so the current timer can't
      * be firing, so we should be able to delete it.
      */
-    qemu_announce_timer_del(timer);
+    qemu_announce_timer_del(timer, false);
 
     QAPI_CLONE_MEMBERS(AnnounceParameters, &timer->params, params);
     timer->round = params->rounds;
@@ -120,7 +145,8 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
         skip = false;
     }
 
-    trace_qemu_announce_self_iter(nic->ncs->name,
+    trace_qemu_announce_self_iter(timer->params.has_id ? timer->params.id : "_",
+                                  nic->ncs->name,
                                   qemu_ether_ntoa(&nic->conf->macaddr), skip);
 
     if (!skip) {
@@ -143,7 +169,7 @@ static void qemu_announce_self_once(void *opaque)
     if (--timer->round) {
         qemu_announce_timer_step(timer);
     } else {
-        qemu_announce_timer_del(timer);
+        qemu_announce_timer_del(timer, true);
     }
 }
 
@@ -154,12 +180,24 @@ void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params)
     if (params->rounds) {
         qemu_announce_self_once(timer);
     } else {
-        qemu_announce_timer_del(timer);
+        qemu_announce_timer_del(timer, true);
     }
 }
 
 void qmp_announce_self(AnnounceParameters *params, Error **errp)
 {
-    static AnnounceTimer announce_timer;
-    qemu_announce_self(&announce_timer, params);
+    AnnounceTimer *named_timer;
+    if (!params->has_id) {
+        params->id = g_strdup("");
+        params->has_id = true;
+    }
+
+    named_timer = g_datalist_get_data(&named_timers, params->id);
+
+    if (!named_timer) {
+        named_timer = g_new0(AnnounceTimer, 1);
+        g_datalist_set_data(&named_timers, params->id, named_timer);
+    }
+
+    qemu_announce_self(named_timer, params);
 }
diff --git a/net/trace-events b/net/trace-events
index 875ef2a..ac57056 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,7 +1,8 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
 # announce.c
-qemu_announce_self_iter(const char *name, const char *mac, int skip) "%s:%s skip: %d"
+qemu_announce_self_iter(const char *id, const char *name, const char *mac, int skip) "%s:%s:%s skip: %d"
+qemu_announce_timer_del(bool free_named, bool free_timer, char *id) "free named: %d free timer: %d id: %s"
 
 # vhost-user.c
 vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
diff --git a/qapi/net.json b/qapi/net.json
index 6f2cd4f..728990f 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -702,6 +702,10 @@
 # @interfaces: An optional list of interface names, which restricts the
 #        announcement to the listed interfaces. (Since 4.1)
 #
+# @id: A name to be used to identify an instance of announce-timers
+#        and to allow it to modified later.  Not for use as
+#        part of the migration parameters. (Since 4.1)
+#
 # Since: 4.0
 ##
 
@@ -710,7 +714,8 @@
             'max': 'int',
             'rounds': 'int',
             'step': 'int',
-            '*interfaces': ['str'] } }
+            '*interfaces': ['str'],
+            '*id' : 'str' } }
 
 ##
 # @announce-self:
@@ -725,7 +730,7 @@
 # -> { "execute": "announce-self",
 #      "arguments": {
 #          "initial": 50, "max": 550, "rounds": 10, "step": 50,
-#          "interfaces": ["vn2", "vn3"] } }
+#          "interfaces": ["vn2", "vn3"], "id": "bob" } }
 # <- { "return": {} }
 #
 # Since: 4.0
-- 
2.5.0

[Qemu-devel] [PULL 11/17] net/announce: Add HMP optional ID

[Jason Wang]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add the optional ID to the HMP command.

e.g.
   # start an announce for a long time on eth1
   migrate_set_parameter announce-rounds 1000
   announce_self "eth1" e1

   # start an announce on eth2
   announce_self "eth2" e2

   # Change e1 to be announcing on eth1 and eth3
   announce_self "eth1,eth3" e1

   # Cancel e1
   migrate_set_parameter announce-rounds 0
   announce_self "" e1

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hmp-commands.hx    | 7 ++++---
 monitor/hmp-cmds.c | 3 +++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index d42c09f..bfa5681 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -955,8 +955,8 @@ ETEXI
 
     {
         .name       = "announce_self",
-        .args_type  = "interfaces:s?",
-        .params     = "[interfaces]",
+        .args_type  = "interfaces:s?,id:s?",
+        .params     = "[interfaces] [id]",
         .help       = "Trigger GARP/RARP announcements",
         .cmd        = hmp_announce_self,
     },
@@ -968,7 +968,8 @@ Trigger a round of GARP/RARP broadcasts; this is useful for explicitly updating
 network infrastructure after a reconfiguration or some forms of migration.
 The timings of the round are set by the migration announce parameters.
 An optional comma separated @var{interfaces} list restricts the announce to the
-named set of interfaces.
+named set of interfaces. An optional @var{id} can be used to start a separate announce
+timer and to change the parameters of it later.
 ETEXI
 
     {
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 0fce179..9de3538 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1660,12 +1660,15 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 void hmp_announce_self(Monitor *mon, const QDict *qdict)
 {
     const char *interfaces_str = qdict_get_try_str(qdict, "interfaces");
+    const char *id = qdict_get_try_str(qdict, "id");
     AnnounceParameters *params = QAPI_CLONE(AnnounceParameters,
                                             migrate_announce_params());
 
     qapi_free_strList(params->interfaces);
     params->interfaces = strList_from_comma_list(interfaces_str);
     params->has_interfaces = params->interfaces != NULL;
+    params->id = g_strdup(id);
+    params->has_id = !!params->id;
     qmp_announce_self(params, NULL);
     qapi_free_AnnounceParameters(params);
 }
-- 
2.5.0

[Qemu-devel] [PULL 12/17] net/announce: Expand test for stopping self announce

[Jason Wang]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Expand self-announce test to check we can stop an announce timer.
We set it up to send 300 packets, but after we receive
the first one we tell it to stop.

We error if:
   a) We receive more than 30 of the packets
   b) We're still receiving packets after a lot longer than the
      30 seconds should have arrived

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 tests/virtio-net-test.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 663cf7e..7aa9622 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -184,21 +184,72 @@ static void announce_self(void *obj, void *data, QGuestAllocator *t_alloc)
     QDict *rsp;
     int ret;
     uint16_t *proto = (uint16_t *)&buffer[12];
+    size_t total_received = 0;
+    uint64_t start, now, last_rxt, deadline;
 
+    /* Send a set of packets over a few second period */
     rsp = qmp("{ 'execute' : 'announce-self', "
                   " 'arguments': {"
-                      " 'initial': 50, 'max': 550,"
-                      " 'rounds': 10, 'step': 50 } }");
+                      " 'initial': 20, 'max': 100,"
+                      " 'rounds': 300, 'step': 10, 'id': 'bob' } }");
     assert(!qdict_haskey(rsp, "error"));
     qobject_unref(rsp);
 
-    /* Catch the packet and make sure it's a RARP */
+    /* Catch the first packet and make sure it's a RARP */
     ret = qemu_recv(sv[0], &len, sizeof(len), 0);
     g_assert_cmpint(ret, ==,  sizeof(len));
     len = ntohl(len);
 
     ret = qemu_recv(sv[0], buffer, len, 0);
     g_assert_cmpint(*proto, ==, htons(ETH_P_RARP));
+
+    /*
+     * Stop the announcment by settings rounds to 0 on the
+     * existing timer.
+     */
+    rsp = qmp("{ 'execute' : 'announce-self', "
+                  " 'arguments': {"
+                      " 'initial': 20, 'max': 100,"
+                      " 'rounds': 0, 'step': 10, 'id': 'bob' } }");
+    assert(!qdict_haskey(rsp, "error"));
+    qobject_unref(rsp);
+
+    /* Now make sure the packets stop */
+
+    /* Times are in us */
+    start = g_get_monotonic_time();
+    /* 30 packets, max gap 100ms, * 4 for wiggle */
+    deadline = start + 1000 * (100 * 30 * 4);
+    last_rxt = start;
+
+    while (true) {
+        int saved_err;
+        ret = qemu_recv(sv[0], buffer, 60, MSG_DONTWAIT);
+        saved_err = errno;
+        now = g_get_monotonic_time();
+        g_assert_cmpint(now, <, deadline);
+
+        if (ret >= 0) {
+            if (ret) {
+                last_rxt = now;
+            }
+            total_received += ret;
+
+            /* Check it's not spewing loads */
+            g_assert_cmpint(total_received, <, 60 * 30 * 2);
+        } else {
+            g_assert_cmpint(saved_err, ==, EAGAIN);
+
+            /* 400ms, i.e. 4 worst case gaps */
+            if ((now - last_rxt) > (1000 * 100 * 4)) {
+                /* Nothings arrived for a while - must have stopped */
+                break;
+            };
+
+            /* 100ms */
+            g_usleep(1000 * 100);
+        }
+    };
 }
 
 static void virtio_net_test_cleanup(void *sockets)
-- 
2.5.0

[Qemu-devel] [PULL 13/17] COLO-compare: Add new parameter to communicate with remote colo-frame

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

We add the "notify_dev=chardevID" parameter. After that colo-compare can connect with
remote(currently just for Xen, KVM-COLO didn't need it.) colo-frame through chardev socket,
it can notify remote(Xen) colo-frame to handle checkpoint event.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 21 +++++++++++++++++++++
 qemu-options.hx    | 33 ++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 103297b..10fae2b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -83,6 +83,7 @@ typedef struct CompareState {
     char *pri_indev;
     char *sec_indev;
     char *outdev;
+    char *notify_dev;
     CharBackend chr_pri_in;
     CharBackend chr_sec_in;
     CharBackend chr_out;
@@ -897,6 +898,21 @@ static void compare_set_vnet_hdr(Object *obj,
     s->vnet_hdr = value;
 }
 
+static char *compare_get_notify_dev(Object *obj, Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+
+    return g_strdup(s->notify_dev);
+}
+
+static void compare_set_notify_dev(Object *obj, const char *value, Error **errp)
+{
+    CompareState *s = COLO_COMPARE(obj);
+
+    g_free(s->notify_dev);
+    s->notify_dev = g_strdup(value);
+}
+
 static void compare_pri_rs_finalize(SocketReadState *pri_rs)
 {
     CompareState *s = container_of(pri_rs, CompareState, pri_rs);
@@ -1057,6 +1073,10 @@ static void colo_compare_init(Object *obj)
                             (Object **)&s->iothread,
                             object_property_allow_set_link,
                             OBJ_PROP_LINK_STRONG, NULL);
+    /* This parameter just for Xen COLO */
+    object_property_add_str(obj, "notify_dev",
+                            compare_get_notify_dev, compare_set_notify_dev,
+                            NULL);
 
     s->vnet_hdr = false;
     object_property_add_bool(obj, "vnet_hdr_support", compare_get_vnet_hdr,
@@ -1103,6 +1123,7 @@ static void colo_compare_finalize(Object *obj)
     g_free(s->pri_indev);
     g_free(s->sec_indev);
     g_free(s->outdev);
+    g_free(s->notify_dev);
 }
 
 static const TypeInfo colo_compare_info = {
diff --git a/qemu-options.hx b/qemu-options.hx
index 0d8beb4..c18b790 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4477,7 +4477,7 @@ Dump the network traffic on netdev @var{dev} to the file specified by
 The file format is libpcap, so it can be analyzed with tools such as tcpdump
 or Wireshark.
 
-@item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid},iothread=@var{id}[,vnet_hdr_support]
+@item -object colo-compare,id=@var{id},primary_in=@var{chardevid},secondary_in=@var{chardevid},outdev=@var{chardevid},iothread=@var{id}[,vnet_hdr_support][,notify_dev=@var{id}]
 
 Colo-compare gets packet from primary_in@var{chardevid} and secondary_in@var{chardevid}, than compare primary packet with
 secondary packet. If the packets are same, we will output primary
@@ -4486,11 +4486,15 @@ do checkpoint and send primary packet to outdev@var{chardevid}.
 In order to improve efficiency, we need to put the task of comparison
 in another thread. If it has the vnet_hdr_support flag, colo compare
 will send/recv packet with vnet_hdr_len.
+If you want to use Xen COLO, will need the notify_dev to notify Xen
+colo-frame to do checkpoint.
 
 we must use it with the help of filter-mirror and filter-redirector.
 
 @example
 
+KVM COLO
+
 primary:
 -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
 -device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
@@ -4514,6 +4518,33 @@ secondary:
 -object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
 -object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
 
+
+Xen COLO
+
+primary:
+-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown
+-device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66
+-chardev socket,id=mirror0,host=3.3.3.3,port=9003,server,nowait
+-chardev socket,id=compare1,host=3.3.3.3,port=9004,server,nowait
+-chardev socket,id=compare0,host=3.3.3.3,port=9001,server,nowait
+-chardev socket,id=compare0-0,host=3.3.3.3,port=9001
+-chardev socket,id=compare_out,host=3.3.3.3,port=9005,server,nowait
+-chardev socket,id=compare_out0,host=3.3.3.3,port=9005
+-chardev socket,id=notify_way,host=3.3.3.3,port=9009,server,nowait
+-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,notify_dev=nofity_way,iothread=iothread1
+
+secondary:
+-netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,down script=/etc/qemu-ifdown
+-device e1000,netdev=hn0,mac=52:a4:00:12:78:66
+-chardev socket,id=red0,host=3.3.3.3,port=9003
+-chardev socket,id=red1,host=3.3.3.3,port=9004
+-object filter-redirector,id=f1,netdev=hn0,queue=tx,indev=red0
+-object filter-redirector,id=f2,netdev=hn0,queue=rx,outdev=red1
+
 @end example
 
 If you want to know the detail of above command line, you can read
-- 
2.5.0

[Qemu-devel] [PULL 14/17] COLO-compare: Add remote notification chardev handler frame

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

Add chardev handler to send notification to remote(current from Xen) colo-frame.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 10fae2b..fda55d5 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -87,8 +87,10 @@ typedef struct CompareState {
     CharBackend chr_pri_in;
     CharBackend chr_sec_in;
     CharBackend chr_out;
+    CharBackend chr_notify_dev;
     SocketReadState pri_rs;
     SocketReadState sec_rs;
+    SocketReadState notify_rs;
     bool vnet_hdr;
 
     /*
@@ -745,6 +747,19 @@ static void compare_sec_chr_in(void *opaque, const uint8_t *buf, int size)
     }
 }
 
+static void compare_notify_chr(void *opaque, const uint8_t *buf, int size)
+{
+    CompareState *s = COLO_COMPARE(opaque);
+    int ret;
+
+    ret = net_fill_rstate(&s->notify_rs, buf, size);
+    if (ret == -1) {
+        qemu_chr_fe_set_handlers(&s->chr_notify_dev, NULL, NULL, NULL, NULL,
+                                 NULL, NULL, true);
+        error_report("colo-compare notify_dev error");
+    }
+}
+
 /*
  * Check old packet regularly so it can watch for any packets
  * that the secondary hasn't produced equivalents of.
@@ -832,6 +847,11 @@ static void colo_compare_iothread(CompareState *s)
     qemu_chr_fe_set_handlers(&s->chr_sec_in, compare_chr_can_read,
                              compare_sec_chr_in, NULL, NULL,
                              s, s->worker_context, true);
+    if (s->notify_dev) {
+        qemu_chr_fe_set_handlers(&s->chr_notify_dev, compare_chr_can_read,
+                                 compare_notify_chr, NULL, NULL,
+                                 s, s->worker_context, true);
+    }
 
     colo_compare_timer_init(s);
     s->event_bh = qemu_bh_new(colo_compare_handle_event, s);
@@ -943,6 +963,10 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs)
     }
 }
 
+static void compare_notify_rs_finalize(SocketReadState *notify_rs)
+{
+    /* Get Xen colo-frame's notify and handle the message */
+}
 
 /*
  * Return 0 is success.
@@ -1013,6 +1037,17 @@ static void colo_compare_complete(UserCreatable *uc, Error **errp)
     net_socket_rs_init(&s->pri_rs, compare_pri_rs_finalize, s->vnet_hdr);
     net_socket_rs_init(&s->sec_rs, compare_sec_rs_finalize, s->vnet_hdr);
 
+    /* Try to enable remote notify chardev, currently just for Xen COLO */
+    if (s->notify_dev) {
+        if (find_and_check_chardev(&chr, s->notify_dev, errp) ||
+            !qemu_chr_fe_init(&s->chr_notify_dev, chr, errp)) {
+            return;
+        }
+
+        net_socket_rs_init(&s->notify_rs, compare_notify_rs_finalize,
+                           s->vnet_hdr);
+    }
+
     QTAILQ_INSERT_TAIL(&net_compares, s, next);
 
     g_queue_init(&s->conn_list);
@@ -1091,6 +1126,10 @@ static void colo_compare_finalize(Object *obj)
     qemu_chr_fe_deinit(&s->chr_pri_in, false);
     qemu_chr_fe_deinit(&s->chr_sec_in, false);
     qemu_chr_fe_deinit(&s->chr_out, false);
+    if (s->notify_dev) {
+        qemu_chr_fe_deinit(&s->chr_notify_dev, false);
+    }
+
     if (s->iothread) {
         colo_compare_timer_del(s);
     }
-- 
2.5.0

[Qemu-devel] [PULL 15/17] COLO-compare: Make the compare_chr_send() can send notification message.

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

We need use this function to send notification message for remote colo-frame(Xen).
So we add new parameter for this job.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index fda55d5..ee03c25 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -129,7 +129,8 @@ static void colo_compare_inconsistency_notify(void)
 static int compare_chr_send(CompareState *s,
                             const uint8_t *buf,
                             uint32_t size,
-                            uint32_t vnet_hdr_len);
+                            uint32_t vnet_hdr_len,
+                            bool notify_remote_frame);
 
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
@@ -241,7 +242,8 @@ static void colo_release_primary_pkt(CompareState *s, Packet *pkt)
     ret = compare_chr_send(s,
                            pkt->data,
                            pkt->size,
-                           pkt->vnet_hdr_len);
+                           pkt->vnet_hdr_len,
+                           false);
     if (ret < 0) {
         error_report("colo send primary packet failed");
     }
@@ -671,7 +673,8 @@ static void colo_compare_connection(void *opaque, void *user_data)
 static int compare_chr_send(CompareState *s,
                             const uint8_t *buf,
                             uint32_t size,
-                            uint32_t vnet_hdr_len)
+                            uint32_t vnet_hdr_len,
+                            bool notify_remote_frame)
 {
     int ret = 0;
     uint32_t len = htonl(size);
@@ -680,7 +683,14 @@ static int compare_chr_send(CompareState *s,
         return 0;
     }
 
-    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
+    if (notify_remote_frame) {
+        ret = qemu_chr_fe_write_all(&s->chr_notify_dev,
+                                    (uint8_t *)&len,
+                                    sizeof(len));
+    } else {
+        ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
+    }
+
     if (ret != sizeof(len)) {
         goto err;
     }
@@ -691,13 +701,26 @@ static int compare_chr_send(CompareState *s,
          * know how to parse net packet correctly.
          */
         len = htonl(vnet_hdr_len);
-        ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len));
+
+        if (!notify_remote_frame) {
+            ret = qemu_chr_fe_write_all(&s->chr_out,
+                                        (uint8_t *)&len,
+                                        sizeof(len));
+        }
+
         if (ret != sizeof(len)) {
             goto err;
         }
     }
 
-    ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
+    if (notify_remote_frame) {
+        ret = qemu_chr_fe_write_all(&s->chr_notify_dev,
+                                    (uint8_t *)buf,
+                                    size);
+    } else {
+        ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size);
+    }
+
     if (ret != size) {
         goto err;
     }
@@ -943,7 +966,8 @@ static void compare_pri_rs_finalize(SocketReadState *pri_rs)
         compare_chr_send(s,
                          pri_rs->buf,
                          pri_rs->packet_len,
-                         pri_rs->vnet_hdr_len);
+                         pri_rs->vnet_hdr_len,
+                         false);
     } else {
         /* compare packet in the specified connection */
         colo_compare_connection(conn, s);
@@ -1075,7 +1099,8 @@ static void colo_flush_packets(void *opaque, void *user_data)
         compare_chr_send(s,
                          pkt->data,
                          pkt->size,
-                         pkt->vnet_hdr_len);
+                         pkt->vnet_hdr_len,
+                         false);
         packet_destroy(pkt, NULL);
     }
     while (!g_queue_is_empty(&conn->secondary_list)) {
-- 
2.5.0

[Qemu-devel] [PULL 16/17] COLO-compare: Add colo-compare remote notify support

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

This patch make colo-compare can send message to remote COLO frame(Xen) when occur checkpoint.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/colo-compare.c | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index ee03c25..909dd6c 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -120,11 +120,6 @@ enum {
     SECONDARY_IN,
 };
 
-static void colo_compare_inconsistency_notify(void)
-{
-    notifier_list_notify(&colo_compare_notifiers,
-                migrate_get_current());
-}
 
 static int compare_chr_send(CompareState *s,
                             const uint8_t *buf,
@@ -132,6 +127,27 @@ static int compare_chr_send(CompareState *s,
                             uint32_t vnet_hdr_len,
                             bool notify_remote_frame);
 
+static void notify_remote_frame(CompareState *s)
+{
+    char msg[] = "DO_CHECKPOINT";
+    int ret = 0;
+
+    ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
+    if (ret < 0) {
+        error_report("Notify Xen COLO-frame failed");
+    }
+}
+
+static void colo_compare_inconsistency_notify(CompareState *s)
+{
+    if (s->notify_dev) {
+        notify_remote_frame(s);
+    } else {
+        notifier_list_notify(&colo_compare_notifiers,
+                             migrate_get_current());
+    }
+}
+
 static gint seq_sorter(Packet *a, Packet *b, gpointer data)
 {
     struct tcp_hdr *atcp, *btcp;
@@ -435,7 +451,7 @@ sec:
         qemu_hexdump((char *)spkt->data, stderr,
                      "colo-compare spkt", spkt->size);
 
-        colo_compare_inconsistency_notify();
+        colo_compare_inconsistency_notify(s);
     }
 }
 
@@ -577,7 +593,7 @@ void colo_compare_unregister_notifier(Notifier *notify)
 }
 
 static int colo_old_packet_check_one_conn(Connection *conn,
-                                           void *user_data)
+                                          CompareState *s)
 {
     GList *result = NULL;
     int64_t check_time = REGULAR_PACKET_CHECK_MS;
@@ -588,7 +604,7 @@ static int colo_old_packet_check_one_conn(Connection *conn,
 
     if (result) {
         /* Do checkpoint will flush old packet */
-        colo_compare_inconsistency_notify();
+        colo_compare_inconsistency_notify(s);
         return 0;
     }
 
@@ -608,7 +624,7 @@ static void colo_old_packet_check(void *opaque)
      * If we find one old packet, stop finding job and notify
      * COLO frame do checkpoint.
      */
-    g_queue_find_custom(&s->conn_list, NULL,
+    g_queue_find_custom(&s->conn_list, s,
                         (GCompareFunc)colo_old_packet_check_one_conn);
 }
 
@@ -637,7 +653,8 @@ static void colo_compare_packet(CompareState *s, Connection *conn,
              */
             trace_colo_compare_main("packet different");
             g_queue_push_head(&conn->primary_list, pkt);
-            colo_compare_inconsistency_notify();
+
+            colo_compare_inconsistency_notify(s);
             break;
         }
     }
@@ -989,7 +1006,24 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs)
 
 static void compare_notify_rs_finalize(SocketReadState *notify_rs)
 {
+    CompareState *s = container_of(notify_rs, CompareState, notify_rs);
+
     /* Get Xen colo-frame's notify and handle the message */
+    char *data = g_memdup(notify_rs->buf, notify_rs->packet_len);
+    char msg[] = "COLO_COMPARE_GET_XEN_INIT";
+    int ret;
+
+    if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) {
+        ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
+        if (ret < 0) {
+            error_report("Notify Xen COLO-frame INIT failed");
+        }
+    }
+
+    if (!strcmp(data, "COLO_CHECKPOINT")) {
+        /* colo-compare do checkpoint, flush pri packet and remove sec packet */
+        g_queue_foreach(&s->conn_list, colo_flush_packets, s);
+    }
 }
 
 /*
-- 
2.5.0

Re: [Qemu-devel] [PULL 16/17] COLO-compare: Add colo-compare remote notify support

[Peter Maydell]

On Tue, 2 Jul 2019 at 03:32, Jason Wang <jasowang@redhat.com> wrote:
>
> From: Zhang Chen <chen.zhang@intel.com>
>
> This patch make colo-compare can send message to remote COLO frame(Xen) when occur checkpoint.
>
> Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Hi; Coverity reports a problem (CID 1402785) with this function:

> @@ -989,7 +1006,24 @@ static void compare_sec_rs_finalize(SocketReadState *sec_rs)
>
>  static void compare_notify_rs_finalize(SocketReadState *notify_rs)
>  {
> +    CompareState *s = container_of(notify_rs, CompareState, notify_rs);
> +
>      /* Get Xen colo-frame's notify and handle the message */
> +    char *data = g_memdup(notify_rs->buf, notify_rs->packet_len);
> +    char msg[] = "COLO_COMPARE_GET_XEN_INIT";
> +    int ret;
> +
> +    if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) {
> +        ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
> +        if (ret < 0) {
> +            error_report("Notify Xen COLO-frame INIT failed");
> +        }
> +    }
> +
> +    if (!strcmp(data, "COLO_CHECKPOINT")) {
> +        /* colo-compare do checkpoint, flush pri packet and remove sec packet */
> +        g_queue_foreach(&s->conn_list, colo_flush_packets, s);
> +    }
>  }

We allocate data using g_memdup(), but we never free it before returning,
so the function has a memory leak. It's not clear to me why we're
duplicating the string at all -- it would be cleaner to do the
check of the packet contents to identify in-place. That would be
the best way to fix/avoid the leak.

Some other things I notice reading the function:

(1) after the first if() we will go ahead and check the second if().
This means we'll unnecessarily check whether the data string
matches COLO_CHECKPOINT, when we know already it cannot. I think
an if (!strcmp(...)) { ... } else if (!strcmp(...)) { ... }
structure would be more normal C here.

(2) the g_memdup() call is treating the data as a buffer-and-length,
and we don't enforce that it is NUL-terminated. But then we do a
strcmp() against it, which assumes that the data is a NUL-terminated
string. Is this safe ?

(3) More minor point: you could mark 'msg' as const here, since
I think we never need to modify it.

thanks
-- PMM

Re: [Qemu-devel] [PULL 16/17] COLO-compare: Add colo-compare remote notify support

[Zhang, Chen]

> -----Original Message-----
> From: Peter Maydell [mailto:peter.maydell@linaro.org]
> Sent: Wednesday, July 3, 2019 2:51 AM
> To: Jason Wang <jasowang@redhat.com>
> Cc: QEMU Developers <qemu-devel@nongnu.org>; Zhang, Chen
> <chen.zhang@intel.com>
> Subject: Re: [PULL 16/17] COLO-compare: Add colo-compare remote notify
> support
> 
> On Tue, 2 Jul 2019 at 03:32, Jason Wang <jasowang@redhat.com> wrote:
> >
> > From: Zhang Chen <chen.zhang@intel.com>
> >
> > This patch make colo-compare can send message to remote COLO frame(Xen)
> when occur checkpoint.
> >
> > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> 
> Hi; Coverity reports a problem (CID 1402785) with this function:
> 
> > @@ -989,7 +1006,24 @@ static void
> > compare_sec_rs_finalize(SocketReadState *sec_rs)
> >
> >  static void compare_notify_rs_finalize(SocketReadState *notify_rs)  {
> > +    CompareState *s = container_of(notify_rs, CompareState,
> > + notify_rs);
> > +
> >      /* Get Xen colo-frame's notify and handle the message */
> > +    char *data = g_memdup(notify_rs->buf, notify_rs->packet_len);
> > +    char msg[] = "COLO_COMPARE_GET_XEN_INIT";
> > +    int ret;
> > +
> > +    if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) {
> > +        ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
> > +        if (ret < 0) {
> > +            error_report("Notify Xen COLO-frame INIT failed");
> > +        }
> > +    }
> > +
> > +    if (!strcmp(data, "COLO_CHECKPOINT")) {
> > +        /* colo-compare do checkpoint, flush pri packet and remove sec packet
> */
> > +        g_queue_foreach(&s->conn_list, colo_flush_packets, s);
> > +    }
> >  }
> 
> We allocate data using g_memdup(), but we never free it before returning, so
> the function has a memory leak. It's not clear to me why we're duplicating the
> string at all -- it would be cleaner to do the check of the packet contents to
> identify in-place. That would be the best way to fix/avoid the leak.
> 
> Some other things I notice reading the function:
> 
> (1) after the first if() we will go ahead and check the second if().
> This means we'll unnecessarily check whether the data string matches
> COLO_CHECKPOINT, when we know already it cannot. I think an if (!strcmp(...))
> { ... } else if (!strcmp(...)) { ... } structure would be more normal C here.
> 
> (2) the g_memdup() call is treating the data as a buffer-and-length, and we
> don't enforce that it is NUL-terminated. But then we do a
> strcmp() against it, which assumes that the data is a NUL-terminated string. Is
> this safe ?
> 
> (3) More minor point: you could mark 'msg' as const here, since I think we
> never need to modify it.

OK, I will fix it in next version.
Thank you for your review.


Thanks
Zhang Chen

> 
> thanks
> -- PMM

Re: [Qemu-devel] [PULL 16/17] COLO-compare: Add colo-compare remote notify support

[Zhang, Chen]

> -----Original Message-----
> From: Zhang, Chen
> Sent: Wednesday, July 3, 2019 9:08 AM
> To: 'Peter Maydell' <peter.maydell@linaro.org>; Jason Wang
> <jasowang@redhat.com>
> Cc: QEMU Developers <qemu-devel@nongnu.org>
> Subject: RE: [PULL 16/17] COLO-compare: Add colo-compare remote notify
> support
> 
> 
> 
> > -----Original Message-----
> > From: Peter Maydell [mailto:peter.maydell@linaro.org]
> > Sent: Wednesday, July 3, 2019 2:51 AM
> > To: Jason Wang <jasowang@redhat.com>
> > Cc: QEMU Developers <qemu-devel@nongnu.org>; Zhang, Chen
> > <chen.zhang@intel.com>
> > Subject: Re: [PULL 16/17] COLO-compare: Add colo-compare remote notify
> > support
> >
> > On Tue, 2 Jul 2019 at 03:32, Jason Wang <jasowang@redhat.com> wrote:
> > >
> > > From: Zhang Chen <chen.zhang@intel.com>
> > >
> > > This patch make colo-compare can send message to remote COLO
> > > frame(Xen)
> > when occur checkpoint.
> > >
> > > Signed-off-by: Zhang Chen <chen.zhang@intel.com>
> > > Signed-off-by: Jason Wang <jasowang@redhat.com>
> >
> > Hi; Coverity reports a problem (CID 1402785) with this function:
> >
> > > @@ -989,7 +1006,24 @@ static void
> > > compare_sec_rs_finalize(SocketReadState *sec_rs)
> > >
> > >  static void compare_notify_rs_finalize(SocketReadState *notify_rs)
> > > {
> > > +    CompareState *s = container_of(notify_rs, CompareState,
> > > + notify_rs);
> > > +
> > >      /* Get Xen colo-frame's notify and handle the message */
> > > +    char *data = g_memdup(notify_rs->buf, notify_rs->packet_len);
> > > +    char msg[] = "COLO_COMPARE_GET_XEN_INIT";
> > > +    int ret;
> > > +
> > > +    if (!strcmp(data, "COLO_USERSPACE_PROXY_INIT")) {
> > > +        ret = compare_chr_send(s, (uint8_t *)msg, strlen(msg), 0, true);
> > > +        if (ret < 0) {
> > > +            error_report("Notify Xen COLO-frame INIT failed");
> > > +        }
> > > +    }
> > > +
> > > +    if (!strcmp(data, "COLO_CHECKPOINT")) {
> > > +        /* colo-compare do checkpoint, flush pri packet and remove
> > > + sec packet
> > */
> > > +        g_queue_foreach(&s->conn_list, colo_flush_packets, s);
> > > +    }
> > >  }
> >
> > We allocate data using g_memdup(), but we never free it before
> > returning, so the function has a memory leak. It's not clear to me why
> > we're duplicating the string at all -- it would be cleaner to do the
> > check of the packet contents to identify in-place. That would be the best way
> to fix/avoid the leak.
> >
> > Some other things I notice reading the function:
> >
> > (1) after the first if() we will go ahead and check the second if().
> > This means we'll unnecessarily check whether the data string matches
> > COLO_CHECKPOINT, when we know already it cannot. I think an if
> > (!strcmp(...)) { ... } else if (!strcmp(...)) { ... } structure would be more normal
> C here.
> >
> > (2) the g_memdup() call is treating the data as a buffer-and-length,
> > and we don't enforce that it is NUL-terminated. But then we do a
> > strcmp() against it, which assumes that the data is a NUL-terminated
> > string. Is this safe ?
> >
> > (3) More minor point: you could mark 'msg' as const here, since I
> > think we never need to modify it.
> 
> OK, I will fix it in next version.
> Thank you for your review.
> 

I found this patch has been merged, I will send a patch to fix the issues of peter comments.

Thanks
Zhang Chen

> 
> Thanks
> Zhang Chen
> 
> >
> > thanks
> > -- PMM

[Qemu-devel] [PULL 17/17] migration/colo.c: Add missed filter notify for Xen COLO.

[Jason Wang]

From: Zhang Chen <chen.zhang@intel.com>

We need to notify net filter to do checkpoint for Xen COLO, like KVM side.

Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 migration/colo.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/colo.c b/migration/colo.c
index 8c16440..9f84b1f 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -259,6 +259,8 @@ ReplicationStatus *qmp_query_xen_replication_status(Error **errp)
 void qmp_xen_colo_do_checkpoint(Error **errp)
 {
     replication_do_checkpoint_all(errp);
+    /* Notify all filters of all NIC to do checkpoint */
+    colo_notify_filters_event(COLO_EVENT_CHECKPOINT, errp);
 }
 #endif
 
-- 
2.5.0

Re: [Qemu-devel] [PULL 00/17] Net patches

[Peter Maydell]

On Tue, 2 Jul 2019 at 03:31, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 7d0e02405fc02a181319b1ab8681d2f72246b7c6:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/trivial-patches-pull-request' into staging (2019-07-01 17:40:32 +0100)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to 0e8818f023616677416840d6ddc880db8de3c967:
>
>   migration/colo.c: Add missed filter notify for Xen COLO. (2019-07-02 10:21:07 +0800)
>
> ----------------------------------------------------------------
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PULL 00/17] Net patches

[Jason Wang]

On 2019/7/3 上午12:40, Peter Maydell wrote:
> On Tue, 2 Jul 2019 at 03:31, Jason Wang <jasowang@redhat.com> wrote:
>> The following changes since commit 7d0e02405fc02a181319b1ab8681d2f72246b7c6:
>>
>>    Merge remote-tracking branch 'remotes/vivier2/tags/trivial-patches-pull-request' into staging (2019-07-01 17:40:32 +0100)
>>
>> are available in the git repository at:
>>
>>    https://github.com/jasowang/qemu.git tags/net-pull-request
>>
>> for you to fetch changes up to 0e8818f023616677416840d6ddc880db8de3c967:
>>
>>    migration/colo.c: Add missed filter notify for Xen COLO. (2019-07-02 10:21:07 +0800)
>>
>> ----------------------------------------------------------------
>>
>
> Applied, thanks.
>
> Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
> for any user-visible changes.
>
> -- PMM


Done.

Thanks


>