On Mon, Oct 23, 2023 at 02:51:50PM -0400, Steven Sistare wrote: > 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.sist...@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. ...
Okay. We can also avoid mentioning "private guest RAM is saved to ..." because that's what migration already does. IMO we can simplify all that to: It is suggested to use share memory with x-ignore-shared when using this mode. > > > >> > >> 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. I think this is why I'm confused: above sentence is describing a very generic migration to file scenario to me. IOW, I think I can get the same result described even with normal migration to file, or am I wrong? IMHO the description here needs to explain the difference and when an user should use this mode. I think the real answer resides in your whole set, I'll try to read that. In all cases, can we name it something like "live-upgrade" v.s. "normal"? > > > > 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.sist...@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. No need to resend. A link is exactly what I need; git tree even better. I'll comment when I get something when reading that. Thanks, -- Peter Xu
