On 05/30/2017 05:58 AM, Juan Quintela wrote: > Vladislav Yasevich <vyase...@redhat.com> wrote: >> Add parameters that control RARP/GARP announcement timeouts. >> The parameters structure is added to the QAPI and a qmp command >> is added to set/get the parameter data. >> >> Based on work by "Dr. David Alan Gilbert" <dgilb...@redhat.com> >> >> Signed-off-by: Vladislav Yasevich <vyase...@redhat.com> > > Hi > >> diff --git a/migration/savevm.c b/migration/savevm.c >> index f5e8194..cee2837 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -78,6 +78,104 @@ static struct mig_cmd_args { >> [MIG_CMD_MAX] = { .len = -1, .name = "MAX" }, >> }; >> > > Once that you are touching this, shouldn't it be better to be in > net/<somewhere>? > They have nothing to do with migration really. >
Yeah, I considered this, but could really find a good slot for them. I can either put them into net.c directly or pull them out into their own little file. > >> +#define QEMU_ANNOUNCE_INITIAL 50 >> +#define QEMU_ANNOUNCE_MAX 550 >> +#define QEMU_ANNOUNCE_ROUNDS 5 >> +#define QEMU_ANNOUNCE_STEP 100 >> + >> +AnnounceParameters *qemu_get_announce_params(void) >> +{ >> + static AnnounceParameters announce = { >> + .initial = QEMU_ANNOUNCE_INITIAL, >> + .max = QEMU_ANNOUNCE_MAX, >> + .rounds = QEMU_ANNOUNCE_ROUNDS, >> + .step = QEMU_ANNOUNCE_STEP, >> + }; >> + >> + return &announce; >> +} >> + >> +void qemu_fill_announce_parameters(AnnounceParameters **to, >> + AnnounceParameters *from) >> +{ >> + AnnounceParameters *params; >> + >> + params = *to = g_malloc0(sizeof(*params)); >> + params->has_initial = true; >> + params->initial = from->initial; >> + params->has_max = true; >> + params->max = from->max; >> + params->has_rounds = true; >> + params->rounds = from->rounds; >> + params->has_step = true; >> + params->step = from->step; >> +} >> + >> +bool qemu_validate_announce_parameters(AnnounceParameters *params, Error >> **errp) >> +{ >> + if (params->has_initial && >> + (params->initial < 1 || params->initial > 100000)) { > > This is independent of this patch, but we really need a macro: > CHECK(field, mininum, maximum) > > We repeat this left and right. > >> +void qemu_set_announce_parameters(AnnounceParameters *announce_params, >> + AnnounceParameters *params) > >> +void qmp_announce_set_parameters(AnnounceParameters *params, Error **errp) > > Really similar functions name. I assume by know that the 1st function > is used somewhere else in the series. > Yes, the first function is going to be used later. Thanks -vlad