[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/12] migration: Introduce announce parameters
From: |
Vlad Yasevich |
Subject: |
Re: [Qemu-devel] [PATCH 01/12] migration: Introduce announce parameters |
Date: |
Tue, 30 May 2017 09:45:28 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 |
On 05/30/2017 05:58 AM, Juan Quintela wrote:
> Vladislav Yasevich <address@hidden> 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" <address@hidden>
>>
>> Signed-off-by: Vladislav Yasevich <address@hidden>
>
> 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