diff mbox

[v2,07/10] replay: allow replay stopping and restarting

Message ID 20160915090122.6440.89499.stgit@PASHA-ISP
State New
Headers show

Commit Message

Pavel Dovgalyuk Sept. 15, 2016, 9:01 a.m. UTC
This patch fixes bug with stopping and restarting replay
through monitor.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 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(-)

Comments

Paolo Bonzini Sept. 15, 2016, 9:38 a.m. UTC | #1
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 <pavel.dovgaluk@ispras.ru>
> ---
>  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);
> 
> 
>
Pavel Dovgalyuk Sept. 16, 2016, 6:35 a.m. UTC | #2
> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of 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 <pavel.dovgaluk@ispras.ru>
> > ---
> > 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?

We have to be sure that events are enabled before any of the callbacks will try to
use events subsystem. Therefore enabling events must be the first thing to do.

Pavel Dovgalyuk
Paolo Bonzini Sept. 16, 2016, 8:55 a.m. UTC | #3
On 16/09/2016 08:35, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of 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 <pavel.dovgaluk@ispras.ru>
>>> ---
>>> 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?
> 
> We have to be sure that events are enabled before any of the callbacks will try to
> use events subsystem. Therefore enabling events must be the first thing to do.

Ok.

Paolo
diff mbox

Patch

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();
         cpu_enable_ticks();
         runstate_set(RUN_STATE_RUNNING);
         vm_state_notify(1, RUN_STATE_RUNNING);