[Qemu-devel] [PATCH 0/9] Network announce changes

[Qemu-devel] [PATCH 0/9] Network announce changes

[Dr. David Alan Gilbert (git)]

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

Hi,
  This is a reworking of a few sets of patches from 2017
that were put together by myself, Germano and Vlad that make
the network announce system more flexible.

  Firstly, it's parameterised, so that you can change the number
of packets and the gap between them; the number can be set to 0
to disable announce completely.

  Secondly, you can force an announce by a qmp or hmp command at
any time.  This is useful if you need the guest to do an announce
for a different reason; for example if the management layer
has just juggled some bonding configuration around.

  The packet creation and timing also moves to net/ from migration/

  The previous set was:
     https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05594.html

Dave

Dr. David Alan Gilbert (9):
  net: Introduce announce timer
  migration: Add announce parameters
  virtio-net: Switch to using announce timer
  migration: Switch to using announce timer
  net: Add a network device specific self-announcement ability
  virtio-net: Allow qemu_announce_self to trigger virtio announcements
  qmp: Add announce-self command
  hmp: Add hmp_announce_self
  tests: Add a test for qemu self announcments

 hmp-commands.hx                |  14 ++++
 hmp.c                          |  33 ++++++++
 hmp.h                          |   1 +
 hw/net/trace-events            |   8 ++
 hw/net/virtio-net.c            |  69 ++++++++++++----
 include/hw/virtio/virtio-net.h |   4 +-
 include/migration/misc.h       |  12 +--
 include/net/announce.h         |  41 ++++++++++
 include/net/net.h              |   2 +
 include/qemu/typedefs.h        |   2 +
 include/sysemu/sysemu.h        |   2 -
 migration/migration.c          | 103 +++++++++++++++++++++++-
 migration/migration.h          |   4 +
 migration/savevm.c             |  72 +----------------
 migration/trace-events         |   1 -
 net/Makefile.objs              |   1 +
 net/announce.c                 | 141 +++++++++++++++++++++++++++++++++
 net/trace-events               |   4 +
 qapi/migration.json            |  56 ++++++++++++-
 qapi/net.json                  |  43 ++++++++++
 tests/Makefile.include         |   2 +
 tests/test-announce-self.c     |  83 +++++++++++++++++++
 tests/test-hmp.c               |   1 +
 23 files changed, 595 insertions(+), 104 deletions(-)
 create mode 100644 include/net/announce.h
 create mode 100644 net/announce.c
 create mode 100644 tests/test-announce-self.c

-- 
2.20.1

[Qemu-devel] [PATCH 1/9] net: Introduce announce timer

[Dr. David Alan Gilbert (git)]

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

The 'announce timer' will be used by migration, and explicit
requests for qemu to perform network announces.

Based on the work by Germano Veit Michel <germano@redhat.com>
 and Vlad Yasevich <vyasevic@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/net/announce.h  | 39 ++++++++++++++++++++++++++
 include/qemu/typedefs.h |  1 +
 migration/migration.c   |  1 +
 net/Makefile.objs       |  1 +
 net/announce.c          | 61 +++++++++++++++++++++++++++++++++++++++++
 qapi/net.json           | 23 ++++++++++++++++
 6 files changed, 126 insertions(+)
 create mode 100644 include/net/announce.h
 create mode 100644 net/announce.c

diff --git a/include/net/announce.h b/include/net/announce.h
new file mode 100644
index 0000000000..b89f1c28b5
--- /dev/null
+++ b/include/net/announce.h
@@ -0,0 +1,39 @@
+/*
+ *  Self-announce facility
+ *  (c) 2017-2019 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_NET_ANNOUNCE_H
+#define QEMU_NET_ANNOUNCE_H
+
+#include "qemu-common.h"
+#include "qapi/qapi-types-net.h"
+#include "qemu/timer.h"
+
+struct AnnounceTimer {
+    QEMUTimer *tm;
+    AnnounceParameters params;
+    QEMUClockType type;
+    int round;
+};
+
+/* Returns: update the timer to the next time point */
+int64_t qemu_announce_timer_step(AnnounceTimer *timer);
+
+/* Delete the underlying timer */
+void qemu_announce_timer_del(AnnounceTimer *timer);
+
+/*
+ * Under BQL/main thread
+ * Reset the timer to the given parameters/type/notifier.
+ */
+void qemu_announce_timer_reset(AnnounceTimer *timer,
+                               AnnounceParameters *params,
+                               QEMUClockType type,
+                               QEMUTimerCB *cb,
+                               void *opaque);
+
+#endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 5d1a2d8329..e4a0a656d1 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -8,6 +8,7 @@
 typedef struct AdapterInfo AdapterInfo;
 typedef struct AddressSpace AddressSpace;
 typedef struct AioContext AioContext;
+typedef struct AnnounceTimer AnnounceTimer;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
 typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
 typedef struct BlockBackend BlockBackend;
diff --git a/migration/migration.c b/migration/migration.c
index 37e06b76dc..a9c4c6f01d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -45,6 +45,7 @@
 #include "migration/colo.h"
 #include "hw/boards.h"
 #include "monitor/monitor.h"
+#include "net/announce.h"
 
 #define MAX_THROTTLE  (32 << 20)      /* Migration transfer speed throttling */
 
diff --git a/net/Makefile.objs b/net/Makefile.objs
index b2bf88a0ef..b363fe92d9 100644
--- a/net/Makefile.objs
+++ b/net/Makefile.objs
@@ -2,6 +2,7 @@ common-obj-y = net.o queue.o checksum.o util.o hub.o
 common-obj-y += socket.o
 common-obj-y += dump.o
 common-obj-y += eth.o
+common-obj-y += announce.o
 common-obj-$(CONFIG_L2TPV3) += l2tpv3.o
 common-obj-$(CONFIG_POSIX) += vhost-user.o
 common-obj-$(CONFIG_SLIRP) += slirp.o
diff --git a/net/announce.c b/net/announce.c
new file mode 100644
index 0000000000..a37089bf27
--- /dev/null
+++ b/net/announce.c
@@ -0,0 +1,61 @@
+/*
+ *  Self-announce
+ *  (c) 2017-2019 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "net/announce.h"
+#include "qapi/clone-visitor.h"
+#include "qapi/qapi-visit-net.h"
+
+int64_t qemu_announce_timer_step(AnnounceTimer *timer)
+{
+    int64_t step;
+
+    step =  timer->params.initial +
+            (timer->params.rounds - timer->round - 1) *
+            timer->params.step;
+
+    if (step < 0 || step > timer->params.max) {
+        step = timer->params.max;
+    }
+    timer_mod(timer->tm, qemu_clock_get_ms(timer->type) + step);
+
+    return step;
+}
+
+void qemu_announce_timer_del(AnnounceTimer *timer)
+{
+    if (timer->tm) {
+        timer_del(timer->tm);
+        timer_free(timer->tm);
+        timer->tm = NULL;
+    }
+}
+
+/*
+ * Under BQL/main thread
+ * Reset the timer to the given parameters/type/notifier.
+ */
+void qemu_announce_timer_reset(AnnounceTimer *timer,
+                               AnnounceParameters *params,
+                               QEMUClockType type,
+                               QEMUTimerCB *cb,
+                               void *opaque)
+{
+    /*
+     * 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);
+
+    QAPI_CLONE_MEMBERS(AnnounceParameters, &timer->params, params);
+    timer->round = params->rounds;
+    timer->type = type;
+    timer->tm = timer_new_ms(type, cb, opaque);
+}
+
diff --git a/qapi/net.json b/qapi/net.json
index a1a0f39f74..b9ff7617bd 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -684,3 +684,26 @@
 ##
 { 'event': 'NIC_RX_FILTER_CHANGED',
   'data': { '*name': 'str', 'path': 'str' } }
+
+##
+# @AnnounceParameters:
+#
+# Parameters for self-announce timers
+#
+# @initial: Initial delay (in ms) before sending the first GARP/RARP
+#       announcement
+#
+# @max: Maximum delay (in ms) to between GARP/RARP announcement packets
+#
+# @rounds: Number of self-announcement attempts
+#
+# @step: Delay increase (in ms) after each self-announcement attempt
+#
+# Since: 4.0
+##
+
+{ 'struct': 'AnnounceParameters',
+  'data': { 'initial': 'int',
+            'max': 'int',
+            'rounds': 'int',
+            'step': 'int' } }
-- 
2.20.1

[Qemu-devel] [PATCH 2/9] migration: Add announce parameters

[Dr. David Alan Gilbert (git)]

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

Add migration parameters that control RARP/GARP announcement timeouts.

Based on earlier patches by myself and
  Vladislav Yasevich <vyasevic@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp.c                    |  28 +++++++++++
 include/migration/misc.h |   2 +
 include/qemu/typedefs.h  |   1 +
 migration/migration.c    | 100 +++++++++++++++++++++++++++++++++++++++
 qapi/migration.json      |  56 ++++++++++++++++++++--
 5 files changed, 183 insertions(+), 4 deletions(-)

diff --git a/hmp.c b/hmp.c
index b2a2b1f84e..319f5134cd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -334,6 +334,18 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
     params = qmp_query_migrate_parameters(NULL);
 
     if (params) {
+        monitor_printf(mon, "%s: %" PRIu64 " ms\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_INITIAL),
+            params->announce_initial);
+        monitor_printf(mon, "%s: %" PRIu64 " ms\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_MAX),
+            params->announce_max);
+        monitor_printf(mon, "%s: %" PRIu64 "\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_ROUNDS),
+            params->announce_rounds);
+        monitor_printf(mon, "%s: %" PRIu64 " ms\n",
+            MigrationParameter_str(MIGRATION_PARAMETER_ANNOUNCE_STEP),
+            params->announce_step);
         assert(params->has_compress_level);
         monitor_printf(mon, "%s: %u\n",
             MigrationParameter_str(MIGRATION_PARAMETER_COMPRESS_LEVEL),
@@ -1757,6 +1769,22 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
         p->has_max_postcopy_bandwidth = true;
         visit_type_size(v, param, &p->max_postcopy_bandwidth, &err);
         break;
+    case MIGRATION_PARAMETER_ANNOUNCE_INITIAL:
+        p->has_announce_initial = true;
+        visit_type_size(v, param, &p->announce_initial, &err);
+        break;
+    case MIGRATION_PARAMETER_ANNOUNCE_MAX:
+        p->has_announce_max = true;
+        visit_type_size(v, param, &p->announce_max, &err);
+        break;
+    case MIGRATION_PARAMETER_ANNOUNCE_ROUNDS:
+        p->has_announce_rounds = true;
+        visit_type_size(v, param, &p->announce_rounds, &err);
+        break;
+    case MIGRATION_PARAMETER_ANNOUNCE_STEP:
+        p->has_announce_step = true;
+        visit_type_size(v, param, &p->announce_step, &err);
+        break;
     default:
         assert(0);
     }
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 4ebf24c6c2..e837ab3042 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -15,6 +15,7 @@
 #define MIGRATION_MISC_H
 
 #include "qemu/notify.h"
+#include "qapi/qapi-types-net.h"
 
 /* migration/ram.c */
 
@@ -38,6 +39,7 @@ int64_t self_announce_delay(int round)
     return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
 }
 
+AnnounceParameters *migrate_announce_params(void);
 /* migration/savevm.c */
 
 void dump_vmstate_json_to_file(FILE *out_fp);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index e4a0a656d1..22b8b20306 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -8,6 +8,7 @@
 typedef struct AdapterInfo AdapterInfo;
 typedef struct AddressSpace AddressSpace;
 typedef struct AioContext AioContext;
+typedef struct AnnounceParameters AnnounceParameters;
 typedef struct AnnounceTimer AnnounceTimer;
 typedef struct BdrvDirtyBitmap BdrvDirtyBitmap;
 typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter;
diff --git a/migration/migration.c b/migration/migration.c
index a9c4c6f01d..ca9c35a5cd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -87,6 +87,15 @@
  */
 #define DEFAULT_MIGRATE_MAX_POSTCOPY_BANDWIDTH 0
 
+/*
+ * Parameters for self_announce_delay giving a stream of RARP/ARP
+ * packets after migration.
+ */
+#define DEFAULT_MIGRATE_ANNOUNCE_INITIAL  50
+#define DEFAULT_MIGRATE_ANNOUNCE_MAX     550
+#define DEFAULT_MIGRATE_ANNOUNCE_ROUNDS    5
+#define DEFAULT_MIGRATE_ANNOUNCE_STEP    100
+
 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
 
@@ -740,10 +749,32 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
     params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
     params->has_max_cpu_throttle = true;
     params->max_cpu_throttle = s->parameters.max_cpu_throttle;
+    params->has_announce_initial = true;
+    params->announce_initial = s->parameters.announce_initial;
+    params->has_announce_max = true;
+    params->announce_max = s->parameters.announce_max;
+    params->has_announce_rounds = true;
+    params->announce_rounds = s->parameters.announce_rounds;
+    params->has_announce_step = true;
+    params->announce_step = s->parameters.announce_step;
 
     return params;
 }
 
+AnnounceParameters *migrate_announce_params(void)
+{
+    static AnnounceParameters ap;
+
+    MigrationState *s = migrate_get_current();
+
+    ap.initial = s->parameters.announce_initial;
+    ap.max = s->parameters.announce_max;
+    ap.rounds = s->parameters.announce_rounds;
+    ap.step = s->parameters.announce_step;
+
+    return &ap;
+}
+
 /*
  * Return true if we're already in the middle of a migration
  * (i.e. any of the active or setup states)
@@ -1117,6 +1148,35 @@ static bool migrate_params_check(MigrationParameters *params, Error **errp)
         return false;
     }
 
+    if (params->has_announce_initial &&
+        params->announce_initial > 100000) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "announce_initial",
+                   "is invalid, it must be less than 100000 ms");
+        return false;
+    }
+    if (params->has_announce_max &&
+        params->announce_max > 100000) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "announce_max",
+                   "is invalid, it must be less than 100000 ms");
+       return false;
+    }
+    if (params->has_announce_rounds &&
+        params->announce_rounds > 1000) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "announce_rounds",
+                   "is invalid, it must be in the range of 0 to 1000");
+       return false;
+    }
+    if (params->has_announce_step &&
+        (params->announce_step < 1 ||
+        params->announce_step > 10000)) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   "announce_step",
+                   "is invalid, it must be in the range of 1 to 10000 ms");
+       return false;
+    }
     return true;
 }
 
@@ -1191,6 +1251,18 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
     if (params->has_max_cpu_throttle) {
         dest->max_cpu_throttle = params->max_cpu_throttle;
     }
+    if (params->has_announce_initial) {
+        dest->announce_initial = params->announce_initial;
+    }
+    if (params->has_announce_max) {
+        dest->announce_max = params->announce_max;
+    }
+    if (params->has_announce_rounds) {
+        dest->announce_rounds = params->announce_rounds;
+    }
+    if (params->has_announce_step) {
+        dest->announce_step = params->announce_step;
+    }
 }
 
 static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
@@ -1273,6 +1345,18 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
     if (params->has_max_cpu_throttle) {
         s->parameters.max_cpu_throttle = params->max_cpu_throttle;
     }
+    if (params->has_announce_initial) {
+        s->parameters.announce_initial = params->announce_initial;
+    }
+    if (params->has_announce_max) {
+        s->parameters.announce_max = params->announce_max;
+    }
+    if (params->has_announce_rounds) {
+        s->parameters.announce_rounds = params->announce_rounds;
+    }
+    if (params->has_announce_step) {
+        s->parameters.announce_step = params->announce_step;
+    }
 }
 
 void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
@@ -3275,6 +3359,18 @@ static Property migration_properties[] = {
     DEFINE_PROP_UINT8("max-cpu-throttle", MigrationState,
                       parameters.max_cpu_throttle,
                       DEFAULT_MIGRATE_MAX_CPU_THROTTLE),
+    DEFINE_PROP_SIZE("announce-initial", MigrationState,
+                      parameters.announce_initial,
+                      DEFAULT_MIGRATE_ANNOUNCE_INITIAL),
+    DEFINE_PROP_SIZE("announce-max", MigrationState,
+                      parameters.announce_max,
+                      DEFAULT_MIGRATE_ANNOUNCE_MAX),
+    DEFINE_PROP_SIZE("announce-rounds", MigrationState,
+                      parameters.announce_rounds,
+                      DEFAULT_MIGRATE_ANNOUNCE_ROUNDS),
+    DEFINE_PROP_SIZE("announce-step", MigrationState,
+                      parameters.announce_step,
+                      DEFAULT_MIGRATE_ANNOUNCE_STEP),
 
     /* Migration capabilities */
     DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE),
@@ -3347,6 +3443,10 @@ static void migration_instance_init(Object *obj)
     params->has_xbzrle_cache_size = true;
     params->has_max_postcopy_bandwidth = true;
     params->has_max_cpu_throttle = true;
+    params->has_announce_initial = true;
+    params->has_announce_max = true;
+    params->has_announce_rounds = true;
+    params->has_announce_step = true;
 
     qemu_sem_init(&ms->postcopy_pause_sem, 0);
     qemu_sem_init(&ms->postcopy_pause_rp_sem, 0);
diff --git a/qapi/migration.json b/qapi/migration.json
index 7a795ecc16..868071d3a0 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -6,6 +6,7 @@
 ##
 
 { 'include': 'common.json' }
+{ 'include': 'net.json' }
 
 ##
 # @MigrationStats:
@@ -480,6 +481,18 @@
 #
 # Migration parameters enumeration
 #
+# @announce-initial: Inital delay (in ms) before sending the first announce
+#          (Since 4.0)
+#
+# @announce-max: Maximum delay (in ms) between packets in the announcment
+#          (Since 4.0)
+#
+# @announce-rounds: Number of self-announce packets sent after migration
+#          (Since 4.0)
+#
+# @announce-step: Increase in delay (in ms) between subsequent packets in
+#          the announcement (Since 4.0)
+#
 # @compress-level: Set the compression level to be used in live migration,
 #          the compression level is an integer between 0 and 9, where 0 means
 #          no compression, 1 means the best compression speed, and 9 means best
@@ -557,10 +570,13 @@
 #
 # @max-cpu-throttle: maximum cpu throttle percentage.
 #                    Defaults to 99. (Since 3.1)
+#
 # Since: 2.4
 ##
 { 'enum': 'MigrationParameter',
-  'data': ['compress-level', 'compress-threads', 'decompress-threads',
+  'data': ['announce-initial', 'announce-max',
+           'announce-rounds', 'announce-step',
+           'compress-level', 'compress-threads', 'decompress-threads',
            'compress-wait-thread',
            'cpu-throttle-initial', 'cpu-throttle-increment',
            'tls-creds', 'tls-hostname', 'max-bandwidth',
@@ -572,6 +588,18 @@
 ##
 # @MigrateSetParameters:
 #
+# @announce-initial: Inital delay (in ms) before sending the first announce
+#          (Since 4.0)
+#
+# @announce-max: Maximum delay (in ms) between packets in the announcment
+#          (Since 4.0)
+#
+# @announce-rounds: Number of self-announce packets sent after migration
+#          (Since 4.0)
+#
+# @announce-step: Increase in delay (in ms) between subsequent packets in
+#          the announcement (Since 4.0)
+#
 # @compress-level: compression level
 #
 # @compress-threads: compression thread count
@@ -653,7 +681,11 @@
 # TODO either fuse back into MigrationParameters, or make
 # MigrationParameters members mandatory
 { 'struct': 'MigrateSetParameters',
-  'data': { '*compress-level': 'int',
+  'data': { '*announce-initial': 'size',
+            '*announce-max': 'size',
+            '*announce-rounds': 'size',
+            '*announce-step': 'size',
+            '*compress-level': 'int',
             '*compress-threads': 'int',
             '*compress-wait-thread': 'bool',
             '*decompress-threads': 'int',
@@ -692,6 +724,18 @@
 #
 # The optional members aren't actually optional.
 #
+# @announce-initial: Inital delay (in ms) before sending the first announce
+#          (Since 4.0)
+#
+# @announce-max: Maximum delay (in ms) between packets in the announcment
+#          (Since 4.0)
+#
+# @announce-rounds: Number of self-announce packets sent after migration
+#          (Since 4.0)
+#
+# @announce-step: Increase in delay (in ms) between subsequent packets in
+#          the announcement (Since 4.0)
+#
 # @compress-level: compression level
 #
 # @compress-threads: compression thread count
@@ -769,7 +813,11 @@
 # Since: 2.4
 ##
 { 'struct': 'MigrationParameters',
-  'data': { '*compress-level': 'uint8',
+  'data': { '*announce-initial': 'size',
+            '*announce-max': 'size',
+            '*announce-rounds': 'size',
+            '*announce-step': 'size',
+            '*compress-level': 'uint8',
             '*compress-threads': 'uint8',
             '*compress-wait-thread': 'bool',
             '*decompress-threads': 'uint8',
@@ -785,7 +833,7 @@
             '*x-multifd-page-count': 'uint32',
             '*xbzrle-cache-size': 'size',
 	    '*max-postcopy-bandwidth': 'size',
-            '*max-cpu-throttle':'uint8'} }
+            '*max-cpu-throttle':'uint8' } }
 
 ##
 # @query-migrate-parameters:
-- 
2.20.1

[Qemu-devel] [PATCH 3/9] virtio-net: Switch to using announce timer

[Dr. David Alan Gilbert (git)]

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

Switch virtio's self announcement to use the AnnounceTimer.
It keeps it's own AnnounceTimer (per device), and starts running it
using a migration post-load and a virtual clock; that way the
announce happens once the guest is actually running.
The timer uses the migration parameters to set the timing of
the repeats.

Based on earlier patches by myself and
 Vladislav Yasevich <vyasevic@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/net/trace-events            |  7 +++++++
 hw/net/virtio-net.c            | 36 ++++++++++++++++++++++------------
 include/hw/virtio/virtio-net.h |  4 ++--
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/hw/net/trace-events b/hw/net/trace-events
index 9d49f62fa1..fc800dd606 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -359,3 +359,10 @@ sunhme_rx_filter_reject(void) "rejecting incoming frame"
 sunhme_rx_filter_accept(void) "accepting incoming frame"
 sunhme_rx_desc(uint32_t addr, int offset, uint32_t status, int len, int cr, int nr) "addr 0x%"PRIx32"(+0x%x) status 0x%"PRIx32 " len %d (ring %d/%d)"
 sunhme_rx_xsum_calc(uint16_t xsum) "calculated incoming xsum as 0x%x"
+
+# hw/net/virtio-net.c
+virtio_net_announce_timer(int round) "%d"
+virtio_net_handle_announce(int round) "%d"
+virtio_net_post_load_device(void)
+
+
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 3f319ef723..b50f86d230 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -21,12 +21,14 @@
 #include "qemu/timer.h"
 #include "hw/virtio/virtio-net.h"
 #include "net/vhost_net.h"
+#include "net/announce.h"
 #include "hw/virtio/virtio-bus.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-net.h"
 #include "hw/virtio/virtio-access.h"
 #include "migration/misc.h"
 #include "standard-headers/linux/ethtool.h"
+#include "trace.h"
 
 #define VIRTIO_NET_VM_VERSION    11
 
@@ -164,8 +166,9 @@ static void virtio_net_announce_timer(void *opaque)
 {
     VirtIONet *n = opaque;
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
+    trace_virtio_net_announce_timer(n->announce_timer.round);
 
-    n->announce_counter--;
+    n->announce_timer.round--;
     n->status |= VIRTIO_NET_S_ANNOUNCE;
     virtio_notify_config(vdev);
 }
@@ -479,8 +482,8 @@ static void virtio_net_reset(VirtIODevice *vdev)
     n->nobcast = 0;
     /* multiqueue is disabled by default */
     n->curr_queues = 1;
-    timer_del(n->announce_timer);
-    n->announce_counter = 0;
+    timer_del(n->announce_timer.tm);
+    n->announce_timer.round = 0;
     n->status &= ~VIRTIO_NET_S_ANNOUNCE;
 
     /* Flush any MAC and VLAN filter table state */
@@ -976,13 +979,12 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd,
 static int virtio_net_handle_announce(VirtIONet *n, uint8_t cmd,
                                       struct iovec *iov, unsigned int iov_cnt)
 {
+    trace_virtio_net_handle_announce(n->announce_timer.round);
     if (cmd == VIRTIO_NET_CTRL_ANNOUNCE_ACK &&
         n->status & VIRTIO_NET_S_ANNOUNCE) {
         n->status &= ~VIRTIO_NET_S_ANNOUNCE;
-        if (n->announce_counter) {
-            timer_mod(n->announce_timer,
-                      qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
-                      self_announce_delay(n->announce_counter));
+        if (n->announce_timer.round) {
+            qemu_announce_timer_step(&n->announce_timer);
         }
         return VIRTIO_NET_OK;
     } else {
@@ -2298,6 +2300,7 @@ static int virtio_net_post_load_device(void *opaque, int version_id)
     VirtIODevice *vdev = VIRTIO_DEVICE(n);
     int i, link_down;
 
+    trace_virtio_net_post_load_device();
     virtio_net_set_mrg_rx_bufs(n, n->mergeable_rx_bufs,
                                virtio_vdev_has_feature(vdev,
                                                        VIRTIO_F_VERSION_1));
@@ -2334,8 +2337,15 @@ static int virtio_net_post_load_device(void *opaque, int version_id)
 
     if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) &&
         virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) {
-        n->announce_counter = SELF_ANNOUNCE_ROUNDS;
-        timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL));
+        qemu_announce_timer_reset(&n->announce_timer, migrate_announce_params(),
+                                  QEMU_CLOCK_VIRTUAL,
+                                  virtio_net_announce_timer, n);
+        if (n->announce_timer.round) {
+            timer_mod(n->announce_timer.tm,
+                      qemu_clock_get_ms(n->announce_timer.type));
+        } else {
+            qemu_announce_timer_del(&n->announce_timer);
+        }
     }
 
     return 0;
@@ -2696,8 +2706,9 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     qemu_macaddr_default_if_unset(&n->nic_conf.macaddr);
     memcpy(&n->mac[0], &n->nic_conf.macaddr, sizeof(n->mac));
     n->status = VIRTIO_NET_S_LINK_UP;
-    n->announce_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
-                                     virtio_net_announce_timer, n);
+    qemu_announce_timer_reset(&n->announce_timer, migrate_announce_params(),
+                              QEMU_CLOCK_VIRTUAL,
+                              virtio_net_announce_timer, n);
 
     if (n->netclient_type) {
         /*
@@ -2760,8 +2771,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
         virtio_net_del_queue(n, i);
     }
 
-    timer_del(n->announce_timer);
-    timer_free(n->announce_timer);
+    qemu_announce_timer_del(&n->announce_timer);
     g_free(n->vqs);
     qemu_del_nic(n->nic);
     virtio_net_rsc_cleanup(n);
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index a1a0be3bea..b96f0c643f 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -17,6 +17,7 @@
 #include "qemu/units.h"
 #include "standard-headers/linux/virtio_net.h"
 #include "hw/virtio/virtio.h"
+#include "net/announce.h"
 
 #define TYPE_VIRTIO_NET "virtio-net-device"
 #define VIRTIO_NET(obj) \
@@ -181,8 +182,7 @@ struct VirtIONet {
     char *netclient_name;
     char *netclient_type;
     uint64_t curr_guest_offloads;
-    QEMUTimer *announce_timer;
-    int announce_counter;
+    AnnounceTimer announce_timer;
     bool needs_vnet_hdr_swap;
     bool mtu_bypass_backend;
 };
-- 
2.20.1

[Qemu-devel] [PATCH 4/9] migration: Switch to using announce timer

[Dr. David Alan Gilbert (git)]

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

Switch the announcements to using the new announce timer.
Move the code that does it to announce.c rather than savevm
because it really has nothing to do with the actual migration.

Migration starts the announce from bh's and so they're all
in the main thread/bql, and so there's never any racing with
the timers themselves.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/migration/misc.h | 10 ------
 include/net/announce.h   |  2 ++
 include/sysemu/sysemu.h  |  2 --
 migration/migration.c    |  2 +-
 migration/migration.h    |  4 +++
 migration/savevm.c       | 72 ++--------------------------------------
 migration/trace-events   |  1 -
 net/announce.c           | 67 +++++++++++++++++++++++++++++++++++++
 net/trace-events         |  4 +++
 9 files changed, 81 insertions(+), 83 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index e837ab3042..0471e04d1f 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -29,16 +29,6 @@ void blk_mig_init(void);
 static inline void blk_mig_init(void) {}
 #endif
 
-#define SELF_ANNOUNCE_ROUNDS 5
-
-static inline
-int64_t self_announce_delay(int round)
-{
-    assert(round < SELF_ANNOUNCE_ROUNDS && round > 0);
-    /* delay 50ms, 150ms, 250ms, ... */
-    return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
-}
-
 AnnounceParameters *migrate_announce_params(void);
 /* migration/savevm.c */
 
diff --git a/include/net/announce.h b/include/net/announce.h
index b89f1c28b5..892d302b65 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -36,4 +36,6 @@ void qemu_announce_timer_reset(AnnounceTimer *timer,
                                QEMUTimerCB *cb,
                                void *opaque);
 
+void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params);
+
 #endif
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 85877b7e43..e322e696f2 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -81,8 +81,6 @@ extern bool machine_init_done;
 void qemu_add_machine_init_done_notifier(Notifier *notify);
 void qemu_remove_machine_init_done_notifier(Notifier *notify);
 
-void qemu_announce_self(void);
-
 extern int autostart;
 
 typedef enum {
diff --git a/migration/migration.c b/migration/migration.c
index ca9c35a5cd..c39d3054ec 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -374,7 +374,7 @@ static void process_incoming_migration_bh(void *opaque)
      * This must happen after all error conditions are dealt with and
      * we're sure the VM is going to be running on this host.
      */
-    qemu_announce_self();
+    qemu_announce_self(&mis->announce_timer, migrate_announce_params());
 
     if (multifd_load_cleanup(&local_err) != 0) {
         error_report_err(local_err);
diff --git a/migration/migration.h b/migration/migration.h
index dcd05d9f87..c99154dea2 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -21,6 +21,7 @@
 #include "qemu/coroutine_int.h"
 #include "hw/qdev.h"
 #include "io/channel.h"
+#include "net/announce.h"
 
 struct PostcopyBlocktimeContext;
 
@@ -36,6 +37,9 @@ struct MigrationIncomingState {
      */
     QemuEvent main_thread_load_event;
 
+    /* For network announces */
+    AnnounceTimer  announce_timer;
+
     size_t         largest_page_size;
     bool           have_fault_thread;
     QemuThread     fault_thread;
diff --git a/migration/savevm.c b/migration/savevm.c
index 322660438d..b3868f7fb5 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -57,13 +57,7 @@
 #include "sysemu/replay.h"
 #include "qjson.h"
 #include "migration/colo.h"
-
-#ifndef ETH_P_RARP
-#define ETH_P_RARP 0x8035
-#endif
-#define ARP_HTYPE_ETH 0x0001
-#define ARP_PTYPE_IP 0x0800
-#define ARP_OP_REQUEST_REV 0x3
+#include "net/announce.h"
 
 const unsigned int postcopy_ram_discard_version = 0;
 
@@ -125,67 +119,6 @@ static struct mig_cmd_args {
  * generic extendable format with an exception for two old entities.
  */
 
-static int announce_self_create(uint8_t *buf,
-                                uint8_t *mac_addr)
-{
-    /* Ethernet header. */
-    memset(buf, 0xff, 6);         /* destination MAC addr */
-    memcpy(buf + 6, mac_addr, 6); /* source MAC addr */
-    *(uint16_t *)(buf + 12) = htons(ETH_P_RARP); /* ethertype */
-
-    /* RARP header. */
-    *(uint16_t *)(buf + 14) = htons(ARP_HTYPE_ETH); /* hardware addr space */
-    *(uint16_t *)(buf + 16) = htons(ARP_PTYPE_IP); /* protocol addr space */
-    *(buf + 18) = 6; /* hardware addr length (ethernet) */
-    *(buf + 19) = 4; /* protocol addr length (IPv4) */
-    *(uint16_t *)(buf + 20) = htons(ARP_OP_REQUEST_REV); /* opcode */
-    memcpy(buf + 22, mac_addr, 6); /* source hw addr */
-    memset(buf + 28, 0x00, 4);     /* source protocol addr */
-    memcpy(buf + 32, mac_addr, 6); /* target hw addr */
-    memset(buf + 38, 0x00, 4);     /* target protocol addr */
-
-    /* Padding to get up to 60 bytes (ethernet min packet size, minus FCS). */
-    memset(buf + 42, 0x00, 18);
-
-    return 60; /* len (FCS will be added by hardware) */
-}
-
-static void qemu_announce_self_iter(NICState *nic, void *opaque)
-{
-    uint8_t buf[60];
-    int len;
-
-    trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
-    len = announce_self_create(buf, nic->conf->macaddr.a);
-
-    qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
-}
-
-
-static void qemu_announce_self_once(void *opaque)
-{
-    static int count = SELF_ANNOUNCE_ROUNDS;
-    QEMUTimer *timer = *(QEMUTimer **)opaque;
-
-    qemu_foreach_nic(qemu_announce_self_iter, NULL);
-
-    if (--count) {
-        /* delay 50ms, 150ms, 250ms, ... */
-        timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
-                  self_announce_delay(count));
-    } else {
-            timer_del(timer);
-            timer_free(timer);
-    }
-}
-
-void qemu_announce_self(void)
-{
-    static QEMUTimer *timer;
-    timer = timer_new_ms(QEMU_CLOCK_REALTIME, qemu_announce_self_once, &timer);
-    qemu_announce_self_once(&timer);
-}
-
 /***********************************************************/
 /* savevm/loadvm support */
 
@@ -1765,13 +1698,14 @@ static void loadvm_postcopy_handle_run_bh(void *opaque)
 {
     Error *local_err = NULL;
     HandleRunBhData *data = opaque;
+    MigrationIncomingState *mis = migration_incoming_get_current();
 
     /* TODO we should move all of this lot into postcopy_ram.c or a shared code
      * in migration.c
      */
     cpu_synchronize_all_post_init();
 
-    qemu_announce_self();
+    qemu_announce_self(&mis->announce_timer, migrate_announce_params());
 
     /* Make sure all file formats flush their mutable metadata.
      * If we get an error here, just don't restart the VM yet. */
diff --git a/migration/trace-events b/migration/trace-events
index bd2d0cd25a..72e3fcb885 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -52,7 +52,6 @@ vmstate_save_state_top(const char *idstr) "%s"
 vmstate_subsection_save_loop(const char *name, const char *sub) "%s/%s"
 vmstate_subsection_save_top(const char *idstr) "%s"
 vmstate_load(const char *idstr, const char *vmsd_name) "%s, %s"
-qemu_announce_self_iter(const char *mac) "%s"
 
 # migration/vmstate.c
 vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
diff --git a/net/announce.c b/net/announce.c
index a37089bf27..13ad9c2ba8 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -9,8 +9,10 @@
 #include "qemu/osdep.h"
 #include "qemu-common.h"
 #include "net/announce.h"
+#include "net/net.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-visit-net.h"
+#include "trace.h"
 
 int64_t qemu_announce_timer_step(AnnounceTimer *timer)
 {
@@ -59,3 +61,68 @@ void qemu_announce_timer_reset(AnnounceTimer *timer,
     timer->tm = timer_new_ms(type, cb, opaque);
 }
 
+#ifndef ETH_P_RARP
+#define ETH_P_RARP 0x8035
+#endif
+#define ARP_HTYPE_ETH 0x0001
+#define ARP_PTYPE_IP 0x0800
+#define ARP_OP_REQUEST_REV 0x3
+
+static int announce_self_create(uint8_t *buf,
+                                uint8_t *mac_addr)
+{
+    /* Ethernet header. */
+    memset(buf, 0xff, 6);         /* destination MAC addr */
+    memcpy(buf + 6, mac_addr, 6); /* source MAC addr */
+    *(uint16_t *)(buf + 12) = htons(ETH_P_RARP); /* ethertype */
+
+    /* RARP header. */
+    *(uint16_t *)(buf + 14) = htons(ARP_HTYPE_ETH); /* hardware addr space */
+    *(uint16_t *)(buf + 16) = htons(ARP_PTYPE_IP); /* protocol addr space */
+    *(buf + 18) = 6; /* hardware addr length (ethernet) */
+    *(buf + 19) = 4; /* protocol addr length (IPv4) */
+    *(uint16_t *)(buf + 20) = htons(ARP_OP_REQUEST_REV); /* opcode */
+    memcpy(buf + 22, mac_addr, 6); /* source hw addr */
+    memset(buf + 28, 0x00, 4);     /* source protocol addr */
+    memcpy(buf + 32, mac_addr, 6); /* target hw addr */
+    memset(buf + 38, 0x00, 4);     /* target protocol addr */
+
+    /* Padding to get up to 60 bytes (ethernet min packet size, minus FCS). */
+    memset(buf + 42, 0x00, 18);
+
+    return 60; /* len (FCS will be added by hardware) */
+}
+
+static void qemu_announce_self_iter(NICState *nic, void *opaque)
+{
+    uint8_t buf[60];
+    int len;
+
+    trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
+    len = announce_self_create(buf, nic->conf->macaddr.a);
+
+    qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
+}
+static void qemu_announce_self_once(void *opaque)
+{
+    AnnounceTimer *timer = (AnnounceTimer *)opaque;
+
+    qemu_foreach_nic(qemu_announce_self_iter, NULL);
+
+    if (--timer->round) {
+        qemu_announce_timer_step(timer);
+    } else {
+        qemu_announce_timer_del(timer);
+    }
+}
+
+void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params)
+{
+    qemu_announce_timer_reset(timer, params, QEMU_CLOCK_REALTIME,
+                              qemu_announce_self_once, timer);
+    if (params->rounds) {
+        qemu_announce_self_once(timer);
+    } else {
+        qemu_announce_timer_del(timer);
+    }
+}
diff --git a/net/trace-events b/net/trace-events
index 7b594cfdd2..7effa62787 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,5 +1,8 @@
 # See docs/devel/tracing.txt for syntax documentation.
 
+# net/announce.c
+qemu_announce_self_iter(const char *mac) "%s"
+
 # net/vhost-user.c
 vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
 
@@ -19,3 +22,4 @@ colo_compare_tcp_info(const char *pkt, uint32_t seq, uint32_t ack, int hdlen, in
 colo_filter_rewriter_debug(void) ""
 colo_filter_rewriter_pkt_info(const char *func, const char *src, const char *dst, uint32_t seq, uint32_t ack, uint32_t flag) "%s: src/dst: %s/%s p: seq/ack=%u/%u  flags=0x%x\n"
 colo_filter_rewriter_conn_offset(uint32_t offset) ": offset=%u\n"
+
-- 
2.20.1

[Qemu-devel] [PATCH 5/9] net: Add a network device specific self-announcement ability

[Dr. David Alan Gilbert (git)]

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

Some network devices have a capability to do self announcements
(ex: virtio-net).  Add infrastructure that would allow devices
to expose this ability.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/net/net.h | 2 ++
 net/announce.c    | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index 643295d163..f888cc3019 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -60,6 +60,7 @@ typedef int (SetVnetLE)(NetClientState *, bool);
 typedef int (SetVnetBE)(NetClientState *, bool);
 typedef struct SocketReadState SocketReadState;
 typedef void (SocketReadStateFinalize)(SocketReadState *rs);
+typedef void (NetAnnounce)(NetClientState *);
 
 typedef struct NetClientInfo {
     NetClientDriver type;
@@ -80,6 +81,7 @@ typedef struct NetClientInfo {
     SetVnetHdrLen *set_vnet_hdr_len;
     SetVnetLE *set_vnet_le;
     SetVnetBE *set_vnet_be;
+    NetAnnounce *announce;
 } NetClientInfo;
 
 struct NetClientState {
diff --git a/net/announce.c b/net/announce.c
index 13ad9c2ba8..070f37a7fa 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -102,6 +102,11 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
     len = announce_self_create(buf, nic->conf->macaddr.a);
 
     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);
+    }
 }
 static void qemu_announce_self_once(void *opaque)
 {
-- 
2.20.1

[Qemu-devel] [PATCH 6/9] virtio-net: Allow qemu_announce_self to trigger virtio announcements

[Dr. David Alan Gilbert (git)]

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

Expose the virtio-net self announcement capability and allow
qemu_announce_self() to call it.

These announces are caused by something external (i.e. the
announce-self command); they won't trigger if the migration
counter is triggering announces at the same time.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/net/trace-events |  1 +
 hw/net/virtio-net.c | 35 ++++++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/hw/net/trace-events b/hw/net/trace-events
index fc800dd606..b419d0b1a7 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -361,6 +361,7 @@ sunhme_rx_desc(uint32_t addr, int offset, uint32_t status, int len, int cr, int
 sunhme_rx_xsum_calc(uint16_t xsum) "calculated incoming xsum as 0x%x"
 
 # hw/net/virtio-net.c
+virtio_net_announce_notify(void) ""
 virtio_net_announce_timer(int round) "%d"
 virtio_net_handle_announce(int round) "%d"
 virtio_net_post_load_device(void)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b50f86d230..ed0fa97b88 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -162,15 +162,42 @@ static bool virtio_net_started(VirtIONet *n, uint8_t status)
         (n->status & VIRTIO_NET_S_LINK_UP) && vdev->vm_running;
 }
 
+static void virtio_net_announce_notify(VirtIONet *net)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(net);
+    trace_virtio_net_announce_notify();
+
+    net->status |= VIRTIO_NET_S_ANNOUNCE;
+    virtio_notify_config(vdev);
+}
+
 static void virtio_net_announce_timer(void *opaque)
 {
     VirtIONet *n = opaque;
-    VirtIODevice *vdev = VIRTIO_DEVICE(n);
     trace_virtio_net_announce_timer(n->announce_timer.round);
 
     n->announce_timer.round--;
-    n->status |= VIRTIO_NET_S_ANNOUNCE;
-    virtio_notify_config(vdev);
+    virtio_net_announce_notify(n);
+}
+
+static void virtio_net_announce(NetClientState *nc)
+{
+    VirtIONet *n = qemu_get_nic_opaque(nc);
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
+
+    /*
+     * Make sure the virtio migration announcement timer isn't running
+     * If it is, let it trigger announcement so that we do not cause
+     * confusion.
+     */
+    if (n->announce_timer.round) {
+        return;
+    }
+
+    if (virtio_vdev_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) &&
+        virtio_vdev_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) {
+            virtio_net_announce_notify(n);
+    }
 }
 
 static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
@@ -2568,6 +2595,7 @@ static NetClientInfo net_virtio_info = {
     .receive = virtio_net_receive,
     .link_status_changed = virtio_net_set_link_status,
     .query_rx_filter = virtio_net_query_rxfilter,
+    .announce = virtio_net_announce,
 };
 
 static bool virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
@@ -2709,6 +2737,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     qemu_announce_timer_reset(&n->announce_timer, migrate_announce_params(),
                               QEMU_CLOCK_VIRTUAL,
                               virtio_net_announce_timer, n);
+    n->announce_timer.round = 0;
 
     if (n->netclient_type) {
         /*
-- 
2.20.1

[Qemu-devel] [PATCH 7/9] qmp: Add announce-self command

[Dr. David Alan Gilbert (git)]

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

Add a qmp command that can trigger guest announcements.

It uses it's own announce-timer instance, and parameters
passed to it explicitly in the command.

Like most qmp commands, it's in the main thread/bql, so
there's no racing with any outstanding timer.

Based on work of Germano Veit Michel <germano@redhat.com> and
                 Vladislav Yasevich <vyasevic@redhat.com>

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 net/announce.c |  8 ++++++++
 qapi/net.json  | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/net/announce.c b/net/announce.c
index 070f37a7fa..8cd9f4d909 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -12,6 +12,7 @@
 #include "net/net.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/qapi-visit-net.h"
+#include "qapi/qapi-commands-net.h"
 #include "trace.h"
 
 int64_t qemu_announce_timer_step(AnnounceTimer *timer)
@@ -131,3 +132,10 @@ void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params)
         qemu_announce_timer_del(timer);
     }
 }
+
+void qmp_announce_self(AnnounceParameters *params, Error **errp)
+{
+    static AnnounceTimer announce_timer;
+    qemu_announce_self(&announce_timer, params);
+}
+
diff --git a/qapi/net.json b/qapi/net.json
index b9ff7617bd..fe1640448b 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -707,3 +707,23 @@
             'max': 'int',
             'rounds': 'int',
             'step': 'int' } }
+
+##
+# @announce-self:
+#
+# Trigger generation of broadcast RARP frames to update network switches.
+# This can be useful when network bonds fail-over the active slave.
+#
+# @params: AnnounceParameters giving timing and repetition count of announce
+#
+# Example:
+#
+# -> { "execute": "announce-self"
+#      "arguments": { "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
+# <- { "return": {} }
+#
+# Since: 4.0
+##
+{ 'command': 'announce-self',
+  'data' : {'params': 'AnnounceParameters'} }
+
-- 
2.20.1

[Qemu-devel] [PATCH 8/9] hmp: Add hmp_announce_self

[Dr. David Alan Gilbert (git)]

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

Add an HMP command to trigger self annocements.
Unlike the QMP command (which takes a set of parameters), the HMP
command reuses the set of parameters used for migration.

Signend-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands.hx  | 14 ++++++++++++++
 hmp.c            |  5 +++++
 hmp.h            |  1 +
 tests/test-hmp.c |  1 +
 4 files changed, 21 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ba71558c25..9f812bc63b 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -928,6 +928,20 @@ Bug: can screw up when the buffer contains invalid UTF-8 sequences,
 NUL characters, after the ring buffer lost data, and when reading
 stops because the size limit is reached.
 
+ETEXI
+
+    {
+        .name       = "announce_self",
+        .args_type  = "",
+        .params     = "",
+        .help       = "Trigger GARP/RARP announcements",
+        .cmd        = hmp_announce_self,
+    },
+
+STEXI
+@item announce_self
+@findex announce_self
+Trigger GARP/RARP announcements.
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index 319f5134cd..ccaa817ecd 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1570,6 +1570,11 @@ 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);
+}
+
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
 {
     qmp_migrate_cancel(NULL);
diff --git a/hmp.h b/hmp.h
index 5f1addcca2..e0f32f04d3 100644
--- a/hmp.h
+++ b/hmp.h
@@ -46,6 +46,7 @@ void hmp_sync_profile(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
 void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
 void hmp_exit_preconfig(Monitor *mon, const QDict *qdict);
+void hmp_announce_self(Monitor *mon, const QDict *qdict);
 void hmp_cpu(Monitor *mon, const QDict *qdict);
 void hmp_memsave(Monitor *mon, const QDict *qdict);
 void hmp_pmemsave(Monitor *mon, const QDict *qdict);
diff --git a/tests/test-hmp.c b/tests/test-hmp.c
index 1a3a9c5099..8c49d2fdf1 100644
--- a/tests/test-hmp.c
+++ b/tests/test-hmp.c
@@ -20,6 +20,7 @@
 static int verbose;
 
 static const char *hmp_cmds[] = {
+    "announce_self",
     "boot_set ndc",
     "chardev-add null,id=testchardev1",
     "chardev-send-break testchardev1",
-- 
2.20.1

[Qemu-devel] [PATCH 9/9] tests: Add a test for qemu self announcments

[Dr. David Alan Gilbert (git)]

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

We now expose qemu_announce_self through QMP and HMP.  Add a test
with some very basic packet validation (make sure we get a RARP).

Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 tests/Makefile.include     |  2 +
 tests/test-announce-self.c | 83 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 tests/test-announce-self.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 19b4c0a696..57ce722d73 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -143,6 +143,8 @@ check-qtest-generic-y += tests/qmp-cmd-test$(EXESUF)
 
 check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
 check-qtest-generic-y += tests/cdrom-test$(EXESUF)
+check-qtest-generic-y += tests/test-announce-self$(EXESUF)
+gcov-files-generic-y = migration/savevm.c
 
 check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
 
diff --git a/tests/test-announce-self.c b/tests/test-announce-self.c
new file mode 100644
index 0000000000..2ff0c77255
--- /dev/null
+++ b/tests/test-announce-self.c
@@ -0,0 +1,83 @@
+/*
+ * QTest testcase for qemu_announce_self
+ *
+ * Copyright (c) 2017 Red hat, Inc.
+ * Copyright (c) 2014 SUSE LINUX Products GmbH
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu-common.h"
+#include "qemu/sockets.h"
+#include "qemu/iov.h"
+#include "libqos/libqos-pc.h"
+#include "libqos/libqos-spapr.h"
+
+#ifndef ETH_P_RARP
+#define ETH_P_RARP 0x8035
+#endif
+
+static QTestState *test_init(int socket)
+{
+    char *args;
+
+    args = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
+                           "virtio-net-pci,netdev=hs0", socket);
+
+    return qtest_start(args);
+}
+
+
+static void test_announce(int socket)
+{
+    char buffer[60];
+    int len;
+    QDict *rsp;
+    int ret;
+    uint16_t *proto = (uint16_t *)&buffer[12];
+
+    rsp = qmp("{ 'execute' : 'announce-self', "
+                  " 'arguments': {"
+                      " 'params': {"
+                          " 'initial': 50, 'max': 550,"
+                          " 'rounds': 10, 'step': 50 } } }");
+    assert(!qdict_haskey(rsp, "error"));
+    qobject_unref(rsp);
+
+    /* Catch the packet and make sure it's a RARP */
+    ret = qemu_recv(socket, &len, sizeof(len), 0);
+    g_assert_cmpint(ret, ==,  sizeof(len));
+    len = ntohl(len);
+
+    ret = qemu_recv(socket, buffer, len, 0);
+    g_assert_cmpint(*proto, ==, htons(ETH_P_RARP));
+}
+
+static void setup(gconstpointer data)
+{
+    QTestState *qs;
+    void (*func) (int socket) = data;
+    int sv[2], ret;
+
+    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
+    g_assert_cmpint(ret, !=, -1);
+
+    qs = test_init(sv[1]);
+    func(sv[0]);
+
+    /* End test */
+    close(sv[0]);
+    qtest_quit(qs);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_data_func("/virtio/net/test_announce_self", test_announce, setup);
+
+    return g_test_run();
+}
-- 
2.20.1

Re: [Qemu-devel] [PATCH 9/9] tests: Add a test for qemu self announcments

[Michael S. Tsirkin]

On Mon, Jan 28, 2019 at 05:03:21PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> We now expose qemu_announce_self through QMP and HMP.  Add a test
> with some very basic packet validation (make sure we get a RARP).
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

typo in subject

> ---
>  tests/Makefile.include     |  2 +
>  tests/test-announce-self.c | 83 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+)
>  create mode 100644 tests/test-announce-self.c
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 19b4c0a696..57ce722d73 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -143,6 +143,8 @@ check-qtest-generic-y += tests/qmp-cmd-test$(EXESUF)
>  
>  check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
>  check-qtest-generic-y += tests/cdrom-test$(EXESUF)
> +check-qtest-generic-y += tests/test-announce-self$(EXESUF)
> +gcov-files-generic-y = migration/savevm.c
>  
>  check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
>  
> diff --git a/tests/test-announce-self.c b/tests/test-announce-self.c
> new file mode 100644
> index 0000000000..2ff0c77255
> --- /dev/null
> +++ b/tests/test-announce-self.c
> @@ -0,0 +1,83 @@
> +/*
> + * QTest testcase for qemu_announce_self
> + *
> + * Copyright (c) 2017 Red hat, Inc.
> + * Copyright (c) 2014 SUSE LINUX Products GmbH
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qemu-common.h"
> +#include "qemu/sockets.h"
> +#include "qemu/iov.h"
> +#include "libqos/libqos-pc.h"
> +#include "libqos/libqos-spapr.h"
> +
> +#ifndef ETH_P_RARP
> +#define ETH_P_RARP 0x8035
> +#endif
> +
> +static QTestState *test_init(int socket)
> +{
> +    char *args;
> +
> +    args = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
> +                           "virtio-net-pci,netdev=hs0", socket);
> +
> +    return qtest_start(args);
> +}
> +
> +
> +static void test_announce(int socket)
> +{
> +    char buffer[60];
> +    int len;
> +    QDict *rsp;
> +    int ret;
> +    uint16_t *proto = (uint16_t *)&buffer[12];
> +
> +    rsp = qmp("{ 'execute' : 'announce-self', "
> +                  " 'arguments': {"
> +                      " 'params': {"
> +                          " 'initial': 50, 'max': 550,"
> +                          " 'rounds': 10, 'step': 50 } } }");
> +    assert(!qdict_haskey(rsp, "error"));
> +    qobject_unref(rsp);
> +
> +    /* Catch the packet and make sure it's a RARP */
> +    ret = qemu_recv(socket, &len, sizeof(len), 0);
> +    g_assert_cmpint(ret, ==,  sizeof(len));
> +    len = ntohl(len);
> +
> +    ret = qemu_recv(socket, buffer, len, 0);
> +    g_assert_cmpint(*proto, ==, htons(ETH_P_RARP));
> +}
> +
> +static void setup(gconstpointer data)
> +{
> +    QTestState *qs;
> +    void (*func) (int socket) = data;
> +    int sv[2], ret;
> +
> +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
> +    g_assert_cmpint(ret, !=, -1);
> +
> +    qs = test_init(sv[1]);
> +    func(sv[0]);
> +
> +    /* End test */
> +    close(sv[0]);
> +    qtest_quit(qs);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    qtest_add_data_func("/virtio/net/test_announce_self", test_announce, setup);
> +
> +    return g_test_run();
> +}
> -- 
> 2.20.1

Re: [Qemu-devel] [PATCH 0/9] Network announce changes

[Michael S. Tsirkin]

On Mon, Jan 28, 2019 at 05:03:12PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Hi,
>   This is a reworking of a few sets of patches from 2017
> that were put together by myself, Germano and Vlad that make
> the network announce system more flexible.
> 
>   Firstly, it's parameterised, so that you can change the number
> of packets and the gap between them; the number can be set to 0
> to disable announce completely.
> 
>   Secondly, you can force an announce by a qmp or hmp command at
> any time.  This is useful if you need the guest to do an announce
> for a different reason; for example if the management layer
> has just juggled some bonding configuration around.
> 
>   The packet creation and timing also moves to net/ from migration/
> 
>   The previous set was:
>      https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05594.html
> 
> Dave

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

Who's applying this? Jason?


> Dr. David Alan Gilbert (9):
>   net: Introduce announce timer
>   migration: Add announce parameters
>   virtio-net: Switch to using announce timer
>   migration: Switch to using announce timer
>   net: Add a network device specific self-announcement ability
>   virtio-net: Allow qemu_announce_self to trigger virtio announcements
>   qmp: Add announce-self command
>   hmp: Add hmp_announce_self
>   tests: Add a test for qemu self announcments
> 
>  hmp-commands.hx                |  14 ++++
>  hmp.c                          |  33 ++++++++
>  hmp.h                          |   1 +
>  hw/net/trace-events            |   8 ++
>  hw/net/virtio-net.c            |  69 ++++++++++++----
>  include/hw/virtio/virtio-net.h |   4 +-
>  include/migration/misc.h       |  12 +--
>  include/net/announce.h         |  41 ++++++++++
>  include/net/net.h              |   2 +
>  include/qemu/typedefs.h        |   2 +
>  include/sysemu/sysemu.h        |   2 -
>  migration/migration.c          | 103 +++++++++++++++++++++++-
>  migration/migration.h          |   4 +
>  migration/savevm.c             |  72 +----------------
>  migration/trace-events         |   1 -
>  net/Makefile.objs              |   1 +
>  net/announce.c                 | 141 +++++++++++++++++++++++++++++++++
>  net/trace-events               |   4 +
>  qapi/migration.json            |  56 ++++++++++++-
>  qapi/net.json                  |  43 ++++++++++
>  tests/Makefile.include         |   2 +
>  tests/test-announce-self.c     |  83 +++++++++++++++++++
>  tests/test-hmp.c               |   1 +
>  23 files changed, 595 insertions(+), 104 deletions(-)
>  create mode 100644 include/net/announce.h
>  create mode 100644 net/announce.c
>  create mode 100644 tests/test-announce-self.c
> 
> -- 
> 2.20.1

Re: [Qemu-devel] [PATCH 0/9] Network announce changes

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Mon, Jan 28, 2019 at 05:03:12PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Hi,
> >   This is a reworking of a few sets of patches from 2017
> > that were put together by myself, Germano and Vlad that make
> > the network announce system more flexible.
> > 
> >   Firstly, it's parameterised, so that you can change the number
> > of packets and the gap between them; the number can be set to 0
> > to disable announce completely.
> > 
> >   Secondly, you can force an announce by a qmp or hmp command at
> > any time.  This is useful if you need the guest to do an announce
> > for a different reason; for example if the management layer
> > has just juggled some bonding configuration around.
> > 
> >   The packet creation and timing also moves to net/ from migration/
> > 
> >   The previous set was:
> >      https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg05594.html
> > 
> > Dave
> 
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Who's applying this? Jason?

That would seem most appropriate.

Dave

> 
> > Dr. David Alan Gilbert (9):
> >   net: Introduce announce timer
> >   migration: Add announce parameters
> >   virtio-net: Switch to using announce timer
> >   migration: Switch to using announce timer
> >   net: Add a network device specific self-announcement ability
> >   virtio-net: Allow qemu_announce_self to trigger virtio announcements
> >   qmp: Add announce-self command
> >   hmp: Add hmp_announce_self
> >   tests: Add a test for qemu self announcments
> > 
> >  hmp-commands.hx                |  14 ++++
> >  hmp.c                          |  33 ++++++++
> >  hmp.h                          |   1 +
> >  hw/net/trace-events            |   8 ++
> >  hw/net/virtio-net.c            |  69 ++++++++++++----
> >  include/hw/virtio/virtio-net.h |   4 +-
> >  include/migration/misc.h       |  12 +--
> >  include/net/announce.h         |  41 ++++++++++
> >  include/net/net.h              |   2 +
> >  include/qemu/typedefs.h        |   2 +
> >  include/sysemu/sysemu.h        |   2 -
> >  migration/migration.c          | 103 +++++++++++++++++++++++-
> >  migration/migration.h          |   4 +
> >  migration/savevm.c             |  72 +----------------
> >  migration/trace-events         |   1 -
> >  net/Makefile.objs              |   1 +
> >  net/announce.c                 | 141 +++++++++++++++++++++++++++++++++
> >  net/trace-events               |   4 +
> >  qapi/migration.json            |  56 ++++++++++++-
> >  qapi/net.json                  |  43 ++++++++++
> >  tests/Makefile.include         |   2 +
> >  tests/test-announce-self.c     |  83 +++++++++++++++++++
> >  tests/test-hmp.c               |   1 +
> >  23 files changed, 595 insertions(+), 104 deletions(-)
> >  create mode 100644 include/net/announce.h
> >  create mode 100644 net/announce.c
> >  create mode 100644 tests/test-announce-self.c
> > 
> > -- 
> > 2.20.1
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/9] net: Introduce announce timer

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1474 bytes --]

On 1/28/19 11:03 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The 'announce timer' will be used by migration, and explicit
> requests for qemu to perform network announces.
> 
> Based on the work by Germano Veit Michel <germano@redhat.com>
>  and Vlad Yasevich <vyasevic@redhat.com>
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

> +++ b/qapi/net.json
> @@ -684,3 +684,26 @@
>  ##
>  { 'event': 'NIC_RX_FILTER_CHANGED',
>    'data': { '*name': 'str', 'path': 'str' } }
> +
> +##
> +# @AnnounceParameters:
> +#
> +# Parameters for self-announce timers
> +#
> +# @initial: Initial delay (in ms) before sending the first GARP/RARP
> +#       announcement
> +#
> +# @max: Maximum delay (in ms) to between GARP/RARP announcement packets

"to between" should be either "to use in between" or merely "between"

> +#
> +# @rounds: Number of self-announcement attempts
> +#
> +# @step: Delay increase (in ms) after each self-announcement attempt
> +#
> +# Since: 4.0
> +##
> +
> +{ 'struct': 'AnnounceParameters',
> +  'data': { 'initial': 'int',
> +            'max': 'int',
> +            'rounds': 'int',
> +            'step': 'int' } }
> 

Can/should any of the parameters be optional with a sane (documented)
default?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH 2/9] migration: Add announce parameters

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3048 bytes --]

On 1/28/19 11:03 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add migration parameters that control RARP/GARP announcement timeouts.
> 
> Based on earlier patches by myself and
>   Vladislav Yasevich <vyasevic@redhat.com>
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

> +++ b/qapi/migration.json
> @@ -6,6 +6,7 @@
>  ##
>  
>  { 'include': 'common.json' }
> +{ 'include': 'net.json' }
>  
>  ##
>  # @MigrationStats:
> @@ -480,6 +481,18 @@
>  #
>  # Migration parameters enumeration
>  #
> +# @announce-initial: Inital delay (in ms) before sending the first announce

s/Inital/Initial/ [2]

> +#          (Since 4.0)
> +#
> +# @announce-max: Maximum delay (in ms) between packets in the announcment
> +#          (Since 4.0)
> +#
> +# @announce-rounds: Number of self-announce packets sent after migration
> +#          (Since 4.0)
> +#
> +# @announce-step: Increase in delay (in ms) between subsequent packets in
> +#          the announcement (Since 4.0)
> +#

The new parameters are optional below [1]; should they have a default
value documented, and are there any constraints such that if you set
one, you must set all four to match the previous' patch having all four
be non-optional?


> @@ -653,7 +681,11 @@
>  # TODO either fuse back into MigrationParameters, or make
>  # MigrationParameters members mandatory
>  { 'struct': 'MigrateSetParameters',
> -  'data': { '*compress-level': 'int',
> +  'data': { '*announce-initial': 'size',
> +            '*announce-max': 'size',
> +            '*announce-rounds': 'size',
> +            '*announce-step': 'size',
> +            '*compress-level': 'int',

[1] mentioned above

>              '*compress-threads': 'int',
>              '*compress-wait-thread': 'bool',
>              '*decompress-threads': 'int',
> @@ -692,6 +724,18 @@
>  #
>  # The optional members aren't actually optional.
>  #
> +# @announce-initial: Inital delay (in ms) before sending the first announce
> +#          (Since 4.0)

[2] again


> @@ -769,7 +813,11 @@
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> -  'data': { '*compress-level': 'uint8',
> +  'data': { '*announce-initial': 'size',
> +            '*announce-max': 'size',
> +            '*announce-rounds': 'size',
> +            '*announce-step': 'size',
> +            '*compress-level': 'uint8',

[1] again

>              '*compress-threads': 'uint8',
>              '*compress-wait-thread': 'bool',
>              '*decompress-threads': 'uint8',
> @@ -785,7 +833,7 @@
>              '*x-multifd-page-count': 'uint32',
>              '*xbzrle-cache-size': 'size',
>  	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-cpu-throttle':'uint8' } }

Why the whitespace churn?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH 7/9] qmp: Add announce-self command

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]

On 1/28/19 11:03 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add a qmp command that can trigger guest announcements.
> 
> It uses it's own announce-timer instance, and parameters

s/it's/its/ (here, you want the possessive form without apostrophe; the
contraction form is correct only when you can use "it is" in the same place)

> passed to it explicitly in the command.
> 
> Like most qmp commands, it's in the main thread/bql, so

Here, "it's" is correct.

> there's no racing with any outstanding timer.
> 
> Based on work of Germano Veit Michel <germano@redhat.com> and
>                  Vladislav Yasevich <vyasevic@redhat.com>
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  net/announce.c |  8 ++++++++
>  qapi/net.json  | 20 ++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 

> +++ b/qapi/net.json
> @@ -707,3 +707,23 @@
>              'max': 'int',
>              'rounds': 'int',
>              'step': 'int' } }
> +
> +##
> +# @announce-self:
> +#
> +# Trigger generation of broadcast RARP frames to update network switches.
> +# This can be useful when network bonds fail-over the active slave.
> +#
> +# @params: AnnounceParameters giving timing and repetition count of announce
> +#
> +# Example:
> +#
> +# -> { "execute": "announce-self"
> +#      "arguments": { "initial": 50, "max": 550, "rounds": 10, "step": 50 } }

Again, can any of these have useful defaults to be left optional?

> +# <- { "return": {} }
> +#
> +# Since: 4.0
> +##
> +{ 'command': 'announce-self',
> +  'data' : {'params': 'AnnounceParameters'} }
> +
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH 9/9] tests: Add a test for qemu self announcments

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (mst@redhat.com) wrote:
> On Mon, Jan 28, 2019 at 05:03:21PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > We now expose qemu_announce_self through QMP and HMP.  Add a test
> > with some very basic packet validation (make sure we get a RARP).
> > 
> > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> typo in subject

Thanks, fixed.

Dave

> > ---
> >  tests/Makefile.include     |  2 +
> >  tests/test-announce-self.c | 83 ++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 85 insertions(+)
> >  create mode 100644 tests/test-announce-self.c
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 19b4c0a696..57ce722d73 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -143,6 +143,8 @@ check-qtest-generic-y += tests/qmp-cmd-test$(EXESUF)
> >  
> >  check-qtest-generic-y += tests/device-introspect-test$(EXESUF)
> >  check-qtest-generic-y += tests/cdrom-test$(EXESUF)
> > +check-qtest-generic-y += tests/test-announce-self$(EXESUF)
> > +gcov-files-generic-y = migration/savevm.c
> >  
> >  check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
> >  
> > diff --git a/tests/test-announce-self.c b/tests/test-announce-self.c
> > new file mode 100644
> > index 0000000000..2ff0c77255
> > --- /dev/null
> > +++ b/tests/test-announce-self.c
> > @@ -0,0 +1,83 @@
> > +/*
> > + * QTest testcase for qemu_announce_self
> > + *
> > + * Copyright (c) 2017 Red hat, Inc.
> > + * Copyright (c) 2014 SUSE LINUX Products GmbH
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "libqtest.h"
> > +#include "qapi/qmp/qdict.h"
> > +#include "qemu-common.h"
> > +#include "qemu/sockets.h"
> > +#include "qemu/iov.h"
> > +#include "libqos/libqos-pc.h"
> > +#include "libqos/libqos-spapr.h"
> > +
> > +#ifndef ETH_P_RARP
> > +#define ETH_P_RARP 0x8035
> > +#endif
> > +
> > +static QTestState *test_init(int socket)
> > +{
> > +    char *args;
> > +
> > +    args = g_strdup_printf("-netdev socket,fd=%d,id=hs0 -device "
> > +                           "virtio-net-pci,netdev=hs0", socket);
> > +
> > +    return qtest_start(args);
> > +}
> > +
> > +
> > +static void test_announce(int socket)
> > +{
> > +    char buffer[60];
> > +    int len;
> > +    QDict *rsp;
> > +    int ret;
> > +    uint16_t *proto = (uint16_t *)&buffer[12];
> > +
> > +    rsp = qmp("{ 'execute' : 'announce-self', "
> > +                  " 'arguments': {"
> > +                      " 'params': {"
> > +                          " 'initial': 50, 'max': 550,"
> > +                          " 'rounds': 10, 'step': 50 } } }");
> > +    assert(!qdict_haskey(rsp, "error"));
> > +    qobject_unref(rsp);
> > +
> > +    /* Catch the packet and make sure it's a RARP */
> > +    ret = qemu_recv(socket, &len, sizeof(len), 0);
> > +    g_assert_cmpint(ret, ==,  sizeof(len));
> > +    len = ntohl(len);
> > +
> > +    ret = qemu_recv(socket, buffer, len, 0);
> > +    g_assert_cmpint(*proto, ==, htons(ETH_P_RARP));
> > +}
> > +
> > +static void setup(gconstpointer data)
> > +{
> > +    QTestState *qs;
> > +    void (*func) (int socket) = data;
> > +    int sv[2], ret;
> > +
> > +    ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
> > +    g_assert_cmpint(ret, !=, -1);
> > +
> > +    qs = test_init(sv[1]);
> > +    func(sv[0]);
> > +
> > +    /* End test */
> > +    close(sv[0]);
> > +    qtest_quit(qs);
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    g_test_init(&argc, &argv, NULL);
> > +    qtest_add_data_func("/virtio/net/test_announce_self", test_announce, setup);
> > +
> > +    return g_test_run();
> > +}
> > -- 
> > 2.20.1
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/9] net: Introduce announce timer

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 1/28/19 11:03 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > The 'announce timer' will be used by migration, and explicit
> > requests for qemu to perform network announces.
> > 
> > Based on the work by Germano Veit Michel <germano@redhat.com>
> >  and Vlad Yasevich <vyasevic@redhat.com>
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> 
> > +++ b/qapi/net.json
> > @@ -684,3 +684,26 @@
> >  ##
> >  { 'event': 'NIC_RX_FILTER_CHANGED',
> >    'data': { '*name': 'str', 'path': 'str' } }
> > +
> > +##
> > +# @AnnounceParameters:
> > +#
> > +# Parameters for self-announce timers
> > +#
> > +# @initial: Initial delay (in ms) before sending the first GARP/RARP
> > +#       announcement
> > +#
> > +# @max: Maximum delay (in ms) to between GARP/RARP announcement packets
> 
> "to between" should be either "to use in between" or merely "between"

I've changed it to the plain 'between'.

> > +#
> > +# @rounds: Number of self-announcement attempts
> > +#
> > +# @step: Delay increase (in ms) after each self-announcement attempt
> > +#
> > +# Since: 4.0
> > +##
> > +
> > +{ 'struct': 'AnnounceParameters',
> > +  'data': { 'initial': 'int',
> > +            'max': 'int',
> > +            'rounds': 'int',
> > +            'step': 'int' } }
> > 
> 
> Can/should any of the parameters be optional with a sane (documented)
> default?

I think last time Dan was arguing for it to be explicit, the only
default we could pick are the values we currently use, which I think are
only defendable as 'seem to have worked well so far'.

Dave

> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 2/9] migration: Add announce parameters

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 1/28/19 11:03 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Add migration parameters that control RARP/GARP announcement timeouts.
> > 
> > Based on earlier patches by myself and
> >   Vladislav Yasevich <vyasevic@redhat.com>
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> 
> > +++ b/qapi/migration.json
> > @@ -6,6 +6,7 @@
> >  ##
> >  
> >  { 'include': 'common.json' }
> > +{ 'include': 'net.json' }
> >  
> >  ##
> >  # @MigrationStats:
> > @@ -480,6 +481,18 @@
> >  #
> >  # Migration parameters enumeration
> >  #
> > +# @announce-initial: Inital delay (in ms) before sending the first announce
> 
> s/Inital/Initial/ [2]

Fixed.

> > +#          (Since 4.0)
> > +#
> > +# @announce-max: Maximum delay (in ms) between packets in the announcment
> > +#          (Since 4.0)
> > +#
> > +# @announce-rounds: Number of self-announce packets sent after migration
> > +#          (Since 4.0)
> > +#
> > +# @announce-step: Increase in delay (in ms) between subsequent packets in
> > +#          the announcement (Since 4.0)
> > +#
> 
> The new parameters are optional below [1]; should they have a default
> value documented, and are there any constraints such that if you set
> one, you must set all four to match the previous' patch having all four
> be non-optional?

I think this is just the same as all the other migration-parameters
isn't it?  It's the way that the 'migrate-set-parameters' command allows
you to set an individual parameter without changing the others; it's
not about defaults, just that whatever setting you currently have
doesn't change.

> 
> > @@ -653,7 +681,11 @@
> >  # TODO either fuse back into MigrationParameters, or make
> >  # MigrationParameters members mandatory
> >  { 'struct': 'MigrateSetParameters',
> > -  'data': { '*compress-level': 'int',
> > +  'data': { '*announce-initial': 'size',
> > +            '*announce-max': 'size',
> > +            '*announce-rounds': 'size',
> > +            '*announce-step': 'size',
> > +            '*compress-level': 'int',
> 
> [1] mentioned above
> 
> >              '*compress-threads': 'int',
> >              '*compress-wait-thread': 'bool',
> >              '*decompress-threads': 'int',
> > @@ -692,6 +724,18 @@
> >  #
> >  # The optional members aren't actually optional.
> >  #
> > +# @announce-initial: Inital delay (in ms) before sending the first announce
> > +#          (Since 4.0)
> 
> [2] again
> 
Fixed.
> 
> > @@ -769,7 +813,11 @@
> >  # Since: 2.4
> >  ##
> >  { 'struct': 'MigrationParameters',
> > -  'data': { '*compress-level': 'uint8',
> > +  'data': { '*announce-initial': 'size',
> > +            '*announce-max': 'size',
> > +            '*announce-rounds': 'size',
> > +            '*announce-step': 'size',
> > +            '*compress-level': 'uint8',
> 
> [1] again
> 
> >              '*compress-threads': 'uint8',
> >              '*compress-wait-thread': 'bool',
> >              '*decompress-threads': 'uint8',
> > @@ -785,7 +833,7 @@
> >              '*x-multifd-page-count': 'uint32',
> >              '*xbzrle-cache-size': 'size',
> >  	    '*max-postcopy-bandwidth': 'size',
> > -            '*max-cpu-throttle':'uint8'} }
> > +            '*max-cpu-throttle':'uint8' } }
> 
> Why the whitespace churn?

Removed (although I think the extra space is cleaner).

Dave


> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 7/9] qmp: Add announce-self command

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 1/28/19 11:03 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Add a qmp command that can trigger guest announcements.
> > 
> > It uses it's own announce-timer instance, and parameters
> 
> s/it's/its/ (here, you want the possessive form without apostrophe; the
> contraction form is correct only when you can use "it is" in the same place)

' removed

> > passed to it explicitly in the command.
> > 
> > Like most qmp commands, it's in the main thread/bql, so
> 
> Here, "it's" is correct.
> 
> > there's no racing with any outstanding timer.
> > 
> > Based on work of Germano Veit Michel <germano@redhat.com> and
> >                  Vladislav Yasevich <vyasevic@redhat.com>
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  net/announce.c |  8 ++++++++
> >  qapi/net.json  | 20 ++++++++++++++++++++
> >  2 files changed, 28 insertions(+)
> > 
> 
> > +++ b/qapi/net.json
> > @@ -707,3 +707,23 @@
> >              'max': 'int',
> >              'rounds': 'int',
> >              'step': 'int' } }
> > +
> > +##
> > +# @announce-self:
> > +#
> > +# Trigger generation of broadcast RARP frames to update network switches.
> > +# This can be useful when network bonds fail-over the active slave.
> > +#
> > +# @params: AnnounceParameters giving timing and repetition count of announce
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "announce-self"
> > +#      "arguments": { "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
> 
> Again, can any of these have useful defaults to be left optional?

Nope, you have to state what you want fully.
However, that line is actually wrong, it should be:
# -> { "execute": "announce-self"
#      "arguments": { "params:" {
#          "initial": 50, "max": 550, "rounds": 10, "step": 50 } } }

(As I found when I wrote the test).
I've fixed that up.

Dave

> > +# <- { "return": {} }
> > +#
> > +# Since: 4.0
> > +##
> > +{ 'command': 'announce-self',
> > +  'data' : {'params': 'AnnounceParameters'} }
> > +
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 



--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 0/9] Network announce changes

[Markus Armbruster]

git-am gripes:

Applying: net: Introduce announce timer
.git/rebase-apply/patch:159: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: migration: Add announce parameters
Applying: virtio-net: Switch to using announce timer
.git/rebase-apply/patch:20: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: migration: Switch to using announce timer
.git/rebase-apply/patch:309: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: net: Add a network device specific self-announcement ability
Applying: virtio-net: Allow qemu_announce_self to trigger virtio announcements
Applying: qmp: Add announce-self command
.git/rebase-apply/patch:28: new blank line at EOF.
+
.git/rebase-apply/patch:56: new blank line at EOF.
+
warning: 2 lines add whitespace errors.
Applying: hmp: Add hmp_announce_self
Applying: tests: Add a test for qemu self announcments

Re: [Qemu-devel] [PATCH 2/9] migration: Add announce parameters

[Markus Armbruster]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Add migration parameters that control RARP/GARP announcement timeouts.
>
> Based on earlier patches by myself and
>   Vladislav Yasevich <vyasevic@redhat.com>
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
[...]
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 7a795ecc16..868071d3a0 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -6,6 +6,7 @@
>  ##
>  
>  { 'include': 'common.json' }
> +{ 'include': 'net.json' }
>  
>  ##
>  # @MigrationStats:

This hunk looks superfluous.

> @@ -480,6 +481,18 @@
>  #
>  # Migration parameters enumeration
>  #
> +# @announce-initial: Inital delay (in ms) before sending the first announce
> +#          (Since 4.0)

Current master has one instance of ms in qapi/, 9 of milliseconds.  I'd
stick to the more common spelling.

> +#
> +# @announce-max: Maximum delay (in ms) between packets in the announcment
> +#          (Since 4.0)
> +#
> +# @announce-rounds: Number of self-announce packets sent after migration
> +#          (Since 4.0)
> +#
> +# @announce-step: Increase in delay (in ms) between subsequent packets in
> +#          the announcement (Since 4.0)
> +#
>  # @compress-level: Set the compression level to be used in live migration,
>  #          the compression level is an integer between 0 and 9, where 0 means
>  #          no compression, 1 means the best compression speed, and 9 means best
> @@ -557,10 +570,13 @@
>  #
>  # @max-cpu-throttle: maximum cpu throttle percentage.
>  #                    Defaults to 99. (Since 3.1)
> +#
>  # Since: 2.4
>  ##
>  { 'enum': 'MigrationParameter',
> -  'data': ['compress-level', 'compress-threads', 'decompress-threads',
> +  'data': ['announce-initial', 'announce-max',
> +           'announce-rounds', 'announce-step',
> +           'compress-level', 'compress-threads', 'decompress-threads',
>             'compress-wait-thread',
>             'cpu-throttle-initial', 'cpu-throttle-increment',
>             'tls-creds', 'tls-hostname', 'max-bandwidth',
> @@ -572,6 +588,18 @@
>  ##
>  # @MigrateSetParameters:
>  #
> +# @announce-initial: Inital delay (in ms) before sending the first announce
> +#          (Since 4.0)
> +#
> +# @announce-max: Maximum delay (in ms) between packets in the announcment
> +#          (Since 4.0)
> +#
> +# @announce-rounds: Number of self-announce packets sent after migration
> +#          (Since 4.0)
> +#
> +# @announce-step: Increase in delay (in ms) between subsequent packets in
> +#          the announcement (Since 4.0)
> +#
>  # @compress-level: compression level
>  #
>  # @compress-threads: compression thread count
> @@ -653,7 +681,11 @@
>  # TODO either fuse back into MigrationParameters, or make
>  # MigrationParameters members mandatory
>  { 'struct': 'MigrateSetParameters',
> -  'data': { '*compress-level': 'int',
> +  'data': { '*announce-initial': 'size',
> +            '*announce-max': 'size',
> +            '*announce-rounds': 'size',
> +            '*announce-step': 'size',
> +            '*compress-level': 'int',
>              '*compress-threads': 'int',
>              '*compress-wait-thread': 'bool',
>              '*decompress-threads': 'int',
> @@ -692,6 +724,18 @@
>  #
>  # The optional members aren't actually optional.
>  #
> +# @announce-initial: Inital delay (in ms) before sending the first announce
> +#          (Since 4.0)
> +#
> +# @announce-max: Maximum delay (in ms) between packets in the announcment
> +#          (Since 4.0)
> +#
> +# @announce-rounds: Number of self-announce packets sent after migration
> +#          (Since 4.0)
> +#
> +# @announce-step: Increase in delay (in ms) between subsequent packets in
> +#          the announcement (Since 4.0)
> +#
>  # @compress-level: compression level
>  #
>  # @compress-threads: compression thread count
> @@ -769,7 +813,11 @@
>  # Since: 2.4
>  ##
>  { 'struct': 'MigrationParameters',
> -  'data': { '*compress-level': 'uint8',
> +  'data': { '*announce-initial': 'size',
> +            '*announce-max': 'size',
> +            '*announce-rounds': 'size',
> +            '*announce-step': 'size',
> +            '*compress-level': 'uint8',
>              '*compress-threads': 'uint8',
>              '*compress-wait-thread': 'bool',
>              '*decompress-threads': 'uint8',
> @@ -785,7 +833,7 @@
>              '*x-multifd-page-count': 'uint32',
>              '*xbzrle-cache-size': 'size',
>  	    '*max-postcopy-bandwidth': 'size',
> -            '*max-cpu-throttle':'uint8'} }
> +            '*max-cpu-throttle':'uint8' } }
>  
>  ##
>  # @query-migrate-parameters:

Since you're tidying up whitespace, consider expanding the tab.

With the superfluous hunk dropped, QAPI part
Acked-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH 0/9] Network announce changes

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190128170321.16936-1-dgilbert@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14
=== TEST SCRIPT END ===


Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install --python=/usr/bin/python3 --cross-prefix=x86_64-w64-mingw32- --enable-trace-backends=simple --enable-gnutls --enable-nettle --enable-curl --enable-vnc --enable-bzip2 --enable-guest-agent --with-sdlabi=2.0
ERROR: unknown option --with-sdlabi=2.0
Try '/tmp/qemu-test/src/configure --help' for more information
# QEMU configure log Sun Feb  3 15:52:37 UTC 2019
# Configured with: '/tmp/qemu-test/src/configure' '--enable-werror' '--target-list=x86_64-softmmu,aarch64-softmmu' '--prefix=/tmp/qemu-test/install' '--python=/usr/bin/python3' '--cross-prefix=x86_64-w64-mingw32-' '--enable-trace-backends=simple' '--enable-gnutls' '--enable-nettle' '--enable-curl' '--enable-vnc' '--enable-bzip2' '--enable-guest-agent' '--with-sdlabi=2.0'
---
funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 617 634 0
x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
config-temp/qemu-conf.c:2:2: error: #error __linux__ not defined
 #error __linux__ not defined
  ^~~~~

---
funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 617 686 0
x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
config-temp/qemu-conf.c:2:2: error: #error __i386__ not defined
 #error __i386__ not defined
  ^~~~~

---
funcs: do_compiler do_cc compile_object check_define main
lines: 92 122 617 689 0
x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c
config-temp/qemu-conf.c:2:2: error: #error __ILP32__ not defined
 #error __ILP32__ not defined
  ^~~~~

---
lines: 92 128 920 0
x86_64-w64-mingw32-gcc -mthreads -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -g -liberty
/usr/lib/gcc/x86_64-w64-mingw32/8.2.0/../../../../x86_64-w64-mingw32/bin/ld: cannot find -liberty
collect2: error: ld returned 1 exit status
Failed to run 'configure'
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 563, in <module>


The full log is available at
http://patchew.org/logs/20190128170321.16936-1-dgilbert@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/9] Network announce changes

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> git-am gripes:

Thanks, fixed.

Dave

> Applying: net: Introduce announce timer
> .git/rebase-apply/patch:159: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
> Applying: migration: Add announce parameters
> Applying: virtio-net: Switch to using announce timer
> .git/rebase-apply/patch:20: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
> Applying: migration: Switch to using announce timer
> .git/rebase-apply/patch:309: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.
> Applying: net: Add a network device specific self-announcement ability
> Applying: virtio-net: Allow qemu_announce_self to trigger virtio announcements
> Applying: qmp: Add announce-self command
> .git/rebase-apply/patch:28: new blank line at EOF.
> +
> .git/rebase-apply/patch:56: new blank line at EOF.
> +
> warning: 2 lines add whitespace errors.
> Applying: hmp: Add hmp_announce_self
> Applying: tests: Add a test for qemu self announcments
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 2/9] migration: Add announce parameters

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:
> 
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Add migration parameters that control RARP/GARP announcement timeouts.
> >
> > Based on earlier patches by myself and
> >   Vladislav Yasevich <vyasevic@redhat.com>
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> [...]
> > diff --git a/qapi/migration.json b/qapi/migration.json
> > index 7a795ecc16..868071d3a0 100644
> > --- a/qapi/migration.json
> > +++ b/qapi/migration.json
> > @@ -6,6 +6,7 @@
> >  ##
> >  
> >  { 'include': 'common.json' }
> > +{ 'include': 'net.json' }
> >  
> >  ##
> >  # @MigrationStats:
> 
> This hunk looks superfluous.

Gone.

> > @@ -480,6 +481,18 @@
> >  #
> >  # Migration parameters enumeration
> >  #
> > +# @announce-initial: Inital delay (in ms) before sending the first announce
> > +#          (Since 4.0)
> 
> Current master has one instance of ms in qapi/, 9 of milliseconds.  I'd
> stick to the more common spelling.

OK, changed (with some wrapping changed now the lines are longer, and an
added 'e' in announcment).

> > +#
> > +# @announce-max: Maximum delay (in ms) between packets in the announcment
> > +#          (Since 4.0)
> > +#
> > +# @announce-rounds: Number of self-announce packets sent after migration
> > +#          (Since 4.0)
> > +#
> > +# @announce-step: Increase in delay (in ms) between subsequent packets in
> > +#          the announcement (Since 4.0)
> > +#
> >  # @compress-level: Set the compression level to be used in live migration,
> >  #          the compression level is an integer between 0 and 9, where 0 means
> >  #          no compression, 1 means the best compression speed, and 9 means best
> > @@ -557,10 +570,13 @@
> >  #
> >  # @max-cpu-throttle: maximum cpu throttle percentage.
> >  #                    Defaults to 99. (Since 3.1)
> > +#
> >  # Since: 2.4
> >  ##
> >  { 'enum': 'MigrationParameter',
> > -  'data': ['compress-level', 'compress-threads', 'decompress-threads',
> > +  'data': ['announce-initial', 'announce-max',
> > +           'announce-rounds', 'announce-step',
> > +           'compress-level', 'compress-threads', 'decompress-threads',
> >             'compress-wait-thread',
> >             'cpu-throttle-initial', 'cpu-throttle-increment',
> >             'tls-creds', 'tls-hostname', 'max-bandwidth',
> > @@ -572,6 +588,18 @@
> >  ##
> >  # @MigrateSetParameters:
> >  #
> > +# @announce-initial: Inital delay (in ms) before sending the first announce
> > +#          (Since 4.0)
> > +#
> > +# @announce-max: Maximum delay (in ms) between packets in the announcment
> > +#          (Since 4.0)
> > +#
> > +# @announce-rounds: Number of self-announce packets sent after migration
> > +#          (Since 4.0)
> > +#
> > +# @announce-step: Increase in delay (in ms) between subsequent packets in
> > +#          the announcement (Since 4.0)
> > +#
> >  # @compress-level: compression level
> >  #
> >  # @compress-threads: compression thread count
> > @@ -653,7 +681,11 @@
> >  # TODO either fuse back into MigrationParameters, or make
> >  # MigrationParameters members mandatory
> >  { 'struct': 'MigrateSetParameters',
> > -  'data': { '*compress-level': 'int',
> > +  'data': { '*announce-initial': 'size',
> > +            '*announce-max': 'size',
> > +            '*announce-rounds': 'size',
> > +            '*announce-step': 'size',
> > +            '*compress-level': 'int',
> >              '*compress-threads': 'int',
> >              '*compress-wait-thread': 'bool',
> >              '*decompress-threads': 'int',
> > @@ -692,6 +724,18 @@
> >  #
> >  # The optional members aren't actually optional.
> >  #
> > +# @announce-initial: Inital delay (in ms) before sending the first announce
> > +#          (Since 4.0)
> > +#
> > +# @announce-max: Maximum delay (in ms) between packets in the announcment
> > +#          (Since 4.0)
> > +#
> > +# @announce-rounds: Number of self-announce packets sent after migration
> > +#          (Since 4.0)
> > +#
> > +# @announce-step: Increase in delay (in ms) between subsequent packets in
> > +#          the announcement (Since 4.0)
> > +#
> >  # @compress-level: compression level
> >  #
> >  # @compress-threads: compression thread count
> > @@ -769,7 +813,11 @@
> >  # Since: 2.4
> >  ##
> >  { 'struct': 'MigrationParameters',
> > -  'data': { '*compress-level': 'uint8',
> > +  'data': { '*announce-initial': 'size',
> > +            '*announce-max': 'size',
> > +            '*announce-rounds': 'size',
> > +            '*announce-step': 'size',
> > +            '*compress-level': 'uint8',
> >              '*compress-threads': 'uint8',
> >              '*compress-wait-thread': 'bool',
> >              '*decompress-threads': 'uint8',
> > @@ -785,7 +833,7 @@
> >              '*x-multifd-page-count': 'uint32',
> >              '*xbzrle-cache-size': 'size',
> >  	    '*max-postcopy-bandwidth': 'size',
> > -            '*max-cpu-throttle':'uint8'} }
> > +            '*max-cpu-throttle':'uint8' } }
> >  
> >  ##
> >  # @query-migrate-parameters:
> 
> Since you're tidying up whitespace, consider expanding the tab.

I'd removed that cleanup based on another review.

> With the superfluous hunk dropped, QAPI part
> Acked-by: Markus Armbruster <armbru@redhat.com>

Thanks,

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK