Re: [Qemu-devel] [PATCH 2/4] multifd: Drop x-multifd-page-count parameter

[Juan Quintela <quintela@redhat.com>]

Laurent Vivier <lvivier@redhat.com> wrote:
> On 06/02/2019 18:58, Juan Quintela wrote:
>> Laurent Vivier <lvivier@redhat.com> wrote:
>>> On 06/02/2019 14:23, Juan Quintela wrote:
>>>> Libvirt don't want to expose (and explain it).  And testing looks like
>>>> 128 is good for all use cases, so just drop it.
>>>>
>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>> ---
>>>>  hmp.c                 |  7 -------
>>>>  migration/migration.c | 30 ------------------------------
>>>>  migration/migration.h |  1 -
>>>>  migration/ram.c       | 13 ++++++++-----
>>>>  qapi/migration.json   | 13 +------------
>>>>  5 files changed, 9 insertions(+), 55 deletions(-)
>>>>
>>> ...
>>>> diff --git a/migration/migration.c b/migration/migration.c
>>>> index f673486679..65df9b566e 100644
>>>> --- a/migration/migration.c
>>>> +++ b/migration/migration.c
>>>> @@ -81,7 +81,6 @@
>>>>  /* The delay time (in ms) between two COLO checkpoints */
>>>>  #define DEFAULT_MIGRATE_X_CHECKPOINT_DELAY (200 * 100)
>>>>  #define DEFAULT_MIGRATE_MULTIFD_CHANNELS 2
>>>> -#define DEFAULT_MIGRATE_MULTIFD_PAGE_COUNT 128
>>>
>>> Why do you update it in the previous patch to remove it in this one?
>> 
>> To make clear that I change the default.  Otherwise it gets hidden into
>> the whole patch.  if you preffer I could have done the other way around.
>
> OK, I understand. It's not really clear because the new default
> (MULTIFD_PAGE_COUNT) is hidden in the patch.
>
> Moreover, in the first patch you update the value, but you don't update
> the comments in qapi/migration.json (I've seen that because you remove
> them in this patch).

Aha, I knew I was forgetting something.

> Perhaps you can proceed in the reverse order: remove the parameter and
> then set the new default... or merge the two patches and saying in the
> commit message you change the default value.

Ok.

> Thanks,
> Laurent

Later, Juan.