diff mbox series

[RFC,08/26] replay: make safe vmstop at record/replay

Message ID 20171031112542.10516.61105.stgit@pasha-VirtualBox
State New
Headers show
Series replay additions | expand

Commit Message

Pavel Dovgalyuk Oct. 31, 2017, 11:25 a.m. UTC 
From: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

This patch disables bdrv flush/drain in record/replay mode.
When block request is in the replay queue it cannot be processed
with drain/flush until it is found in the log.
Therefore vm should just stop leaving unfinished operations
in the queue.

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>

---
 cpus.c             |    7 ++++---
 migration/savevm.c |    4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

Comments

Paolo Bonzini Nov. 2, 2017, 11:28 a.m. UTC | #1 
On 31/10/2017 12:25, Pavel Dovgalyuk wrote:
>  
> -    bdrv_drain_all();
> -    replay_disable_events();
> -    ret = bdrv_flush_all();
> +    if (!replay_events_enabled()) {
> +        bdrv_drain_all();
> +        ret = bdrv_flush_all();
> +    }
>  
>      return ret;
>  }
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 20cebe1..41a13c0 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2143,8 +2143,8 @@ int save_snapshot(const char *name, Error **errp)
>      AioContext *aio_context;
>  
>      if (!replay_can_snapshot()) {
> -        monitor_printf(mon, "Record/replay does not allow making snapshot "
> -                        "right now. Try once more later.\n");
> +        error_report("Record/replay does not allow making snapshot "
> +                     "right now. Try once more later.\n");

Why this change?

Paolo

>          return ret;
Pavel Dovgalyuk Nov. 2, 2017, 11:57 a.m. UTC | #2 
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> On 31/10/2017 12:25, Pavel Dovgalyuk wrote:
> >
> > -    bdrv_drain_all();
> > -    replay_disable_events();
> > -    ret = bdrv_flush_all();
> > +    if (!replay_events_enabled()) {
> > +        bdrv_drain_all();
> > +        ret = bdrv_flush_all();
> > +    }
> >
> >      return ret;
> >  }
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 20cebe1..41a13c0 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2143,8 +2143,8 @@ int save_snapshot(const char *name, Error **errp)
> >      AioContext *aio_context;
> >
> >      if (!replay_can_snapshot()) {
> > -        monitor_printf(mon, "Record/replay does not allow making snapshot "
> > -                        "right now. Try once more later.\n");
> > +        error_report("Record/replay does not allow making snapshot "
> > +                     "right now. Try once more later.\n");
> 
> Why this change?

Kind of a mess in changes.
It should be in patch 06 which introduce these lines.
I'll fix it in the next version.

Pavel Dovgalyuk
Paolo Bonzini Nov. 2, 2017, noon UTC | #3 
On 02/11/2017 12:57, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>> On 31/10/2017 12:25, Pavel Dovgalyuk wrote:
>>>
>>> -    bdrv_drain_all();
>>> -    replay_disable_events();
>>> -    ret = bdrv_flush_all();
>>> +    if (!replay_events_enabled()) {
>>> +        bdrv_drain_all();
>>> +        ret = bdrv_flush_all();
>>> +    }
>>>
>>>      return ret;
>>>  }
>>> diff --git a/migration/savevm.c b/migration/savevm.c
>>> index 20cebe1..41a13c0 100644
>>> --- a/migration/savevm.c
>>> +++ b/migration/savevm.c
>>> @@ -2143,8 +2143,8 @@ int save_snapshot(const char *name, Error **errp)
>>>      AioContext *aio_context;
>>>
>>>      if (!replay_can_snapshot()) {
>>> -        monitor_printf(mon, "Record/replay does not allow making snapshot "
>>> -                        "right now. Try once more later.\n");
>>> +        error_report("Record/replay does not allow making snapshot "
>>> +                     "right now. Try once more later.\n");
>>
>> Why this change?
> 
> Kind of a mess in changes.
> It should be in patch 06 which introduce these lines.
> I'll fix it in the next version.

No problem.  Remember to delete the \n too since otherwise patchew
complains.

Paolo
Pavel Dovgalyuk Nov. 2, 2017, 12:04 p.m. UTC | #4 
> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
> >>> diff --git a/migration/savevm.c b/migration/savevm.c
> >>> index 20cebe1..41a13c0 100644
> >>> --- a/migration/savevm.c
> >>> +++ b/migration/savevm.c
> >>> @@ -2143,8 +2143,8 @@ int save_snapshot(const char *name, Error **errp)
> >>>      AioContext *aio_context;
> >>>
> >>>      if (!replay_can_snapshot()) {
> >>> -        monitor_printf(mon, "Record/replay does not allow making snapshot "
> >>> -                        "right now. Try once more later.\n");
> >>> +        error_report("Record/replay does not allow making snapshot "
> >>> +                     "right now. Try once more later.\n");
> >>
> >> Why this change?
> >
> > Kind of a mess in changes.
> > It should be in patch 06 which introduce these lines.
> > I'll fix it in the next version.
> 
> No problem.  Remember to delete the \n too since otherwise patchew
> complains.

Is this ok for the line over 80 symbols here?

Pavel Dovgalyuk
Paolo Bonzini Nov. 2, 2017, 12:21 p.m. UTC | #5 
On 02/11/2017 13:04, Pavel Dovgalyuk wrote:
>> From: Paolo Bonzini [mailto:pbonzini@redhat.com]
>>>>> diff --git a/migration/savevm.c b/migration/savevm.c
>>>>> index 20cebe1..41a13c0 100644
>>>>> --- a/migration/savevm.c
>>>>> +++ b/migration/savevm.c
>>>>> @@ -2143,8 +2143,8 @@ int save_snapshot(const char *name, Error **errp)
>>>>>      AioContext *aio_context;
>>>>>
>>>>>      if (!replay_can_snapshot()) {
>>>>> -        monitor_printf(mon, "Record/replay does not allow making snapshot "
>>>>> -                        "right now. Try once more later.\n");
>>>>> +        error_report("Record/replay does not allow making snapshot "
>>>>> +                     "right now. Try once more later.\n");
>>>>
>>>> Why this change?
>>>
>>> Kind of a mess in changes.
>>> It should be in patch 06 which introduce these lines.
>>> I'll fix it in the next version.
>>
>> No problem.  Remember to delete the \n too since otherwise patchew
>> complains.
> 
> Is this ok for the line over 80 symbols here?

Patchew (and checkpatch) are complaining about the \n in the string,
i.e. "later.\n" -> "later.".

Thanks,

Paolo
diff mbox series

Patch

diff --git a/cpus.c b/cpus.c
index 114c29b..c728f3a 100644
--- a/cpus.c
+++ b/cpus.c
@@ -942,9 +942,10 @@  static int do_vm_stop(RunState state)
         qapi_event_send_stop(&error_abort);
     }
 
-    bdrv_drain_all();
-    replay_disable_events();
-    ret = bdrv_flush_all();
+    if (!replay_events_enabled()) {
+        bdrv_drain_all();
+        ret = bdrv_flush_all();
+    }
 
     return ret;
 }
diff --git a/migration/savevm.c b/migration/savevm.c
index 20cebe1..41a13c0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2143,8 +2143,8 @@  int save_snapshot(const char *name, Error **errp)
     AioContext *aio_context;
 
     if (!replay_can_snapshot()) {
-        monitor_printf(mon, "Record/replay does not allow making snapshot "
-                        "right now. Try once more later.\n");
+        error_report("Record/replay does not allow making snapshot "
+                     "right now. Try once more later.\n");
         return ret;
     }