Re: [Qemu-devel] [RFC PATCH v8 19/21] replay: initialization and deinitialization

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:address@hidden
> On 22/01/2015 09:53, Pavel Dovgalyuk wrote:
> > This patch introduces the functions for enabling the record/replay and for
> > freeing the resources when simulator closes.
> >
> > Signed-off-by: Pavel Dovgalyuk <address@hidden>
> 
> >  };
> >
> >  /* Asynchronous events IDs */
> > diff --git a/replay/replay.c b/replay/replay.c
> > index 7c4a801..fa738c2 100755
> > --- a/replay/replay.c
> > +++ b/replay/replay.c
> > @@ -15,9 +15,18 @@
> >  #include "qemu/timer.h"
> >  #include "sysemu/sysemu.h"
> >
> > +/* Current version of the replay mechanism.
> > +   Increase it when file format changes. */
> > +#define REPLAY_VERSION              0xe02002
> 
> Any meaning?  Perhaps make it 'QRR\0' or something identifiable?

Version id has no special meaning, but has to be distinct from other RR 
versions.
When format of the log file changes we have to update version id to prevent
reading incompatible files.

> > +    replay_file = fopen(fname, fmode);
> > +    if (replay_file == NULL) {
> > +        fprintf(stderr, "Replay: open %s: %s\n", fname, strerror(errno));
> > +        exit(1);
> > +    }
> > +
> > +    replay_filename = g_strdup(fname);
> > +
> > +    replay_mode = mode;
> > +    replay_has_unread_data = 0;
> > +    replay_data_kind = -1;
> > +    replay_state.instructions_count = 0;
> > +    replay_state.current_step = 0;
> > +
> > +    /* skip file header for RECORD and check it for PLAY */
> > +    if (replay_mode == REPLAY_MODE_RECORD) {
> > +        fseek(replay_file, HEADER_SIZE, SEEK_SET);
> 
> Why can't you write the header here?

We don't write header here to detect broken log files at the replay stage.

> > +    replay_save_instructions();
> > +
> > +    /* finalize the file */
> > +    if (replay_file) {
> > +        if (replay_mode == REPLAY_MODE_RECORD) {
> > +            uint64_t offset = 0;
> > +            /* write end event */
> > +            replay_put_event(EVENT_END);
> > +
> > +            /* write header */
> > +            fseek(replay_file, 0, SEEK_SET);
> > +            replay_put_dword(REPLAY_VERSION);
> > +            replay_put_qword(offset);
> 
> Why is this done here?  You're writing a constant zero.

Right, this offset is the field reserved for snapshots table.
It will be introduced in other patches.

Pavel Dovgalyuk