Re: [Qemu-devel] [PATCH 01/12] migration: Introduce announce parameters

[Juan Quintela <quintela@redhat.com>]

Vladislav Yasevich <vyasevic@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" <dgilbert@redhat.com>
>
> Signed-off-by: Vladislav Yasevich <vyasevic@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.


> +#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.