From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50716) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKnyU-00064l-5c for qemu-devel@nongnu.org; Mon, 09 Feb 2015 08:02:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YKnyM-0004x2-QJ for qemu-devel@nongnu.org; Mon, 09 Feb 2015 08:02:02 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKnyM-0004wj-Ic for qemu-devel@nongnu.org; Mon, 09 Feb 2015 08:01:54 -0500 Message-ID: <54D8AFB3.5070600@redhat.com> Date: Mon, 09 Feb 2015 14:01:39 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <20150122085127.5276.53895.stgit@PASHA-ISP.def.inno> <20150122085317.5276.57371.stgit@PASHA-ISP.def.inno> <54CB64C1.6080102@redhat.com> <001d01d04468$40a272e0$c1e758a0$@Dovgaluk@ispras.ru> In-Reply-To: <001d01d04468$40a272e0$c1e758a0$@Dovgaluk@ispras.ru> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v8 19/21] replay: initialization and deinitialization List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgaluk , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, alex.bennee@linaro.org, mark.burton@greensocs.com, real@ispras.ru, batuzovk@ispras.ru, maria.klimushenkova@ispras.ru, afaerber@suse.de, fred.konrad@greensocs.com On 09/02/2015 13:59, Pavel Dovgaluk wrote: >> From: Paolo Bonzini [mailto:pbonzini@redhat.com] >> 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 >> >>> }; >>> >>> /* 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. I guess this is also a provision for something that you'll do in the future. >>> + 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. Okay, then please add comments. Paolo