"Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
> * Germano Veit Michel (germ...@redhat.com) 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.
>
> Markus: Since you're asking for tests for qmp commands; how would you
> test this?
Good question, as tests/ isn't exactly full of examples you could crib.
Let me look at the patch...
> Jason: Does this look OK from the networking side of things?
>
>> Signed-off-by: Germano Veit Michel <germ...@redhat.com>
>> ---
>> 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));
>
> I prefer g_new0 - i.e.
> struct AnnounceRound *round = g_new0(struct AnnounceRound, 1)
>
>> + if (!round)
>> + return;
>> + round->count = SELF_ANNOUNCE_ROUNDS;
>> + round->timer = timer_new_ms(QEMU_CLOCK_REALTIME,
>> qemu_announce_self_once, round);
>
> An odd line break?
>
>> + qemu_announce_self_once(round);
>> +}
>> +
>> +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.
Please drop this line.
>> +#
>> +# Example:
>> +#
>> +# -> { "execute": "announce-self" }
>> +# <- { "return": {} }
>> +#
>> +# Since: 2.9
>> +##
>> +{ 'command': 'announce-self' }
>> +
>From QMP's point of view, this command is as simple as they get: no
arguments, no return values, no errors.
I think a basic smoke test would do: try the command, check no magic
smoke comes out. Untested sketch adapted from qmp-test.c:
/*
* Test cases for network-related QMP commands
*
* Copyright (c) 2017 Red Hat Inc.
*
* Authors:
* Markus Armbruster <arm...@redhat.com>,
*
* 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/error.h"
const char common_args[] = "-nodefaults -machine none";
static void test_qmp_announce_self(void)
{
QDict *resp, *ret;
qtest_start(common_args);
resp = qmp("{ 'execute': 'qmp_capabilities' }");
ret = qdict_get_qdict(resp, "return");
g_assert(ret && !qdict_size(ret));
QDECREF(resp);
qtest_end();
}
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
qtest_add_func("qmp/net/announce_self", test_qmp_announce_self);
return g_test_run();
}
If you want to go the extra mile: is there a way to set up networking so
you can observe the RARPs this should trigger?
I'd call this qmp-net-test. Add to Makefile.include exactly like
qmp-test.
Test cases for existing networking-related QMP commands should be added,
but not in this patch, and not necessarily by you.
Alternatively, have a more general test program for networking stuff,
and make this one of its test cases. Your choice.
Hope this helps!
>
> Please wire hmp up as well (see hmp-commands.hx).
>
> Dave
>
>> --
>> 2.9.3
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
