Re: [Qemu-devel] [PATCH v9 06/10] replay: fix save/load vm for non-empty queue

[Paolo Bonzini <pbonzini@redhat.com>]

On 04/05/2017 10:42, Pavel Dovgalyuk wrote:
> From: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
> 
> This patch does not allows saving/loading vmstate when
> replay events queue is not empty. There is no reliable
> way to save events queue, because it describes internal
> coroutine state. Therefore saving and loading operations
> should be deferred to another record/replay step.

Can it actually be non-empty after bdrv_drain_all?

Paolo

> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  include/sysemu/replay.h  |    3 +++
>  migration/savevm.c       |   13 +++++++++++++
>  replay/replay-snapshot.c |    6 ++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index f1c0712795..5c9db2a2ef 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -164,5 +164,8 @@ void replay_audio_in(int *recorded, void *samples, int *wpos, int size);
>  /*! Called at the start of execution.
>      Loads or saves initial vmstate depending on execution mode. */
>  void replay_vmstate_init(void);
> +/*! Called to ensure that replay state is consistent and VM snapshot
> +    can be created */
> +bool replay_can_snapshot(void);
>  
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 03ae1bdeb4..358d22170d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -54,6 +54,7 @@
>  #include "qemu/cutils.h"
>  #include "io/channel-buffer.h"
>  #include "io/channel-file.h"
> +#include "sysemu/replay.h"
>  
>  #ifndef ETH_P_RARP
>  #define ETH_P_RARP 0x8035
> @@ -2083,6 +2084,12 @@ int save_vmstate(Monitor *mon, const char *name)
>      Error *local_err = NULL;
>      AioContext *aio_context;
>  
> +    if (!replay_can_snapshot()) {
> +        monitor_printf(mon, "Record/replay does not allow making snapshot right now. "
> +                        "Try stopping at another step.\n");
> +        return ret;
> +    }
> +
>      if (!bdrv_all_can_snapshot(&bs)) {
>          monitor_printf(mon, "Device '%s' is writable but does not "
>                         "support snapshots.\n", bdrv_get_device_name(bs));
> @@ -2244,6 +2251,12 @@ int load_vmstate(const char *name)
>      AioContext *aio_context;
>      MigrationIncomingState *mis = migration_incoming_get_current();
>  
> +    if (!replay_can_snapshot()) {
> +        error_report("Record/replay does not allow loading snapshot right now. "
> +                     "Try stopping at another step.\n");
> +        return -EINVAL;
> +    }
> +
>      if (!bdrv_all_can_snapshot(&bs)) {
>          error_report("Device '%s' is writable but does not support snapshots.",
>                       bdrv_get_device_name(bs));
> diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
> index 65e2d375c2..438d82ec33 100644
> --- a/replay/replay-snapshot.c
> +++ b/replay/replay-snapshot.c
> @@ -76,3 +76,9 @@ void replay_vmstate_init(void)
>          }
>      }
>  }
> +
> +bool replay_can_snapshot(void)
> +{
> +    return replay_mode == REPLAY_MODE_NONE
> +        || !replay_has_events();
> +}
>