Am 12.03.2012 17:01, schrieb Paolo Bonzini:
> In most cases, bdrv_co_flush_to_disk just needs to flush the underlying
> file for protocols. Do this implicitly in the block layer.
>
> The backing file is also flushed, because it may still be open read-write
> in the case of live snapshots.
Is this an independent change? I'm also not convinced that it's the
right thing to do because even though it is still opened read-write, we
don't write to it any more. Once bdrv_reopen() is ready, we'll want to
change it to read-only after taking the snapshot.
> @@ -3516,10 +3514,13 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
> {
> int ret;
>
> - if (!bs->drv) {
> + if (!bs || !bdrv_is_inserted(bs) || bdrv_is_read_only(bs)) {
> return 0;
> }
>
> + bdrv_co_flush(bs->file);
> + bdrv_co_flush(bs->backing_hd);
Error handling is missing here.
> +
> /* Write back cached data to the OS even with cache=unsafe */
> if (bs->drv->bdrv_co_flush_to_os) {
> ret = bs->drv->bdrv_co_flush_to_os(bs);
Now you first flush bs->file and then write out the internal caches.
This doesn't look quite right. Probably the recursion should be at the
very end of the function.
Kevin
