[Qemu-devel] [PATCH] cpus: Let vm_stop[_force_state]() always flush block devices

[Qemu-devel] [PATCH] cpus: Let vm_stop[_force_state]() always flush block devices

[Kevin Wolf]

Even if the VM is already stopped, we cannot assume that all data has
already been successfully flushed to disk. The flush during the previous
vm_stop() could have failed.

Run bdrv_flush_all() unconditionally so that we get an error each time
if the block device isn't really flushed.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 cpus.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8062cdd..2509eb5 100644
--- a/cpus.c
+++ b/cpus.c
@@ -443,11 +443,12 @@ static int do_vm_stop(RunState state)
         pause_all_vcpus();
         runstate_set(state);
         vm_state_notify(0, state);
-        bdrv_drain_all();
-        ret = bdrv_flush_all();
         monitor_protocol_event(QEVENT_STOP, NULL);
     }
 
+    bdrv_drain_all();
+    ret = bdrv_flush_all();
+
     return ret;
 }
 
@@ -1126,7 +1127,9 @@ int vm_stop_force_state(RunState state)
         return vm_stop(state);
     } else {
         runstate_set(state);
-        return 0;
+        /* Make sure to return an error if the flush in a previous vm_stop()
+         * failed. */
+        return bdrv_flush_all();
     }
 }
 
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] cpus: Let vm_stop[_force_state]() always flush block devices

[Kevin Wolf]

Am 18.07.2013 um 14:52 hat Kevin Wolf geschrieben:
> Even if the VM is already stopped, we cannot assume that all data has
> already been successfully flushed to disk. The flush during the previous
> vm_stop() could have failed.
> 
> Run bdrv_flush_all() unconditionally so that we get an error each time
> if the block device isn't really flushed.
> 

Oops, forgot it again:

Cc: qemu-stable@nongnu.org

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  cpus.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 8062cdd..2509eb5 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -443,11 +443,12 @@ static int do_vm_stop(RunState state)
>          pause_all_vcpus();
>          runstate_set(state);
>          vm_state_notify(0, state);
> -        bdrv_drain_all();
> -        ret = bdrv_flush_all();
>          monitor_protocol_event(QEVENT_STOP, NULL);
>      }
>  
> +    bdrv_drain_all();
> +    ret = bdrv_flush_all();
> +
>      return ret;
>  }
>  
> @@ -1126,7 +1127,9 @@ int vm_stop_force_state(RunState state)
>          return vm_stop(state);
>      } else {
>          runstate_set(state);
> -        return 0;
> +        /* Make sure to return an error if the flush in a previous vm_stop()
> +         * failed. */
> +        return bdrv_flush_all();
>      }
>  }
>  
> -- 
> 1.8.1.4
> 

Re: [Qemu-devel] [PATCH] cpus: Let vm_stop[_force_state]() always flush block devices

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 772 bytes --]

On 07/18/2013 06:55 AM, Kevin Wolf wrote:
> Am 18.07.2013 um 14:52 hat Kevin Wolf geschrieben:
>> Even if the VM is already stopped, we cannot assume that all data has
>> already been successfully flushed to disk. The flush during the previous
>> vm_stop() could have failed.
>>
>> Run bdrv_flush_all() unconditionally so that we get an error each time
>> if the block device isn't really flushed.
>>
> 
> Oops, forgot it again:
> 
> Cc: qemu-stable@nongnu.org
> 
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  cpus.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH] cpus: Let vm_stop[_force_state]() always flush block devices

[Stefan Hajnoczi]

On Thu, Jul 18, 2013 at 02:52:19PM +0200, Kevin Wolf wrote:
> Even if the VM is already stopped, we cannot assume that all data has
> already been successfully flushed to disk. The flush during the previous
> vm_stop() could have failed.
> 
> Run bdrv_flush_all() unconditionally so that we get an error each time
> if the block device isn't really flushed.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  cpus.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan