* [Qemu-devel] [PATCH v3 0/4] network announce; interface selection & IDs
@ 2019-06-10 18:43 Dr. David Alan Gilbert (git)
2019-06-10 18:43 ` [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces Dr. David Alan Gilbert (git)
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-06-10 18:43 UTC (permalink / raw)
To: qemu-devel, jasowang, eblake, armbru, laine
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Laine asked for some extra features on the network announce support;
The first allows the announce timer to announce on a subset of the
interfaces.
The second allows there to be multiple timers, each with their own
parameters (including the interface list).
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
v3
Support for multiple timers.
Dr. David Alan Gilbert (4):
net/announce: Allow optional list of interfaces
net/announce: Add HMP optional interface list
net/announce: Add optional ID
net/announce: Add HMP optional ID
hmp-commands.hx | 7 +++-
hmp.c | 41 +++++++++++++++++++-
hw/net/virtio-net.c | 4 +-
include/net/announce.h | 8 +++-
net/announce.c | 83 ++++++++++++++++++++++++++++++++++-------
net/trace-events | 3 +-
qapi/net.json | 16 ++++++--
tests/virtio-net-test.c | 2 +-
8 files changed, 139 insertions(+), 25 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces
2019-06-10 18:43 [Qemu-devel] [PATCH v3 0/4] network announce; interface selection & IDs Dr. David Alan Gilbert (git)
@ 2019-06-10 18:43 ` Dr. David Alan Gilbert (git)
2019-06-10 19:47 ` Eric Blake
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 2/4] net/announce: Add HMP optional interface list Dr. David Alan Gilbert (git)
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-06-10 18:43 UTC (permalink / raw)
To: qemu-devel, jasowang, eblake, armbru, laine
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Allow the caller to restrict the set of interfaces that announces are
sent on. The default is still to send on all interfaces.
e.g.
{ "execute": "announce-self", "arguments": { "initial": 50, "max": 550, "rounds": 5, "step": 50, "interfaces": ["vn2","vn1"] } }
This doesn't affect the behaviour of migraiton announcments.
Note: There's still only one timer for the qmp command, so that
performing an 'announce-self' on one list of interfaces followed
by another 'announce-self' on another list will stop the announces
on the existing set.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
include/net/announce.h | 2 +-
net/announce.c | 39 ++++++++++++++++++++++++++++++++-------
net/trace-events | 2 +-
qapi/net.json | 11 ++++++++---
4 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/include/net/announce.h b/include/net/announce.h
index 892d302b65..3ebffe517e 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -23,7 +23,7 @@ struct AnnounceTimer {
/* Returns: update the timer to the next time point */
int64_t qemu_announce_timer_step(AnnounceTimer *timer);
-/* Delete the underlying timer */
+/* Delete the underlying timer and other data */
void qemu_announce_timer_del(AnnounceTimer *timer);
/*
diff --git a/net/announce.c b/net/announce.c
index 91e9a6e267..1ce42b571d 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -38,6 +38,8 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
timer_free(timer->tm);
timer->tm = NULL;
}
+ qapi_free_strList(timer->params.interfaces);
+ timer->params.interfaces = NULL;
}
/*
@@ -96,24 +98,47 @@ static int announce_self_create(uint8_t *buf,
static void qemu_announce_self_iter(NICState *nic, void *opaque)
{
+ AnnounceTimer *timer = opaque;
uint8_t buf[60];
int len;
+ bool skip;
+
+ if (timer->params.has_interfaces) {
+ strList *entry = timer->params.interfaces;
+ /* Skip unless we find our name in the requested list */
+ skip = true;
+
+ while (entry) {
+ if (!strcmp(entry->value, nic->ncs->name)) {
+ /* Found us */
+ skip = false;
+ break;
+ }
+ entry = entry->next;
+ }
+ } else {
+ skip = false;
+ }
+
+ trace_qemu_announce_self_iter(nic->ncs->name,
+ qemu_ether_ntoa(&nic->conf->macaddr), skip);
- trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
- len = announce_self_create(buf, nic->conf->macaddr.a);
+ if (!skip) {
+ len = announce_self_create(buf, nic->conf->macaddr.a);
- qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
+ qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
- /* if the NIC provides it's own announcement support, use it as well */
- if (nic->ncs->info->announce) {
- nic->ncs->info->announce(nic->ncs);
+ /* if the NIC provides it's own announcement support, use it as well */
+ if (nic->ncs->info->announce) {
+ nic->ncs->info->announce(nic->ncs);
+ }
}
}
static void qemu_announce_self_once(void *opaque)
{
AnnounceTimer *timer = (AnnounceTimer *)opaque;
- qemu_foreach_nic(qemu_announce_self_iter, NULL);
+ qemu_foreach_nic(qemu_announce_self_iter, timer);
if (--timer->round) {
qemu_announce_timer_step(timer);
diff --git a/net/trace-events b/net/trace-events
index a7937f3f3a..875ef2a0f3 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,7 +1,7 @@
# See docs/devel/tracing.txt for syntax documentation.
# announce.c
-qemu_announce_self_iter(const char *mac) "%s"
+qemu_announce_self_iter(const char *name, const char *mac, int skip) "%s:%s skip: %d"
# vhost-user.c
vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
diff --git a/qapi/net.json b/qapi/net.json
index 5f7bff1637..ee9bf2c5f5 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -699,6 +699,9 @@
#
# @step: Delay increase (in ms) after each self-announcement attempt
#
+# @interfaces: An optional list of interface names, which restrict the
+# announcment to the listed interfaces. (Since 4.1)
+#
# Since: 4.0
##
@@ -706,7 +709,8 @@
'data': { 'initial': 'int',
'max': 'int',
'rounds': 'int',
- 'step': 'int' } }
+ 'step': 'int',
+ '*interfaces': ['str'] } }
##
# @announce-self:
@@ -718,9 +722,10 @@
#
# Example:
#
-# -> { "execute": "announce-self"
+# -> { "execute": "announce-self",
# "arguments": {
-# "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
+# "initial": 50, "max": 550, "rounds": 10, "step": 50,
+# "interfaces": ["vn2","vn3"] } }
# <- { "return": {} }
#
# Since: 4.0
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 2/4] net/announce: Add HMP optional interface list
2019-06-10 18:43 [Qemu-devel] [PATCH v3 0/4] network announce; interface selection & IDs Dr. David Alan Gilbert (git)
2019-06-10 18:43 ` [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces Dr. David Alan Gilbert (git)
@ 2019-06-10 18:44 ` Dr. David Alan Gilbert (git)
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID Dr. David Alan Gilbert (git)
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 4/4] net/announce: Add HMP " Dr. David Alan Gilbert (git)
3 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-06-10 18:44 UTC (permalink / raw)
To: qemu-devel, jasowang, eblake, armbru, laine
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Add the optional interface list to the HMP command.
i.e.
All interfaces
announce_self
Just the named interfaces:
announce_self vn1,vn2
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hmp-commands.hx | 6 ++++--
hmp.c | 38 +++++++++++++++++++++++++++++++++++++-
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 3dc421cb6a..1b63372713 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -955,8 +955,8 @@ ETEXI
{
.name = "announce_self",
- .args_type = "",
- .params = "",
+ .args_type = "interfaces:s?",
+ .params = "[interfaces]",
.help = "Trigger GARP/RARP announcements",
.cmd = hmp_announce_self,
},
@@ -967,6 +967,8 @@ STEXI
Trigger a round of GARP/RARP broadcasts; this is useful for explicitly updating the
network infrastructure after a reconfiguration or some forms of migration.
The timings of the round are set by the migration announce parameters.
+An optional comma separated @var{interfaces} list restricts the announce to the
+named set of interfaces.
ETEXI
{
diff --git a/hmp.c b/hmp.c
index d4460992b6..52efb4a4fa 100644
--- a/hmp.c
+++ b/hmp.c
@@ -27,6 +27,7 @@
#include "monitor/monitor.h"
#include "monitor/qdev.h"
#include "qapi/error.h"
+#include "qapi/clone-visitor.h"
#include "qapi/opts-visitor.h"
#include "qapi/qapi-builtin-visit.h"
#include "qapi/qapi-commands-block.h"
@@ -38,6 +39,7 @@
#include "qapi/qapi-commands-run-state.h"
#include "qapi/qapi-commands-tpm.h"
#include "qapi/qapi-commands-ui.h"
+#include "qapi/qapi-visit-net.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qapi/string-input-visitor.h"
@@ -67,6 +69,32 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
}
}
+/*
+ * Produce a strList from a comma separated list.
+ * A NULL or empty input string return NULL.
+ */
+static strList *strList_from_comma_list(const char *in)
+{
+ strList *res = NULL;
+ strList **hook = &res;
+
+ while (in && in[0]) {
+ char *comma = strchr(in, ',');
+ *hook = g_new0(strList, 1);
+
+ if (comma) {
+ (*hook)->value = g_strndup(in, comma - in);
+ in = comma + 1; /* skip the , */
+ } else {
+ (*hook)->value = g_strdup(in);
+ in = NULL;
+ }
+ hook = &(*hook)->next;
+ }
+
+ return res;
+}
+
void hmp_info_name(Monitor *mon, const QDict *qdict)
{
NameInfo *info;
@@ -1640,7 +1668,15 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
void hmp_announce_self(Monitor *mon, const QDict *qdict)
{
- qmp_announce_self(migrate_announce_params(), NULL);
+ const char *interfaces_str = qdict_get_try_str(qdict, "interfaces");
+ AnnounceParameters *params = QAPI_CLONE(AnnounceParameters,
+ migrate_announce_params());
+
+ qapi_free_strList(params->interfaces);
+ params->interfaces = strList_from_comma_list(interfaces_str);
+ params->has_interfaces = params->interfaces != NULL;
+ qmp_announce_self(params, NULL);
+ qapi_free_AnnounceParameters(params);
}
void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID
2019-06-10 18:43 [Qemu-devel] [PATCH v3 0/4] network announce; interface selection & IDs Dr. David Alan Gilbert (git)
2019-06-10 18:43 ` [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces Dr. David Alan Gilbert (git)
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 2/4] net/announce: Add HMP optional interface list Dr. David Alan Gilbert (git)
@ 2019-06-10 18:44 ` Dr. David Alan Gilbert (git)
2019-06-10 19:53 ` Eric Blake
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 4/4] net/announce: Add HMP " Dr. David Alan Gilbert (git)
3 siblings, 1 reply; 9+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-06-10 18:44 UTC (permalink / raw)
To: qemu-devel, jasowang, eblake, armbru, laine
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Previously there was a single instance of the timer used by
monitor triggered announces, that's OK, but when combined with the
previous change that lets you have announces for subsets of interfaces
it's a bit restrictive if you want to do different things to different
interfaces.
Add an 'id' field to the announce, and maintain a list of the
timers based on id.
This allows you to for example:
a) Start an announce going on interface eth0 for a long time
b) Start an announce going on interface eth1 for a long time
c) Kill the announce on eth0 while leaving eth1 going.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hw/net/virtio-net.c | 4 ++--
include/net/announce.h | 8 +++++--
net/announce.c | 46 ++++++++++++++++++++++++++++++++++-------
net/trace-events | 3 ++-
qapi/net.json | 9 ++++++--
tests/virtio-net-test.c | 2 +-
6 files changed, 57 insertions(+), 15 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index ffe0872fff..120248bbf6 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2359,7 +2359,7 @@ static int virtio_net_post_load_device(void *opaque, int version_id)
timer_mod(n->announce_timer.tm,
qemu_clock_get_ms(n->announce_timer.type));
} else {
- qemu_announce_timer_del(&n->announce_timer);
+ qemu_announce_timer_del(&n->announce_timer, false);
}
}
@@ -2783,7 +2783,7 @@ static void virtio_net_device_unrealize(DeviceState *dev, Error **errp)
virtio_net_del_queue(n, i);
}
- qemu_announce_timer_del(&n->announce_timer);
+ qemu_announce_timer_del(&n->announce_timer, false);
g_free(n->vqs);
qemu_del_nic(n->nic);
virtio_net_rsc_cleanup(n);
diff --git a/include/net/announce.h b/include/net/announce.h
index 3ebffe517e..9cf7a4f835 100644
--- a/include/net/announce.h
+++ b/include/net/announce.h
@@ -23,8 +23,12 @@ struct AnnounceTimer {
/* Returns: update the timer to the next time point */
int64_t qemu_announce_timer_step(AnnounceTimer *timer);
-/* Delete the underlying timer and other data */
-void qemu_announce_timer_del(AnnounceTimer *timer);
+/*
+ * Delete the underlying timer and other datas
+ * If 'free_named' true and the timer is a named timer, then remove
+ * it from the list of named timers and free the AnnounceTimer itself.
+ */
+void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named);
/*
* Under BQL/main thread
diff --git a/net/announce.c b/net/announce.c
index 1ce42b571d..4d4e2c22a1 100644
--- a/net/announce.c
+++ b/net/announce.c
@@ -15,6 +15,8 @@
#include "qapi/qapi-commands-net.h"
#include "trace.h"
+static GData *named_timers;
+
int64_t qemu_announce_timer_step(AnnounceTimer *timer)
{
int64_t step;
@@ -31,8 +33,13 @@ int64_t qemu_announce_timer_step(AnnounceTimer *timer)
return step;
}
-void qemu_announce_timer_del(AnnounceTimer *timer)
+/*
+ * If 'free_named' is true, then remove the timer from the list
+ * and free the timer itself.
+ */
+void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named)
{
+ bool free_timer = false;
if (timer->tm) {
timer_del(timer->tm);
timer_free(timer->tm);
@@ -40,6 +47,18 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
}
qapi_free_strList(timer->params.interfaces);
timer->params.interfaces = NULL;
+ if (free_named && timer->params.has_id) {
+ free_timer = timer ==
+ g_datalist_get_data(&named_timers, timer->params.id);
+ g_datalist_remove_data(&named_timers, timer->params.id);
+ }
+ trace_qemu_announce_timer_del(free_named, free_timer, timer->params.id);
+ g_free(timer->params.id);
+ timer->params.id = NULL;
+
+ if (free_timer) {
+ g_free(timer);
+ }
}
/*
@@ -56,7 +75,7 @@ void qemu_announce_timer_reset(AnnounceTimer *timer,
* We're under the BQL, so the current timer can't
* be firing, so we should be able to delete it.
*/
- qemu_announce_timer_del(timer);
+ qemu_announce_timer_del(timer, false);
QAPI_CLONE_MEMBERS(AnnounceParameters, &timer->params, params);
timer->round = params->rounds;
@@ -120,7 +139,8 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
skip = false;
}
- trace_qemu_announce_self_iter(nic->ncs->name,
+ trace_qemu_announce_self_iter(timer->params.has_id ? timer->params.id : "_",
+ nic->ncs->name,
qemu_ether_ntoa(&nic->conf->macaddr), skip);
if (!skip) {
@@ -143,7 +163,7 @@ static void qemu_announce_self_once(void *opaque)
if (--timer->round) {
qemu_announce_timer_step(timer);
} else {
- qemu_announce_timer_del(timer);
+ qemu_announce_timer_del(timer, true);
}
}
@@ -154,12 +174,24 @@ void qemu_announce_self(AnnounceTimer *timer, AnnounceParameters *params)
if (params->rounds) {
qemu_announce_self_once(timer);
} else {
- qemu_announce_timer_del(timer);
+ qemu_announce_timer_del(timer, true);
}
}
void qmp_announce_self(AnnounceParameters *params, Error **errp)
{
- static AnnounceTimer announce_timer;
- qemu_announce_self(&announce_timer, params);
+ AnnounceTimer *named_timer;
+ if (!params->has_id) {
+ params->id = g_strdup("");
+ params->has_id = true;
+ }
+
+ named_timer = g_datalist_get_data(&named_timers, params->id);
+
+ if (!named_timer) {
+ named_timer = g_new0(AnnounceTimer, 1);
+ g_datalist_set_data(&named_timers, params->id, named_timer);
+ }
+
+ qemu_announce_self(named_timer, params);
}
diff --git a/net/trace-events b/net/trace-events
index 875ef2a0f3..ac57056497 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -1,7 +1,8 @@
# See docs/devel/tracing.txt for syntax documentation.
# announce.c
-qemu_announce_self_iter(const char *name, const char *mac, int skip) "%s:%s skip: %d"
+qemu_announce_self_iter(const char *id, const char *name, const char *mac, int skip) "%s:%s:%s skip: %d"
+qemu_announce_timer_del(bool free_named, bool free_timer, char *id) "free named: %d free timer: %d id: %s"
# vhost-user.c
vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
diff --git a/qapi/net.json b/qapi/net.json
index ee9bf2c5f5..9da9b5bbc5 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -702,6 +702,10 @@
# @interfaces: An optional list of interface names, which restrict the
# announcment to the listed interfaces. (Since 4.1)
#
+# @id: A name to be used to identify an instance of announce-timers
+# and to allow it to modified later. Not for use as
+# part of the migration paramters. (Since 4.1)
+#
# Since: 4.0
##
@@ -710,7 +714,8 @@
'max': 'int',
'rounds': 'int',
'step': 'int',
- '*interfaces': ['str'] } }
+ '*interfaces': ['str'],
+ '*id' : 'str' } }
##
# @announce-self:
@@ -725,7 +730,7 @@
# -> { "execute": "announce-self",
# "arguments": {
# "initial": 50, "max": 550, "rounds": 10, "step": 50,
-# "interfaces": ["vn2","vn3"] } }
+# "interfaces": ["vn2","vn3"], "id": "bob" } }
# <- { "return": {} }
#
# Since: 4.0
diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 163126cf07..7184e2bff4 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -186,7 +186,7 @@ static void announce_self(void *obj, void *data, QGuestAllocator *t_alloc)
rsp = qmp("{ 'execute' : 'announce-self', "
" 'arguments': {"
" 'initial': 50, 'max': 550,"
- " 'rounds': 10, 'step': 50 } }");
+ " 'rounds': 10, 'step': 50, 'id': 'bob' } }");
assert(!qdict_haskey(rsp, "error"));
qobject_unref(rsp);
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v3 4/4] net/announce: Add HMP optional ID
2019-06-10 18:43 [Qemu-devel] [PATCH v3 0/4] network announce; interface selection & IDs Dr. David Alan Gilbert (git)
` (2 preceding siblings ...)
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID Dr. David Alan Gilbert (git)
@ 2019-06-10 18:44 ` Dr. David Alan Gilbert (git)
3 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2019-06-10 18:44 UTC (permalink / raw)
To: qemu-devel, jasowang, eblake, armbru, laine
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Add the optional ID to the HMP command.
e.g.
# start an announce for a long time on eth1
migrate_set_parameter announce-rounds 1000
announce_self "eth1" e1
# start an announce on eth2
announce_self "eth2" e2
# Change e1 to be announcing on eth1 and eth3
announce_self "eth1,eth3" e1
# Cancel e1
migrate_set_parameter announce-rounds 0
announce_self "" e1
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
hmp-commands.hx | 7 ++++---
hmp.c | 3 +++
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1b63372713..7ba8543cc3 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -955,8 +955,8 @@ ETEXI
{
.name = "announce_self",
- .args_type = "interfaces:s?",
- .params = "[interfaces]",
+ .args_type = "interfaces:s?,id:s?",
+ .params = "[interfaces] [id]",
.help = "Trigger GARP/RARP announcements",
.cmd = hmp_announce_self,
},
@@ -968,7 +968,8 @@ Trigger a round of GARP/RARP broadcasts; this is useful for explicitly updating
network infrastructure after a reconfiguration or some forms of migration.
The timings of the round are set by the migration announce parameters.
An optional comma separated @var{interfaces} list restricts the announce to the
-named set of interfaces.
+named set of interfaces. An optional @var{id} can be used to start a separate announce
+timer and to change the parameters of it later.
ETEXI
{
diff --git a/hmp.c b/hmp.c
index 52efb4a4fa..fd498ca0a8 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1669,12 +1669,15 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
void hmp_announce_self(Monitor *mon, const QDict *qdict)
{
const char *interfaces_str = qdict_get_try_str(qdict, "interfaces");
+ const char *id = qdict_get_try_str(qdict, "id");
AnnounceParameters *params = QAPI_CLONE(AnnounceParameters,
migrate_announce_params());
qapi_free_strList(params->interfaces);
params->interfaces = strList_from_comma_list(interfaces_str);
params->has_interfaces = params->interfaces != NULL;
+ params->id = g_strdup(id);
+ params->has_id = !!params->id;
qmp_announce_self(params, NULL);
qapi_free_AnnounceParameters(params);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces
2019-06-10 18:43 ` [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces Dr. David Alan Gilbert (git)
@ 2019-06-10 19:47 ` Eric Blake
2019-06-11 10:36 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2019-06-10 19:47 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git), qemu-devel, jasowang, armbru, laine
[-- Attachment #1.1: Type: text/plain, Size: 2057 bytes --]
On 6/10/19 1:43 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Allow the caller to restrict the set of interfaces that announces are
> sent on. The default is still to send on all interfaces.
>
> e.g.
>
> { "execute": "announce-self", "arguments": { "initial": 50, "max": 550, "rounds": 5, "step": 50, "interfaces": ["vn2","vn1"] } }
>
> This doesn't affect the behaviour of migraiton announcments.
>
> Note: There's still only one timer for the qmp command, so that
> performing an 'announce-self' on one list of interfaces followed
> by another 'announce-self' on another list will stop the announces
> on the existing set.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> +++ b/qapi/net.json
> @@ -699,6 +699,9 @@
> #
> # @step: Delay increase (in ms) after each self-announcement attempt
> #
> +# @interfaces: An optional list of interface names, which restrict the
restricts
> +# announcment to the listed interfaces. (Since 4.1)
announcement
> +#
> # Since: 4.0
> ##
>
> @@ -706,7 +709,8 @@
> 'data': { 'initial': 'int',
> 'max': 'int',
> 'rounds': 'int',
> - 'step': 'int' } }
> + 'step': 'int',
> + '*interfaces': ['str'] } }
>
> ##
> # @announce-self:
> @@ -718,9 +722,10 @@
> #
> # Example:
> #
> -# -> { "execute": "announce-self"
> +# -> { "execute": "announce-self",
Embarrassing that we didn't notice that one earlier.
> # "arguments": {
> -# "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
> +# "initial": 50, "max": 550, "rounds": 10, "step": 50,
> +# "interfaces": ["vn2","vn3"] } }
Worth a space after the comma? Not required, but I think it looks nicer.
As I only focused on doc issues, I'll leave the full review to others.
--
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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID Dr. David Alan Gilbert (git)
@ 2019-06-10 19:53 ` Eric Blake
2019-06-12 15:32 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2019-06-10 19:53 UTC (permalink / raw)
To: Dr. David Alan Gilbert (git), qemu-devel, jasowang, armbru, laine
[-- Attachment #1.1: Type: text/plain, Size: 3403 bytes --]
On 6/10/19 1:44 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Previously there was a single instance of the timer used by
> monitor triggered announces, that's OK, but when combined with the
> previous change that lets you have announces for subsets of interfaces
> it's a bit restrictive if you want to do different things to different
> interfaces.
>
> Add an 'id' field to the announce, and maintain a list of the
> timers based on id.
>
> This allows you to for example:
> a) Start an announce going on interface eth0 for a long time
> b) Start an announce going on interface eth1 for a long time
> c) Kill the announce on eth0 while leaving eth1 going.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> +++ b/include/net/announce.h
> @@ -23,8 +23,12 @@ struct AnnounceTimer {
> /* Returns: update the timer to the next time point */
> int64_t qemu_announce_timer_step(AnnounceTimer *timer);
>
> -/* Delete the underlying timer and other data */
> -void qemu_announce_timer_del(AnnounceTimer *timer);
> +/*
> + * Delete the underlying timer and other datas
'data' is already plural, 'datas' is not a word.
> + * If 'free_named' true and the timer is a named timer, then remove
> + * it from the list of named timers and free the AnnounceTimer itself.
> + */
> +void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named);
>
> +++ b/qapi/net.json
> @@ -702,6 +702,10 @@
> # @interfaces: An optional list of interface names, which restrict the
> # announcment to the listed interfaces. (Since 4.1)
> #
> +# @id: A name to be used to identify an instance of announce-timers
> +# and to allow it to modified later. Not for use as
> +# part of the migration paramters. (Since 4.1)
parameters
> +#
> # Since: 4.0
> ##
>
> @@ -710,7 +714,8 @@
> 'max': 'int',
> 'rounds': 'int',
> 'step': 'int',
> - '*interfaces': ['str'] } }
> + '*interfaces': ['str'],
> + '*id' : 'str' } }
>
> ##
> # @announce-self:
> @@ -725,7 +730,7 @@
> # -> { "execute": "announce-self",
> # "arguments": {
> # "initial": 50, "max": 550, "rounds": 10, "step": 50,
> -# "interfaces": ["vn2","vn3"] } }
> +# "interfaces": ["vn2","vn3"], "id": "bob" } }
> # <- { "return": {} }
> #
Worth an example of deleting a timer by id?
> # Since: 4.0
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index 163126cf07..7184e2bff4 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -186,7 +186,7 @@ static void announce_self(void *obj, void *data, QGuestAllocator *t_alloc)
> rsp = qmp("{ 'execute' : 'announce-self', "
> " 'arguments': {"
> " 'initial': 50, 'max': 550,"
> - " 'rounds': 10, 'step': 50 } }");
> + " 'rounds': 10, 'step': 50, 'id': 'bob' } }");
And here, is it worth testing that you can delete by id, rather than
just create with an id?
> assert(!qdict_haskey(rsp, "error"));
> qobject_unref(rsp);
>
>
--
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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces
2019-06-10 19:47 ` Eric Blake
@ 2019-06-11 10:36 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-11 10:36 UTC (permalink / raw)
To: Eric Blake; +Cc: jasowang, qemu-devel, laine, armbru
* Eric Blake (eblake@redhat.com) wrote:
> On 6/10/19 1:43 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Allow the caller to restrict the set of interfaces that announces are
> > sent on. The default is still to send on all interfaces.
> >
> > e.g.
> >
> > { "execute": "announce-self", "arguments": { "initial": 50, "max": 550, "rounds": 5, "step": 50, "interfaces": ["vn2","vn1"] } }
> >
> > This doesn't affect the behaviour of migraiton announcments.
> >
> > Note: There's still only one timer for the qmp command, so that
> > performing an 'announce-self' on one list of interfaces followed
> > by another 'announce-self' on another list will stop the announces
> > on the existing set.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
>
> > +++ b/qapi/net.json
> > @@ -699,6 +699,9 @@
> > #
> > # @step: Delay increase (in ms) after each self-announcement attempt
> > #
> > +# @interfaces: An optional list of interface names, which restrict the
>
> restricts
Done
> > +# announcment to the listed interfaces. (Since 4.1)
>
> announcement
Done
> > +#
> > # Since: 4.0
> > ##
> >
> > @@ -706,7 +709,8 @@
> > 'data': { 'initial': 'int',
> > 'max': 'int',
> > 'rounds': 'int',
> > - 'step': 'int' } }
> > + 'step': 'int',
> > + '*interfaces': ['str'] } }
> >
> > ##
> > # @announce-self:
> > @@ -718,9 +722,10 @@
> > #
> > # Example:
> > #
> > -# -> { "execute": "announce-self"
> > +# -> { "execute": "announce-self",
>
> Embarrassing that we didn't notice that one earlier.
The way to avoid it I guess would be to parse the example code.
> > # "arguments": {
> > -# "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
> > +# "initial": 50, "max": 550, "rounds": 10, "step": 50,
> > +# "interfaces": ["vn2","vn3"] } }
>
> Worth a space after the comma? Not required, but I think it looks nicer.
Added
> As I only focused on doc issues, I'll leave the full review to others.
Thanks,
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID
2019-06-10 19:53 ` Eric Blake
@ 2019-06-12 15:32 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 9+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-12 15:32 UTC (permalink / raw)
To: Eric Blake; +Cc: jasowang, qemu-devel, laine, armbru
* Eric Blake (eblake@redhat.com) wrote:
> On 6/10/19 1:44 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Previously there was a single instance of the timer used by
> > monitor triggered announces, that's OK, but when combined with the
> > previous change that lets you have announces for subsets of interfaces
> > it's a bit restrictive if you want to do different things to different
> > interfaces.
> >
> > Add an 'id' field to the announce, and maintain a list of the
> > timers based on id.
> >
> > This allows you to for example:
> > a) Start an announce going on interface eth0 for a long time
> > b) Start an announce going on interface eth1 for a long time
> > c) Kill the announce on eth0 while leaving eth1 going.
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
>
> > +++ b/include/net/announce.h
> > @@ -23,8 +23,12 @@ struct AnnounceTimer {
> > /* Returns: update the timer to the next time point */
> > int64_t qemu_announce_timer_step(AnnounceTimer *timer);
> >
> > -/* Delete the underlying timer and other data */
> > -void qemu_announce_timer_del(AnnounceTimer *timer);
> > +/*
> > + * Delete the underlying timer and other datas
>
> 'data' is already plural, 'datas' is not a word.
oops yes, fixed.
> > + * If 'free_named' true and the timer is a named timer, then remove
> > + * it from the list of named timers and free the AnnounceTimer itself.
> > + */
> > +void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named);
> >
>
> > +++ b/qapi/net.json
> > @@ -702,6 +702,10 @@
> > # @interfaces: An optional list of interface names, which restrict the
> > # announcment to the listed interfaces. (Since 4.1)
> > #
> > +# @id: A name to be used to identify an instance of announce-timers
> > +# and to allow it to modified later. Not for use as
> > +# part of the migration paramters. (Since 4.1)
>
> parameters
Fixed.
> > +#
> > # Since: 4.0
> > ##
> >
> > @@ -710,7 +714,8 @@
> > 'max': 'int',
> > 'rounds': 'int',
> > 'step': 'int',
> > - '*interfaces': ['str'] } }
> > + '*interfaces': ['str'],
> > + '*id' : 'str' } }
> >
> > ##
> > # @announce-self:
> > @@ -725,7 +730,7 @@
> > # -> { "execute": "announce-self",
> > # "arguments": {
> > # "initial": 50, "max": 550, "rounds": 10, "step": 50,
> > -# "interfaces": ["vn2","vn3"] } }
> > +# "interfaces": ["vn2","vn3"], "id": "bob" } }
> > # <- { "return": {} }
> > #
>
> Worth an example of deleting a timer by id?
>
The syntax is actually the same - the only thing you need to do
to cancel is set 'rounds' to 0 for the named id.
> > # Since: 4.0
> > diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> > index 163126cf07..7184e2bff4 100644
> > --- a/tests/virtio-net-test.c
> > +++ b/tests/virtio-net-test.c
> > @@ -186,7 +186,7 @@ static void announce_self(void *obj, void *data, QGuestAllocator *t_alloc)
> > rsp = qmp("{ 'execute' : 'announce-self', "
> > " 'arguments': {"
> > " 'initial': 50, 'max': 550,"
> > - " 'rounds': 10, 'step': 50 } }");
> > + " 'rounds': 10, 'step': 50, 'id': 'bob' } }");
>
> And here, is it worth testing that you can delete by id, rather than
> just create with an id?
OK, I'll have a look at how painful that is.
Dave
> > assert(!qdict_haskey(rsp, "error"));
> > qobject_unref(rsp);
> >
> >
>
> --
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-06-12 15:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-10 18:43 [Qemu-devel] [PATCH v3 0/4] network announce; interface selection & IDs Dr. David Alan Gilbert (git)
2019-06-10 18:43 ` [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces Dr. David Alan Gilbert (git)
2019-06-10 19:47 ` Eric Blake
2019-06-11 10:36 ` Dr. David Alan Gilbert
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 2/4] net/announce: Add HMP optional interface list Dr. David Alan Gilbert (git)
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID Dr. David Alan Gilbert (git)
2019-06-10 19:53 ` Eric Blake
2019-06-12 15:32 ` Dr. David Alan Gilbert
2019-06-10 18:44 ` [Qemu-devel] [PATCH v3 4/4] net/announce: Add HMP " Dr. David Alan Gilbert (git)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).