From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35266) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bID0c-0002NX-6t for qemu-devel@nongnu.org; Wed, 29 Jun 2016 06:46:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bID0W-0004qs-9Z for qemu-devel@nongnu.org; Wed, 29 Jun 2016 06:46:17 -0400 Received: from mail-wm0-x229.google.com ([2a00:1450:400c:c09::229]:38111) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bID0W-0004qo-29 for qemu-devel@nongnu.org; Wed, 29 Jun 2016 06:46:12 -0400 Received: by mail-wm0-x229.google.com with SMTP id r201so67287458wme.1 for ; Wed, 29 Jun 2016 03:46:11 -0700 (PDT) Sender: Paolo Bonzini References: <20160608051352.1688.7877.stgit@PASHA-ISP> <20160608051404.1688.65453.stgit@PASHA-ISP> <257976361.20784345.1465381879882.JavaMail.zimbra@redhat.com> <001201d1cabc$b1b003e0$15100ba0$@ru> From: Paolo Bonzini Message-ID: Date: Wed, 29 Jun 2016 12:45:57 +0200 MIME-Version: 1.0 In-Reply-To: <001201d1cabc$b1b003e0$15100ba0$@ru> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] replay: allow replay stopping and restarting List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Dovgalyuk , 'Pavel Dovgalyuk' Cc: jasowang@redhat.com, david@gibson.dropbear.id.au, qemu-devel@nongnu.org, agraf@suse.de On 20/06/2016 08:26, Pavel Dovgalyuk wrote: >> From: Paolo Bonzini [mailto:pbonzini@redhat.com] >>> From: "Pavel Dovgalyuk" >>> This patch fixes bug with stopping and restarting replay >>> through monitor. >>> >>> Signed-off-by: Pavel Dovgalyuk >>> --- >>> block/blkreplay.c | 18 +++++++++++++----- >>> cpus.c | 1 + >>> include/sysemu/replay.h | 2 ++ >>> replay/replay-internal.h | 2 -- >>> vl.c | 1 + >>> 5 files changed, 17 insertions(+), 7 deletions(-) >>> >>> diff --git a/block/blkreplay.c b/block/blkreplay.c >>> index 42f1813..438170c 100644 >>> --- a/block/blkreplay.c >>> +++ b/block/blkreplay.c >>> @@ -70,6 +70,14 @@ static void blkreplay_bh_cb(void *opaque) >>> g_free(req); >>> } >>> >>> +static uint64_t blkreplay_next_id(void) >>> +{ >>> + if (replay_events_enabled()) { >>> + return request_id++; >>> + } >>> + return 0; >>> +} >> >> What happens if 0 is returned? > > It could be any value. When replay events are disables, > it means that either replay is disabled or execution is stopped. > In first case we won't pass this requests through the replay queue > and therefore id is useless. > In stopped mode we have to keep request_id unchanged to make > record/replay deterministic. > >> I think that you want to call >> replay_disable_events... >> >>> bdrv_drain_all(); >> >> ... after this bdrv_drain_all. > > Why? We disable replay events to avoid adding new block requests > to the queue. How this is related to drain all? drain all completes the guest's pending requests. If you disable events before drain all, doesn't that cause a mismatch between record and replay? >> >> I was going to suggest using qemu_add_vm_change_state_handler >> in replay_start (which could have replaced the existing call >> to replay_enable_events), but that's not possible if you have >> to do your calls after bdrv_drain_all. > > Pavel Dovgalyuk > > >