On 20/06/2016 08:26, Pavel Dovgalyuk wrote: >> From: Paolo Bonzini [mailto:pbonz...@redhat.com] >>> From: "Pavel Dovgalyuk" <pavel.dovga...@ispras.ru> >>> This patch fixes bug with stopping and restarting replay >>> through monitor. >>> >>> Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@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?
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 > > >