From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:43723)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dovgaluk@ispras.ru>) id 1bEsfX-0005PQ-PP
	for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:26:48 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <dovgaluk@ispras.ru>) id 1bEsfT-0005Py-HW
	for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:26:46 -0400
Received: from mail.ispras.ru ([83.149.199.45]:43492)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dovgaluk@ispras.ru>) id 1bEsfT-0005Od-9R
	for qemu-devel@nongnu.org; Mon, 20 Jun 2016 02:26:43 -0400
From: "Pavel Dovgalyuk" <dovgaluk@ispras.ru>
References: <20160608051352.1688.7877.stgit@PASHA-ISP>
	<20160608051404.1688.65453.stgit@PASHA-ISP>
	<257976361.20784345.1465381879882.JavaMail.zimbra@redhat.com>
In-Reply-To: <257976361.20784345.1465381879882.JavaMail.zimbra@redhat.com>
Date: Mon, 20 Jun 2016 09:26:33 +0300
Message-ID: <001201d1cabc$b1b003e0$15100ba0$@ru>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: 7bit
Content-Language: ru
Subject: Re: [Qemu-devel] [PATCH 2/3] replay: allow replay stopping and
 restarting
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: 'Paolo Bonzini' <pbonzini@redhat.com>, 'Pavel Dovgalyuk' <pavel.dovgaluk@ispras.ru>
Cc: qemu-devel@nongnu.org, jasowang@redhat.com, agraf@suse.de, david@gibson.dropbear.id.au

> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> > From: "Pavel Dovgalyuk" <pavel.dovgaluk@ispras.ru>
> > This patch fixes bug with stopping and restarting replay
> > through monitor.
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> >  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?

> 
> 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