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

[Steven Sistare]

On 10/23/2023 11:39 AM, Peter Xu wrote:
> On Thu, Oct 19, 2023 at 01:47:46PM -0700, Steve Sistare 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.
>>
>> 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)
> 
> What happens if someone migrates with non-shared memory, or without
> ignore-shared?  Is it only because it'll be slow saving and loading?
> 
> If that's required, we should fail the mode set if (1) non-shared memory is
> used, or (2) x-ignore-shared is not enabled.  But I had a feeling it's the
> other way round.

Juan also asked me to clarify this.  I plan to resubmit this:

#                                        ...  Private guest RAM is saved in
#              the file.  To avoid this cost, the guest RAM memory-backend
#              must be shared, and the @x-ignore-shared migration capability
#              must be set.  ...

> 
> Reading the whole series, if it's so far all about "local storage", why
> "cpr-reboot"?  Why not "local" or "local storage" as the name?

The use case is about rebooting and updating the host, so reboot is in 
the name.  Local storage just happens to be allowed for it.

> I had a feeling that this patchset mixed a lot of higher level use case
> into the mode definition.  IMHO we should provide clear definition of each
> mode on what it does.  It's so far not so clear to me, even if I kind of
> know what you plan to do.

I believe I already have, in the cover letter, commit message, and qapi 
definition, at the start of each:

# @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 cover letter hints at the cpr-exec use case, and the long V9 patch series
describes it, and I will make sure the use case comes first when I submit 
cpr-exec,
which is:
  * much shorter guest downtime than cpr reboot
  * support vfio without requiring guest suspension
  * keep certain character devices alive

> I tried again google what CPR is for and found this:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg08541.html
> 
> I also prefer spell it out, at least make it clear on what that means..  I
> didn't even see "Checkpoint/restart" words mentioned anywhere in this
> patchset.

Will do.

> Besides: do you have a tree somewhere for the whole set of latest CPR work?

I have the V9 patch series:
  
https://lore.kernel.org/qemu-devel/1658851843-236870-1-git-send-email-steven.sistare@oracle.com
and I can re-send my proposal for breaking it down into patch sets that I 
presented in the
qemu community meeting, if you did not save it.

- Steve