[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V1 1/4] migration: mode parameter
From: |
Steven Sistare |
Subject: |
Re: [PATCH V1 1/4] migration: mode parameter |
Date: |
Fri, 20 Oct 2023 10:08:57 -0400 |
User-agent: |
Mozilla Thunderbird |
On 10/20/2023 5:29 AM, Juan Quintela wrote:
> Steve Sistare <steven.sistare@oracle.com> wrote:
>> Create a mode migration parameter that can be used to select alternate
>> migration algorithms. The default mode is normal, representing the
>> current migration algorithm, and does not need to be explicitly set.
>>
>> No functional change until a new mode is added, except that the mode is
>> shown by the 'info migrate' command.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>
> [... qdev definition ...]
>
> Looks legit, but I am not an expert here.
Nor I, but I copied a similar definition line for line, see
qdev_prop_blockdev_on_error
DEFINE_PROP_BLOCKDEV_ON_ERROR
However, I now see I am missing:
QEMU_BUILD_BUG_ON(sizeof(MigMode) != sizeof(int));
I can ask Daniel Berrange to review this part if you prefer.
>> @@ -867,6 +870,13 @@ uint64_t migrate_xbzrle_cache_size(void)
>> return s->parameters.xbzrle_cache_size;
>> }
>>
>> +MigMode migrate_mode(void)
>> +{
>> + MigrationState *s = migrate_get_current();
>> +
>> + return s->parameters.mode;
>> +}
>> +
>
> Inside parameters, I try to get the functions sorted by name. the same
> for options.h
Sure, will do.
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index db3df12..184fb78 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -616,6 +616,15 @@
>> { 'name': 'zstd', 'if': 'CONFIG_ZSTD' } ] }
>>
>> ##
>> +# @MigMode:
>> +#
>> +# @normal: the original form of migration. (since 8.2)
>> +#
>> +##
>> +{ 'enum': 'MigMode',
>> + 'data': [ 'normal' ] }
>> +
>> +##
>
> Here you only have normal, but in qdev you also have exec.
Good eye. I will remove exec from .description in this patch, and add
cpr-reboot to it in patch 4.
>> # @BitmapMigrationBitmapAliasTransform:
>> #
>> # @persistent: If present, the bitmap will be made persistent or
>> @@ -675,6 +684,9 @@
>> #
>> # Migration parameters enumeration
>> #
>> +# @mode: Migration mode. See description in @MigMode. Default is 'normal'.
>> +# (Since 8.2)
>> +#
>
> You normally put comments and values at the end of the comments and
> sections. Your sshould be last.
Do you mean, add the mode parameter at the end of the existing parameters,
after vcpu-dirty-limit?
> Feel free to use a single line in the json. More than one value for
> line make it a bit more compress, but makes changes more complicated.
Like this?
{ 'enum': 'MigrationParameter',
'data': ['announce-initial', 'announce-max',
...
'vcpu-dirty-limit',
'mode'] }
- Steve
[PATCH V1 4/4] cpr: reboot mode, Steve Sistare, 2023/10/19