From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40017) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YO5OW-0006ad-Qa for qemu-devel@nongnu.org; Wed, 18 Feb 2015 09:14:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YO5OT-0007BN-HW for qemu-devel@nongnu.org; Wed, 18 Feb 2015 09:14:28 -0500 Received: from mail-wi0-x22f.google.com ([2a00:1450:400c:c05::22f]:60373) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YO5OT-0007AF-BV for qemu-devel@nongnu.org; Wed, 18 Feb 2015 09:14:25 -0500 Received: by mail-wi0-f175.google.com with SMTP id r20so41341417wiv.2 for ; Wed, 18 Feb 2015 06:14:24 -0800 (PST) Sender: Paolo Bonzini Message-ID: <54E49E3A.9050804@redhat.com> Date: Wed, 18 Feb 2015 15:14:18 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <20150218115534.4176.12578.stgit@PASHA-ISP> <20150218115703.4176.16661.stgit@PASHA-ISP> In-Reply-To: <20150218115703.4176.16661.stgit@PASHA-ISP> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH v9 14/23] replay: checkpoints List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk , 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 18/02/2015 12:57, Pavel Dovgalyuk wrote: > > + > + if (replay_file) { > + if (replay_mode == REPLAY_MODE_PLAY) { > + replay_mutex_lock(); > + if (!skip_async_events(EVENT_CHECKPOINT + checkpoint)) { > + if (replay_data_kind == EVENT_ASYNC) { > + replay_read_events(checkpoint); > + replay_fetch_data_kind(); > + res = replay_data_kind != EVENT_ASYNC; > + replay_mutex_unlock(); > + return res; > + } > + replay_mutex_unlock(); > + return res; > + } > + replay_has_unread_data = 0; > + replay_read_events(checkpoint); > + replay_fetch_data_kind(); > + res = replay_data_kind != EVENT_ASYNC; > + replay_mutex_unlock(); > + return res; > + } else if (replay_mode == REPLAY_MODE_RECORD) { > + replay_mutex_lock(); > + replay_put_event(EVENT_CHECKPOINT + checkpoint); > + replay_save_events(checkpoint); > + replay_mutex_unlock(); > + } > + } > + > + return true; This is cleaner: if (!replay_file) { return true; } replay_mutex_lock(); if (replay_mode == REPLAY_MODE_PLAY) { if (skip_async_events(EVENT_CHECKPOINT + checkpoint)) { replay_has_unread_data = 0; } else if (replay_data_kind != EVENT_ASYNC) { res = false; goto out; } replay_read_events(checkpoint); replay_fetch_data_kind(); res = replay_data_kind != EVENT_ASYNC; } else if (replay_mode == REPLAY_MODE_RECORD) { replay_put_event(EVENT_CHECKPOINT + checkpoint); replay_save_events(checkpoint); res = true; } out: replay_mutex_unlock(); return res; I'm not sure however of the meaning of "res = replay_data_kind != EVENT_ASYNC;". Why not just return true since skip_async_events has returned true? Can you add a comment? Paolo > + if (!replay_checkpoint(CHECKPOINT_BDRV_DRAIN)) { > + /* Do not wait anymore, we stopped at some place in > + the middle of execution during replay */ > + return; > + } > busy = false; > > QTAILQ_FOREACH(bs, &bdrv_states, device_list) { > @@ -2004,6 +2010,12 @@ void bdrv_drain_all(void) > aio_context_release(aio_context); > } > } > + if (replay_mode == REPLAY_MODE_PLAY) { > + /* Skip checkpoints from the log */ > + while (replay_checkpoint(CHECKPOINT_BDRV_DRAIN)) { > + /* Nothing */ > + } > + } > } Can you explain this? Otherwise the patch looks great.