[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2] migration: expose qemu_announce_self() via q
From: |
Vlad Yasevich |
Subject: |
Re: [Qemu-devel] [PATCH V2] migration: expose qemu_announce_self() via qmp |
Date: |
Thu, 11 May 2017 17:37:47 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/20/2017 07:16 PM, Germano Veit Michel wrote:
> qemu_announce_self() is triggered by qemu at the end of migrations
> to update the network regarding the path to the guest l2addr.
>
> however it is also useful when there is a network change such as
> an active bond slave swap. Essentially, it's the same as a migration
> from a network perspective - the guest moves to a different point
> in the network topology.
>
> this exposes the function via qmp.
>
> Signed-off-by: Germano Veit Michel <address@hidden>
> ---
> include/migration/vmstate.h | 5 +++++
> migration/savevm.c | 30 +++++++++++++++++++-----------
> qapi-schema.json | 18 ++++++++++++++++++
> 3 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 63e7b02..a08715c 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -1042,6 +1042,11 @@ int64_t self_announce_delay(int round)
> return 50 + (SELF_ANNOUNCE_ROUNDS - round - 1) * 100;
> }
>
> +struct AnnounceRound {
> + QEMUTimer *timer;
> + int count;
> +};
> +
> void dump_vmstate_json_to_file(FILE *out_fp);
>
> #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 5ecd264..44e196b 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -118,29 +118,37 @@ static void qemu_announce_self_iter(NICState
> *nic, void *opaque)
> 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;
> + struct AnnounceRound *round = opaque;
>
> qemu_foreach_nic(qemu_announce_self_iter, NULL);
>
> - if (--count) {
> + round->count--;
> + if (round->count) {
> /* delay 50ms, 150ms, 250ms, ... */
> - timer_mod(timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> - self_announce_delay(count));
> + timer_mod(round->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) +
> + self_announce_delay(round->count));
> } else {
> - timer_del(timer);
> - timer_free(timer);
> + timer_del(round->timer);
> + timer_free(round->timer);
> + g_free(round);
> }
> }
>
> 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);
> + struct AnnounceRound *round = g_malloc(sizeof(struct AnnounceRound));
> + if (!round)
> + return;
> + round->count = SELF_ANNOUNCE_ROUNDS;
> + round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
> qemu_announce_self_once, round);
> + qemu_announce_self_once(round);
> +}
So, I've been looking and this code and have been playing with it and with
David's
patches and my patches to include virtio self announcements as well. What I've
discovered
is what I think is a possible packet amplification issue here.
This creates a new timer every time we do do a announce_self. With just
migration,
this is not an issue since you only migrate once at a time, so there is only 1
timer.
With exposing this as an API, a user can potentially call it in a tight loop
and now you have a ton of timers being created. Add in David's patches
allowing timeouts
and retries to be configurable, and you may now have a ton of long lived timers.
Add in the patches I am working on to let virtio do self announcements too (to
really fix
bonding issues), and now you add in a possibility of a lot of packets being
sent for
each timeout (RARP, GARP, NA, IGMPv4 Reports, IGMPv6 Reports [even worse if
MLD1 is used]).
As you can see, this can get rather ugly...
I think we need timer user here. Migration and QMP being two to begin with.
Each
one would get a single timer to play with. If a given user already has a timer
running,
we could return an error or just not do anything.
-vlad
> +
> +void qmp_announce_self(Error **errp)
> +{
> + qemu_announce_self();
> }
>
> /***********************************************************/
> diff --git a/qapi-schema.json b/qapi-schema.json
> index baa0d26..0d9bffd 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -6080,3 +6080,21 @@
> #
> ##
> { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] }
> +
> +##
> +# @announce-self:
> +#
> +# Trigger generation of broadcast RARP frames to update network switches.
> +# This can be useful when network bonds fail-over the active slave.
> +#
> +# Arguments: None.
> +#
> +# Example:
> +#
> +# -> { "execute": "announce-self" }
> +# <- { "return": {} }
> +#
> +# Since: 2.9
> +##
> +{ 'command': 'announce-self' }
> +
>