Re: [Qemu-devel] [PATCH v2 07/10] replay: allow replay stopping and restarting

[Paolo Bonzini]

On 15/09/2016 11:01, Pavel Dovgalyuk wrote:
> This patch fixes bug with stopping and restarting replay
> through monitor.
> 
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> ---
>  block/blkreplay.c        |   15 +++++----------
>  cpus.c                   |    1 +
>  include/sysemu/replay.h  |    4 ++++
>  replay/replay-events.c   |    8 ++++++++
>  replay/replay-internal.h |    6 ++++--
>  replay/replay-snapshot.c |    1 +
>  stubs/replay.c           |    5 +++++
>  vl.c                     |    1 +
>  8 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/block/blkreplay.c b/block/blkreplay.c
> index 62ae861..c2c3615 100644
> --- a/block/blkreplay.c
> +++ b/block/blkreplay.c
> @@ -21,11 +21,6 @@ typedef struct Request {
>      QEMUBH *bh;
>  } Request;
>  
> -/* Next request id.
> -   This counter is global, because requests from different
> -   block devices should not get overlapping ids. */
> -static uint64_t request_id;
> -
>  static BlockDriverState *blkreplay_append_snapshot(BlockDriverState *bs,
>                                                     int flags,
>                                                     QDict *snapshot_options,
> @@ -194,7 +189,7 @@ static void block_request_create(uint64_t reqid, 
> BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_preadv(BlockDriverState *bs,
>      uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_preadv(bs->file, offset, bytes, qiov, flags);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -205,7 +200,7 @@ static int coroutine_fn 
> blkreplay_co_preadv(BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_pwritev(BlockDriverState *bs,
>      uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_pwritev(bs->file, offset, bytes, qiov, flags);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -216,7 +211,7 @@ static int coroutine_fn 
> blkreplay_co_pwritev(BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
>      int64_t offset, int count, BdrvRequestFlags flags)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_pwrite_zeroes(bs->file, offset, count, flags);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -227,7 +222,7 @@ static int coroutine_fn 
> blkreplay_co_pwrite_zeroes(BlockDriverState *bs,
>  static int coroutine_fn blkreplay_co_pdiscard(BlockDriverState *bs,
>                                                int64_t offset, int count)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_pdiscard(bs->file->bs, offset, count);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> @@ -237,7 +232,7 @@ static int coroutine_fn 
> blkreplay_co_pdiscard(BlockDriverState *bs,
>  
>  static int coroutine_fn blkreplay_co_flush(BlockDriverState *bs)
>  {
> -    uint64_t reqid = request_id++;
> +    uint64_t reqid = blkreplay_next_id();
>      int ret = bdrv_co_flush(bs->file->bs);
>      block_request_create(reqid, bs, qemu_coroutine_self());
>      qemu_coroutine_yield();
> diff --git a/cpus.c b/cpus.c
> index 0ab4ab1..3938213 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -745,6 +745,7 @@ static int do_vm_stop(RunState state)
>      }
>  
>      bdrv_drain_all();
> +    replay_disable_events();
>      ret = blk_flush_all();
>  
>      return ret;
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 1123fc2..d893c63 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -107,6 +107,8 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint);
>  
>  /*! Disables storing events in the queue */
>  void replay_disable_events(void);
> +/*! Enables storing events in the queue */
> +void replay_enable_events(void);
>  /*! Returns true when saving events is enabled */
>  bool replay_events_enabled(void);
>  /*! Adds bottom half event to the queue */
> @@ -117,6 +119,8 @@ void replay_input_event(QemuConsole *src, InputEvent 
> *evt);
>  void replay_input_sync_event(void);
>  /*! Adds block layer event to the queue */
>  void replay_block_event(QEMUBH *bh, uint64_t id);
> +/*! Returns ID for the next block event */
> +uint64_t blkreplay_next_id(void);
>  
>  /* Character device */
>  
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 4ee2f5d..94a6dcc 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -320,3 +320,11 @@ bool replay_events_enabled(void)
>  {
>      return events_enabled;
>  }
> +
> +uint64_t blkreplay_next_id(void)
> +{
> +    if (replay_events_enabled()) {
> +        return replay_state.block_request_id++;
> +    }
> +    return 0;
> +}
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 9bc4a29..c5afc58 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -69,6 +69,10 @@ typedef struct ReplayState {
>      unsigned int has_unread_data;
>      /*! Temporary variable for saving current log offset. */
>      uint64_t file_offset;
> +    /*! Next block operation id.
> +        This counter is global, because requests from different
> +        block devices should not get overlapping ids. */
> +    uint64_t block_request_id;
>  } ReplayState;
>  extern ReplayState replay_state;
>  
> @@ -124,8 +128,6 @@ void replay_read_next_clock(unsigned int kind);
>  void replay_init_events(void);
>  /*! Clears internal data structures for events handling */
>  void replay_finish_events(void);
> -/*! Enables storing events in the queue */
> -void replay_enable_events(void);
>  /*! Flushes events queue */
>  void replay_flush_events(void);
>  /*! Clears events list before loading new VM state */
> diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
> index 4933bdd..b7b75d6 100644
> --- a/replay/replay-snapshot.c
> +++ b/replay/replay-snapshot.c
> @@ -49,6 +49,7 @@ static const VMStateDescription vmstate_replay = {
>          VMSTATE_UINT32(data_kind, ReplayState),
>          VMSTATE_UINT32(has_unread_data, ReplayState),
>          VMSTATE_UINT64(file_offset, ReplayState),
> +        VMSTATE_UINT64(block_request_id, ReplayState),
>          VMSTATE_END_OF_LIST()
>      },
>  };
> diff --git a/stubs/replay.c b/stubs/replay.c
> index de9fa1e..d9a6da9 100644
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -67,3 +67,8 @@ void replay_char_read_all_save_buf(uint8_t *buf, int offset)
>  void replay_block_event(QEMUBH *bh, uint64_t id)
>  {
>  }
> +
> +uint64_t blkreplay_next_id(void)
> +{
> +    return 0;
> +}
> diff --git a/vl.c b/vl.c
> index f2193cb..32155f5 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -746,6 +746,7 @@ void vm_start(void)
>      if (runstate_is_running()) {
>          qapi_event_send_stop(&error_abort);
>      } else {
> +        replay_enable_events();

Can this be done with a change state notifier?

Paolo

>          cpu_enable_ticks();
>          runstate_set(RUN_STATE_RUNNING);
>          vm_state_notify(1, RUN_STATE_RUNNING);
> 
> 
>