Re: [PATCH 2/5] softmmu: add trace point when bdrv_flush_all fails

[Dr. David Alan Gilbert]

* Daniel P. Berrangé (berrange@redhat.com) wrote:
> The VM stop process has to flush outstanding I/O and this is a critical
> failure scenario that is hard to diagnose. Add a probe point that
> records the flush return code.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  softmmu/cpus.c       | 7 ++++++-
>  softmmu/trace-events | 3 +++
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index a7ee431187..c3caaeb26e 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -44,6 +44,7 @@
>  #include "sysemu/whpx.h"
>  #include "hw/boards.h"
>  #include "hw/hw.h"
> +#include "trace.h"
>  
>  #ifdef CONFIG_LINUX
>  
> @@ -266,6 +267,7 @@ static int do_vm_stop(RunState state, bool send_stop)
>  
>      bdrv_drain_all();
>      ret = bdrv_flush_all();
> +    trace_vm_stop_flush_all(ret);
>  
>      return ret;
>  }
> @@ -704,12 +706,15 @@ int vm_stop_force_state(RunState state)
>      if (runstate_is_running()) {
>          return vm_stop(state);
>      } else {
> +        int ret;
>          runstate_set(state);
>  
>          bdrv_drain_all();
>          /* Make sure to return an error if the flush in a previous vm_stop()
>           * failed. */
> -        return bdrv_flush_all();
> +        ret = bdrv_flush_all();
> +        trace_vm_stop_flush_all(ret);
> +        return ret;
>      }
>  }
>  
> diff --git a/softmmu/trace-events b/softmmu/trace-events
> index b80ca042e1..f11b427544 100644
> --- a/softmmu/trace-events
> +++ b/softmmu/trace-events
> @@ -19,6 +19,9 @@ flatview_new(void *view, void *root) "%p (root %p)"
>  flatview_destroy(void *view, void *root) "%p (root %p)"
>  flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
>  
> +# softmmu.c
> +vm_stop_flush_all(int ret) "ret %d"
> +
>  # vl.c
>  vm_state_notify(int running, int reason, const char *reason_str) "running %d 
> reason %d (%s)"
>  load_file(const char *name, const char *path) "name %s location %s"
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK