Re: [PATCH V1 4/4] cpr: reboot mode

[Juan Quintela <quintela@redhat.com>]

Steven Sistare <steven.sistare@oracle.com> wrote:
> On 10/20/2023 5:45 AM, Juan Quintela wrote:
>> Steve Sistare <steven.sistare@oracle.com> wrote:
>>> Add the cpr-reboot migration mode.  Usage:
>>>
>>> $ qemu-system-$arch -monitor stdio ...
>>> QEMU 8.1.50 monitor - type 'help' for more information
>>> (qemu) migrate_set_capability x-ignore-shared on
>>> (qemu) migrate_set_parameter mode cpr-reboot
>>> (qemu) migrate -d file:vm.state
>>> (qemu) info status
>>> VM status: paused (postmigrate)
>>> (qemu) quit
>>>
>>> $ qemu-system-$arch -monitor stdio -incoming defer ...
>>> QEMU 8.1.50 monitor - type 'help' for more information
>>> (qemu) migrate_set_capability x-ignore-shared on
>>> (qemu) migrate_set_parameter mode cpr-reboot
>>> (qemu) migrate_incoming file:vm.state
>>> (qemu) info status
>>> VM status: running
>>>
>>> In this mode, the migrate command saves state to a file, allowing one
>>> to quit qemu, reboot to an updated kernel, and restart an updated version
>>> of qemu.  The caller must specify a migration URI that writes to and reads
>>> from a file.  Unlike normal mode, the use of certain local storage options
>>> does not block the migration, but the caller must not modify guest block
>>> devices between the quit and restart.  The guest RAM memory-backend must
>>> be shared, and the @x-ignore-shared migration capability must be set,
>>> to avoid saving RAM to the file.  Guest RAM must be non-volatile across
>>> reboot, such as by backing it with a dax device, but this is not enforced.
>>> The restarted qemu arguments must match those used to initially start qemu,
>>> plus the -incoming option.
>> 
>> Please, add this message to doc/<somewhere> instead (or additionally) to
>> the commit log.
>> 
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>  qapi/migration.json | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi/migration.json b/qapi/migration.json
>>> index 184fb78..2d862fa 100644
>>> --- a/qapi/migration.json
>>> +++ b/qapi/migration.json
>>> @@ -620,9 +620,23 @@
>>>  #
>>>  # @normal: the original form of migration. (since 8.2)
>>>  #
>>> +# @cpr-reboot: The migrate command saves state to a file, allowing one to
>>> +#              quit qemu, reboot to an updated kernel, and restart an updated
>>> +#              version of qemu.  The caller must specify a migration URI
>>> +#              that writes to and reads from a file.  Unlike normal mode,
>>> +#              the use of certain local storage options does not block the
>>> +#              migration, but the caller must not modify guest block devices
>>> +#              between the quit and restart.  The guest RAM memory-backend
>>> +#              must be shared, and the @x-ignore-shared migration capability
>>> +#              must be set, to avoid saving it to the file.  Guest RAM must
>>> +#              be non-volatile across reboot, such as by backing it with
>>> +#              a dax device, but this is not enforced.  The restarted qemu
>>> +#              arguments must match those used to initially start qemu, plus
>>> +#              the -incoming option. (since 8.2)
>>> +#
>>>  ##
>>>  { 'enum': 'MigMode',
>>> -  'data': [ 'normal' ] }
>>> +  'data': [ 'normal', 'cpr-reboot' ] }
>>>  
>>>  ##
>>>  # @BitmapMigrationBitmapAliasTransform:
>> 
>> It only works with file backend, and we don't have any check for that.
>> Wondering how to add that check.
>
> Actually, it works for other backends, but the ram contents are saved in the
> state file, which is slower. I should spell that out in the json comment and
> in the commit message.

Thanks.
>
>> Additionally, you are not adding a migration test that does exactly what
>> you put there in the comment.
>
> I provide tests/avocado/cpr.py in the original long series.  Would you
> like me to add it to this series, or post it later?  Would you prefer I
> add a test to tests/qtest/migration-test.c?

test/qtest/migration-test.c

please.

Something simple like what you say in the commit should be a good start.

Thanks, Juan.