On Tue Mar 12, 2024 at 7:00 PM AEST, Pavel Dovgalyuk wrote: > On 11.03.2024 20:40, Nicholas Piggin wrote: > > record makes an initial snapshot when the machine is created, to enable > > reverse-debugging. Often the issue being debugged appears near the end of > > the trace, so it is important for performance to keep snapshots close to > > the end. > > > > This implements a periodic snapshot mode that keeps a rolling set of > > recent snapshots. This could be done by the debugger or other program > > that talks QMP, but for setting up simple scenarios and tests, this is > > more convenient. > > > > Signed-off-by: Nicholas Piggin <npig...@gmail.com> > > --- > > docs/system/replay.rst | 5 ++++ > > include/sysemu/replay.h | 11 ++++++++ > > replay/replay-snapshot.c | 57 ++++++++++++++++++++++++++++++++++++++++ > > replay/replay.c | 27 +++++++++++++++++-- > > system/vl.c | 9 +++++++ > > qemu-options.hx | 9 +++++-- > > 6 files changed, 114 insertions(+), 4 deletions(-) > > > > diff --git a/docs/system/replay.rst b/docs/system/replay.rst > > index ca7c17c63d..1ae8614475 100644 > > --- a/docs/system/replay.rst > > +++ b/docs/system/replay.rst > > @@ -156,6 +156,11 @@ for storing VM snapshots. Here is the example of the > > command line for this: > > ``empty.qcow2`` drive does not connected to any virtual block device and > > used > > for VM snapshots only. > > > > +``rrsnapmode`` can be used to select just an initial snapshot or periodic > > +snapshots, with ``rrsnapcount`` specifying the number of periodic snapshots > > +to maintain, and ``rrsnaptime`` the amount of run time in seconds between > > +periodic snapshots. > > + > > .. _network-label: > > > > Network devices > > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h > > index 8102fa54f0..92fa82842b 100644 > > --- a/include/sysemu/replay.h > > +++ b/include/sysemu/replay.h > > @@ -48,6 +48,17 @@ typedef enum ReplayCheckpoint ReplayCheckpoint; > > > > typedef struct ReplayNetState ReplayNetState; > > > > +enum ReplaySnapshotMode { > > + REPLAY_SNAPSHOT_MODE_INITIAL, > > + REPLAY_SNAPSHOT_MODE_PERIODIC, > > +}; > > This should be defined in replay-internal.h, because it is internal for > replay. > > > +typedef enum ReplaySnapshotMode ReplaySnapshotMode; > > + > > +extern ReplaySnapshotMode replay_snapshot_mode; > > + > > +extern uint64_t replay_snapshot_periodic_delay; > > +extern int replay_snapshot_periodic_nr_keep; > > These ones are internal too.
Okay for both. > > > + > > /* Name of the initial VM snapshot */ > > extern char *replay_snapshot; > > > > diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c > > index ccb4d89dda..762555feaa 100644 > > --- a/replay/replay-snapshot.c > > +++ b/replay/replay-snapshot.c > > @@ -70,6 +70,53 @@ void replay_vmstate_register(void) > > vmstate_register(NULL, 0, &vmstate_replay, &replay_state); > > } > > > > +static QEMUTimer *replay_snapshot_timer; > > +static int replay_snapshot_count; > > + > > +static void replay_snapshot_timer_cb(void *opaque) > > +{ > > + Error *err = NULL; > > + char *name; > > + > > + if (!replay_can_snapshot()) { > > + /* Try again soon */ > > + timer_mod(replay_snapshot_timer, > > + qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > > + replay_snapshot_periodic_delay / 10); > > + return; > > + } > > + > > + name = g_strdup_printf("%s-%d", replay_snapshot, > > replay_snapshot_count); > > + if (!save_snapshot(name, > > + true, NULL, false, NULL, &err)) { > > + error_report_err(err); > > + error_report("Could not create periodic snapshot " > > + "for icount record, disabling"); > > + g_free(name); > > + return; > > + } > > + g_free(name); > > + replay_snapshot_count++; > > + > > + if (replay_snapshot_periodic_nr_keep >= 1 && > > + replay_snapshot_count > replay_snapshot_periodic_nr_keep) { > > + int del_nr; > > + > > + del_nr = replay_snapshot_count - replay_snapshot_periodic_nr_keep > > - 1; > > + name = g_strdup_printf("%s-%d", replay_snapshot, del_nr); > > Copy-paste of snapshot name format. Yes good catch. > > > + if (!delete_snapshot(name, false, NULL, &err)) { > > + error_report_err(err); > > + error_report("Could not delete periodic snapshot " > > + "for icount record"); > > + } > > + g_free(name); > > + } > > + > > + timer_mod(replay_snapshot_timer, > > + qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > > + replay_snapshot_periodic_delay); > > +} > > + > > void replay_vmstate_init(void) > > { > > Error *err = NULL; > > @@ -82,6 +129,16 @@ void replay_vmstate_init(void) > > error_report("Could not create snapshot for icount > > record"); > > exit(1); > > } > > + > > + if (replay_snapshot_mode == REPLAY_SNAPSHOT_MODE_PERIODIC) { > > + replay_snapshot_timer = timer_new_ms(QEMU_CLOCK_REALTIME, > > + > > replay_snapshot_timer_cb, > > + NULL); > > + timer_mod(replay_snapshot_timer, > > + qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + > > + replay_snapshot_periodic_delay); > > + } > > + > > } else if (replay_mode == REPLAY_MODE_PLAY) { > > if (!load_snapshot(replay_snapshot, NULL, false, NULL, &err)) > > { > > error_report_err(err); > > diff --git a/replay/replay.c b/replay/replay.c > > index 895fa6b67a..c916e71d30 100644 > > --- a/replay/replay.c > > +++ b/replay/replay.c > > @@ -29,6 +29,10 @@ > > ReplayMode replay_mode = REPLAY_MODE_NONE; > > char *replay_snapshot; > > > > +ReplaySnapshotMode replay_snapshot_mode; > > +uint64_t replay_snapshot_periodic_delay; > > +int replay_snapshot_periodic_nr_keep; > > + > > /* Name of replay file */ > > static char *replay_filename; > > ReplayState replay_state; > > @@ -424,6 +428,27 @@ void replay_configure(QemuOpts *opts) > > } > > > > replay_snapshot = g_strdup(qemu_opt_get(opts, "rrsnapshot")); > > + if (replay_snapshot && mode == REPLAY_MODE_RECORD) { > > + const char *snapmode; > > + > > + snapmode = qemu_opt_get(opts, "rrsnapmode"); > > + if (!snapmode || !strcmp(snapmode, "initial")) { > > + replay_snapshot_mode = REPLAY_SNAPSHOT_MODE_INITIAL; > > + } else if (!strcmp(snapmode, "periodic")) { > > + replay_snapshot_mode = REPLAY_SNAPSHOT_MODE_PERIODIC; > > + } else { > > + error_report("Invalid rrsnapmode option: %s", snapmode); > > + exit(1); > > + } > > + > > + /* Default 10 host seconds of machine runtime per snapshot. */ > > + replay_snapshot_periodic_delay = > > + qemu_opt_get_number(opts, "rrsnaptime", 10) * > > 1000; > > Can user set it to zero here? I guess so. It would just continually snapshot, which is probably okay if that's what you want. Thanks, Nick