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

[Steven Sistare <steven.sistare@oracle.com>]

On 10/23/2023 2:29 PM, Steven Sistare wrote:
> 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:
    * restart an updated version of qemu     (I buried the lead - steve)
>   * 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