[Qemu-devel] [PATCH v2 0/3] block: Write out internal caches even with cache=unsafe

[Qemu-devel] [PATCH v2 0/3] block: Write out internal caches even with cache=unsafe

[Kevin Wolf]

Paolo, does this implement what you were suggesting?

Kevin Wolf (3):
  block: Rename bdrv_co_flush to bdrv_co_flush_to_disk
  block: Introduce bdrv_co_flush_to_os
  block: Make cache=unsafe flush to the OS

 block.c           |   23 +++++++++++++++++++----
 block/cow.c       |   22 ++++++++++++----------
 block/qcow.c      |   18 ++++++++++--------
 block/qcow2.c     |   14 ++++++++++----
 block/raw-win32.c |   15 +++++++++------
 block/raw.c       |    8 ++++----
 block/rbd.c       |   10 +++++-----
 block/vdi.c       |    2 +-
 block/vmdk.c      |    4 ++--
 block/vpc.c       |    8 +++++---
 block_int.h       |   14 +++++++++++++-
 11 files changed, 90 insertions(+), 48 deletions(-)

-- 
1.7.6.4

[Qemu-devel] [PATCH v2 1/3] block: Rename bdrv_co_flush to bdrv_co_flush_to_disk

[Kevin Wolf]

There are two different types of flush that you can do: Flushing one level up
to the OS (i.e. writing data to the host page cache) or flushing it all the way
down to the disk. The existing functions flush to the disk, reflect this in the
function name.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c           |    4 ++--
 block/cow.c       |   22 ++++++++++++----------
 block/qcow.c      |   18 ++++++++++--------
 block/qcow2.c     |    6 +++---
 block/raw-win32.c |   15 +++++++++------
 block/raw.c       |    8 ++++----
 block/rbd.c       |   10 +++++-----
 block/vdi.c       |    2 +-
 block/vmdk.c      |    4 ++--
 block/vpc.c       |    8 +++++---
 block_int.h       |    7 ++++++-
 11 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/block.c b/block.c
index 9bb236c..d3042da 100644
--- a/block.c
+++ b/block.c
@@ -2786,8 +2786,8 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
         return 0;
     } else if (!bs->drv) {
         return 0;
-    } else if (bs->drv->bdrv_co_flush) {
-        return bs->drv->bdrv_co_flush(bs);
+    } else if (bs->drv->bdrv_co_flush_to_disk) {
+        return bs->drv->bdrv_co_flush_to_disk(bs);
     } else if (bs->drv->bdrv_aio_flush) {
         BlockDriverAIOCB *acb;
         CoroutineIOCompletion co = {
diff --git a/block/cow.c b/block/cow.c
index 707c0aa..089d395 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -326,16 +326,18 @@ static QEMUOptionParameter cow_create_options[] = {
 };
 
 static BlockDriver bdrv_cow = {
-    .format_name	= "cow",
-    .instance_size	= sizeof(BDRVCowState),
-    .bdrv_probe		= cow_probe,
-    .bdrv_open		= cow_open,
-    .bdrv_read          = cow_co_read,
-    .bdrv_write         = cow_co_write,
-    .bdrv_close		= cow_close,
-    .bdrv_create	= cow_create,
-    .bdrv_co_flush      = cow_co_flush,
-    .bdrv_is_allocated	= cow_is_allocated,
+    .format_name    = "cow",
+    .instance_size  = sizeof(BDRVCowState),
+
+    .bdrv_probe     = cow_probe,
+    .bdrv_open      = cow_open,
+    .bdrv_close     = cow_close,
+    .bdrv_create    = cow_create,
+
+    .bdrv_read              = cow_co_read,
+    .bdrv_write             = cow_co_write,
+    .bdrv_co_flush_to_disk  = cow_co_flush,
+    .bdrv_is_allocated      = cow_is_allocated,
 
     .create_options = cow_create_options,
 };
diff --git a/block/qcow.c b/block/qcow.c
index 35e21eb..adecee0 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -828,14 +828,16 @@ static BlockDriver bdrv_qcow = {
     .bdrv_open		= qcow_open,
     .bdrv_close		= qcow_close,
     .bdrv_create	= qcow_create,
-    .bdrv_is_allocated	= qcow_is_allocated,
-    .bdrv_set_key	= qcow_set_key,
-    .bdrv_make_empty	= qcow_make_empty,
-    .bdrv_co_readv      = qcow_co_readv,
-    .bdrv_co_writev     = qcow_co_writev,
-    .bdrv_co_flush      = qcow_co_flush,
-    .bdrv_write_compressed = qcow_write_compressed,
-    .bdrv_get_info	= qcow_get_info,
+
+    .bdrv_co_readv          = qcow_co_readv,
+    .bdrv_co_writev         = qcow_co_writev,
+    .bdrv_co_flush_to_disk  = qcow_co_flush,
+    .bdrv_is_allocated      = qcow_is_allocated,
+
+    .bdrv_set_key           = qcow_set_key,
+    .bdrv_make_empty        = qcow_make_empty,
+    .bdrv_write_compressed  = qcow_write_compressed,
+    .bdrv_get_info          = qcow_get_info,
 
     .create_options = qcow_create_options,
 };
diff --git a/block/qcow2.c b/block/qcow2.c
index ef057d3..f7f73fe 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1243,9 +1243,9 @@ static BlockDriver bdrv_qcow2 = {
     .bdrv_set_key       = qcow2_set_key,
     .bdrv_make_empty    = qcow2_make_empty,
 
-    .bdrv_co_readv      = qcow2_co_readv,
-    .bdrv_co_writev     = qcow2_co_writev,
-    .bdrv_co_flush      = qcow2_co_flush,
+    .bdrv_co_readv          = qcow2_co_readv,
+    .bdrv_co_writev         = qcow2_co_writev,
+    .bdrv_co_flush_to_disk  = qcow2_co_flush,
 
     .bdrv_co_discard        = qcow2_co_discard,
     .bdrv_truncate          = qcow2_truncate,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index f5f73bc..e4b0b75 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -281,9 +281,11 @@ static BlockDriver bdrv_file = {
     .bdrv_file_open	= raw_open,
     .bdrv_close		= raw_close,
     .bdrv_create	= raw_create,
-    .bdrv_co_flush      = raw_flush,
-    .bdrv_read		= raw_read,
-    .bdrv_write		= raw_write,
+
+    .bdrv_read              = raw_read,
+    .bdrv_write             = raw_write,
+    .bdrv_co_flush_to_disk  = raw_flush,
+
     .bdrv_truncate	= raw_truncate,
     .bdrv_getlength	= raw_getlength,
     .bdrv_get_allocated_file_size
@@ -409,11 +411,12 @@ static BlockDriver bdrv_host_device = {
     .bdrv_probe_device	= hdev_probe_device,
     .bdrv_file_open	= hdev_open,
     .bdrv_close		= raw_close,
-    .bdrv_co_flush      = raw_flush,
     .bdrv_has_zero_init = hdev_has_zero_init,
 
-    .bdrv_read		= raw_read,
-    .bdrv_write	        = raw_write,
+    .bdrv_read              = raw_read,
+    .bdrv_write             = raw_write,
+    .bdrv_co_flush_to_disk  = raw_flush,
+
     .bdrv_getlength	= raw_getlength,
     .bdrv_get_allocated_file_size
                         = raw_get_allocated_file_size,
diff --git a/block/raw.c b/block/raw.c
index 33cc471..6098070 100644
--- a/block/raw.c
+++ b/block/raw.c
@@ -111,10 +111,10 @@ static BlockDriver bdrv_raw = {
     .bdrv_open          = raw_open,
     .bdrv_close         = raw_close,
 
-    .bdrv_co_readv      = raw_co_readv,
-    .bdrv_co_writev     = raw_co_writev,
-    .bdrv_co_flush      = raw_co_flush,
-    .bdrv_co_discard    = raw_co_discard,
+    .bdrv_co_readv          = raw_co_readv,
+    .bdrv_co_writev         = raw_co_writev,
+    .bdrv_co_flush_to_disk  = raw_co_flush,
+    .bdrv_co_discard        = raw_co_discard,
 
     .bdrv_probe         = raw_probe,
     .bdrv_getlength     = raw_getlength,
diff --git a/block/rbd.c b/block/rbd.c
index c684e0c..9088c52 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -851,18 +851,18 @@ static BlockDriver bdrv_rbd = {
     .bdrv_file_open     = qemu_rbd_open,
     .bdrv_close         = qemu_rbd_close,
     .bdrv_create        = qemu_rbd_create,
-    .bdrv_co_flush      = qemu_rbd_co_flush,
     .bdrv_get_info      = qemu_rbd_getinfo,
     .create_options     = qemu_rbd_create_options,
     .bdrv_getlength     = qemu_rbd_getlength,
     .bdrv_truncate      = qemu_rbd_truncate,
     .protocol_name      = "rbd",
 
-    .bdrv_aio_readv     = qemu_rbd_aio_readv,
-    .bdrv_aio_writev    = qemu_rbd_aio_writev,
+    .bdrv_aio_readv         = qemu_rbd_aio_readv,
+    .bdrv_aio_writev        = qemu_rbd_aio_writev,
+    .bdrv_co_flush_to_disk  = qemu_rbd_co_flush,
 
-    .bdrv_snapshot_create = qemu_rbd_snap_create,
-    .bdrv_snapshot_list = qemu_rbd_snap_list,
+    .bdrv_snapshot_create   = qemu_rbd_snap_create,
+    .bdrv_snapshot_list     = qemu_rbd_snap_list,
 };
 
 static void bdrv_rbd_init(void)
diff --git a/block/vdi.c b/block/vdi.c
index 523a640..684a4a8 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -980,7 +980,7 @@ static BlockDriver bdrv_vdi = {
     .bdrv_open = vdi_open,
     .bdrv_close = vdi_close,
     .bdrv_create = vdi_create,
-    .bdrv_co_flush = vdi_co_flush,
+    .bdrv_co_flush_to_disk = vdi_co_flush,
     .bdrv_is_allocated = vdi_is_allocated,
     .bdrv_make_empty = vdi_make_empty,
 
diff --git a/block/vmdk.c b/block/vmdk.c
index 985006e..e53a2f0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1581,8 +1581,8 @@ static BlockDriver bdrv_vmdk = {
     .bdrv_write     = vmdk_co_write,
     .bdrv_close     = vmdk_close,
     .bdrv_create    = vmdk_create,
-    .bdrv_co_flush  = vmdk_co_flush,
-    .bdrv_is_allocated  = vmdk_is_allocated,
+    .bdrv_co_flush_to_disk  = vmdk_co_flush,
+    .bdrv_is_allocated      = vmdk_is_allocated,
     .bdrv_get_allocated_file_size  = vmdk_get_allocated_file_size,
 
     .create_options = vmdk_create_options,
diff --git a/block/vpc.c b/block/vpc.c
index 79be7d0..1b6434f 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -661,14 +661,16 @@ static QEMUOptionParameter vpc_create_options[] = {
 static BlockDriver bdrv_vpc = {
     .format_name    = "vpc",
     .instance_size  = sizeof(BDRVVPCState),
+
     .bdrv_probe     = vpc_probe,
     .bdrv_open      = vpc_open,
-    .bdrv_read      = vpc_co_read,
-    .bdrv_write     = vpc_co_write,
-    .bdrv_co_flush  = vpc_co_flush,
     .bdrv_close     = vpc_close,
     .bdrv_create    = vpc_create,
 
+    .bdrv_read              = vpc_co_read,
+    .bdrv_write             = vpc_co_write,
+    .bdrv_co_flush_to_disk  = vpc_co_flush,
+
     .create_options = vpc_create_options,
 };
 
diff --git a/block_int.h b/block_int.h
index f4547f6..5aadc1f 100644
--- a/block_int.h
+++ b/block_int.h
@@ -84,10 +84,15 @@ struct BlockDriver {
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
     int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
-    int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
     int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs,
         int64_t sector_num, int nb_sectors);
 
+    /*
+     * Flushes all data that was already written to the OS all the way down to
+     * the disk (for example raw-posix calls fsync()).
+     */
+    int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
+
     int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
         int num_reqs);
     int (*bdrv_merge_requests)(BlockDriverState *bs, BlockRequest* a,
-- 
1.7.6.4

[Qemu-devel] [PATCH v2 2/3] block: Introduce bdrv_co_flush_to_os

[Kevin Wolf]

qcow2 has a writeback metadata cache, so flushing a qcow2 image actually
consists of writing back that cache to the protocol and only then flushes the
protocol in order to get everything stable on disk.

This introduces a separate bdrv_co_flush_to_os to reflect the split.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c       |   13 ++++++++++++-
 block/qcow2.c |   10 ++++++++--
 block_int.h   |    7 +++++++
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index d3042da..f81dd9f 100644
--- a/block.c
+++ b/block.c
@@ -2782,11 +2782,22 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
 
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
 {
+    int ret;
+
     if (bs->open_flags & BDRV_O_NO_FLUSH) {
         return 0;
     } else if (!bs->drv) {
         return 0;
-    } else if (bs->drv->bdrv_co_flush_to_disk) {
+    }
+
+    if (bs->drv->bdrv_co_flush_to_os) {
+        ret = bs->drv->bdrv_co_flush_to_os(bs);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    if (bs->drv->bdrv_co_flush_to_disk) {
         return bs->drv->bdrv_co_flush_to_disk(bs);
     } else if (bs->drv->bdrv_aio_flush) {
         BlockDriverAIOCB *acb;
diff --git a/block/qcow2.c b/block/qcow2.c
index f7f73fe..5c784ee 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1105,7 +1105,7 @@ fail:
     return ret;
 }
 
-static int qcow2_co_flush(BlockDriverState *bs)
+static int qcow2_co_flush_to_os(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
     int ret;
@@ -1124,6 +1124,11 @@ static int qcow2_co_flush(BlockDriverState *bs)
     }
     qemu_co_mutex_unlock(&s->lock);
 
+    return 0;
+}
+
+static int qcow2_co_flush_to_disk(BlockDriverState *bs)
+{
     return bdrv_co_flush(bs->file);
 }
 
@@ -1245,7 +1250,8 @@ static BlockDriver bdrv_qcow2 = {
 
     .bdrv_co_readv          = qcow2_co_readv,
     .bdrv_co_writev         = qcow2_co_writev,
-    .bdrv_co_flush_to_disk  = qcow2_co_flush,
+    .bdrv_co_flush_to_os    = qcow2_co_flush_to_os,
+    .bdrv_co_flush_to_disk  = qcow2_co_flush_to_disk,
 
     .bdrv_co_discard        = qcow2_co_discard,
     .bdrv_truncate          = qcow2_truncate,
diff --git a/block_int.h b/block_int.h
index 5aadc1f..1ec4921 100644
--- a/block_int.h
+++ b/block_int.h
@@ -93,6 +93,13 @@ struct BlockDriver {
      */
     int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
 
+    /*
+     * Flushes all internal caches to the OS. The data may still sit in a
+     * writeback cache of the host OS, but it will survive a crash of the qemu
+     * process.
+     */
+    int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
+
     int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
         int num_reqs);
     int (*bdrv_merge_requests)(BlockDriverState *bs, BlockRequest* a,
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH v2 2/3] block: Introduce bdrv_co_flush_to_os

[Zhi Yong Wu]

On Fri, Nov 11, 2011 at 1:32 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> qcow2 has a writeback metadata cache, so flushing a qcow2 image actually
> consists of writing back that cache to the protocol and only then flushes the
> protocol in order to get everything stable on disk.
>
> This introduces a separate bdrv_co_flush_to_os to reflect the split.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c       |   13 ++++++++++++-
>  block/qcow2.c |   10 ++++++++--
>  block_int.h   |    7 +++++++
>  3 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index d3042da..f81dd9f 100644
> --- a/block.c
> +++ b/block.c
> @@ -2782,11 +2782,22 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
>
>  int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>  {
> +    int ret;
> +
>     if (bs->open_flags & BDRV_O_NO_FLUSH) {
>         return 0;
>     } else if (!bs->drv) {
>         return 0;
> -    } else if (bs->drv->bdrv_co_flush_to_disk) {
> +    }
> +
> +    if (bs->drv->bdrv_co_flush_to_os) {
> +        ret = bs->drv->bdrv_co_flush_to_os(bs);
> +        if (ret < 0) {
> +            return ret;
> +        }
> +    }
> +
> +    if (bs->drv->bdrv_co_flush_to_disk) {
>         return bs->drv->bdrv_co_flush_to_disk(bs);
>     } else if (bs->drv->bdrv_aio_flush) {
>         BlockDriverAIOCB *acb;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index f7f73fe..5c784ee 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1105,7 +1105,7 @@ fail:
>     return ret;
>  }
>
> -static int qcow2_co_flush(BlockDriverState *bs)
> +static int qcow2_co_flush_to_os(BlockDriverState *bs)
>  {
>     BDRVQcowState *s = bs->opaque;
>     int ret;
> @@ -1124,6 +1124,11 @@ static int qcow2_co_flush(BlockDriverState *bs)
>     }
>     qemu_co_mutex_unlock(&s->lock);
>
> +    return 0;
> +}
> +
> +static int qcow2_co_flush_to_disk(BlockDriverState *bs)
> +{
>     return bdrv_co_flush(bs->file);
>  }
>
> @@ -1245,7 +1250,8 @@ static BlockDriver bdrv_qcow2 = {
>
>     .bdrv_co_readv          = qcow2_co_readv,
>     .bdrv_co_writev         = qcow2_co_writev,
> -    .bdrv_co_flush_to_disk  = qcow2_co_flush,
> +    .bdrv_co_flush_to_os    = qcow2_co_flush_to_os,
> +    .bdrv_co_flush_to_disk  = qcow2_co_flush_to_disk,
>
>     .bdrv_co_discard        = qcow2_co_discard,
>     .bdrv_truncate          = qcow2_truncate,
> diff --git a/block_int.h b/block_int.h
> index 5aadc1f..1ec4921 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -93,6 +93,13 @@ struct BlockDriver {
>      */
>     int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
>
> +    /*
> +     * Flushes all internal caches to the OS. The data may still sit in a
> +     * writeback cache of the host OS, but it will survive a crash of the qemu
Sorry, How to use the data in cache of host OS to survive  acrash of the qemu?

> +     * process.
> +     */
> +    int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
> +
>     int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
>         int num_reqs);
>     int (*bdrv_merge_requests)(BlockDriverState *bs, BlockRequest* a,
> --
> 1.7.6.4
>
>
>



-- 
Regards,

Zhi Yong Wu

Re: [Qemu-devel] [PATCH v2 2/3] block: Introduce bdrv_co_flush_to_os

[Kevin Wolf]

Am 11.11.2011 05:06, schrieb Zhi Yong Wu:
> On Fri, Nov 11, 2011 at 1:32 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>> qcow2 has a writeback metadata cache, so flushing a qcow2 image actually
>> consists of writing back that cache to the protocol and only then flushes the
>> protocol in order to get everything stable on disk.
>>
>> This introduces a separate bdrv_co_flush_to_os to reflect the split.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block.c       |   13 ++++++++++++-
>>  block/qcow2.c |   10 ++++++++--
>>  block_int.h   |    7 +++++++
>>  3 files changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index d3042da..f81dd9f 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -2782,11 +2782,22 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
>>
>>  int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>>  {
>> +    int ret;
>> +
>>     if (bs->open_flags & BDRV_O_NO_FLUSH) {
>>         return 0;
>>     } else if (!bs->drv) {
>>         return 0;
>> -    } else if (bs->drv->bdrv_co_flush_to_disk) {
>> +    }
>> +
>> +    if (bs->drv->bdrv_co_flush_to_os) {
>> +        ret = bs->drv->bdrv_co_flush_to_os(bs);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    if (bs->drv->bdrv_co_flush_to_disk) {
>>         return bs->drv->bdrv_co_flush_to_disk(bs);
>>     } else if (bs->drv->bdrv_aio_flush) {
>>         BlockDriverAIOCB *acb;
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index f7f73fe..5c784ee 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -1105,7 +1105,7 @@ fail:
>>     return ret;
>>  }
>>
>> -static int qcow2_co_flush(BlockDriverState *bs)
>> +static int qcow2_co_flush_to_os(BlockDriverState *bs)
>>  {
>>     BDRVQcowState *s = bs->opaque;
>>     int ret;
>> @@ -1124,6 +1124,11 @@ static int qcow2_co_flush(BlockDriverState *bs)
>>     }
>>     qemu_co_mutex_unlock(&s->lock);
>>
>> +    return 0;
>> +}
>> +
>> +static int qcow2_co_flush_to_disk(BlockDriverState *bs)
>> +{
>>     return bdrv_co_flush(bs->file);
>>  }
>>
>> @@ -1245,7 +1250,8 @@ static BlockDriver bdrv_qcow2 = {
>>
>>     .bdrv_co_readv          = qcow2_co_readv,
>>     .bdrv_co_writev         = qcow2_co_writev,
>> -    .bdrv_co_flush_to_disk  = qcow2_co_flush,
>> +    .bdrv_co_flush_to_os    = qcow2_co_flush_to_os,
>> +    .bdrv_co_flush_to_disk  = qcow2_co_flush_to_disk,
>>
>>     .bdrv_co_discard        = qcow2_co_discard,
>>     .bdrv_truncate          = qcow2_truncate,
>> diff --git a/block_int.h b/block_int.h
>> index 5aadc1f..1ec4921 100644
>> --- a/block_int.h
>> +++ b/block_int.h
>> @@ -93,6 +93,13 @@ struct BlockDriver {
>>      */
>>     int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
>>
>> +    /*
>> +     * Flushes all internal caches to the OS. The data may still sit in a
>> +     * writeback cache of the host OS, but it will survive a crash of the qemu
> Sorry, How to use the data in cache of host OS to survive  acrash of the qemu?

Just use the image file. Start a new qemu instance or whatever and
you'll get the right data.

Kevin

Re: [Qemu-devel] [PATCH v2 2/3] block: Introduce bdrv_co_flush_to_os

[Zhi Yong Wu]

On Fri, Nov 11, 2011 at 5:10 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 11.11.2011 05:06, schrieb Zhi Yong Wu:
>> On Fri, Nov 11, 2011 at 1:32 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> qcow2 has a writeback metadata cache, so flushing a qcow2 image actually
>>> consists of writing back that cache to the protocol and only then flushes the
>>> protocol in order to get everything stable on disk.
>>>
>>> This introduces a separate bdrv_co_flush_to_os to reflect the split.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  block.c       |   13 ++++++++++++-
>>>  block/qcow2.c |   10 ++++++++--
>>>  block_int.h   |    7 +++++++
>>>  3 files changed, 27 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block.c b/block.c
>>> index d3042da..f81dd9f 100644
>>> --- a/block.c
>>> +++ b/block.c
>>> @@ -2782,11 +2782,22 @@ static void coroutine_fn bdrv_flush_co_entry(void *opaque)
>>>
>>>  int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
>>>  {
>>> +    int ret;
>>> +
>>>     if (bs->open_flags & BDRV_O_NO_FLUSH) {
>>>         return 0;
>>>     } else if (!bs->drv) {
>>>         return 0;
>>> -    } else if (bs->drv->bdrv_co_flush_to_disk) {
>>> +    }
>>> +
>>> +    if (bs->drv->bdrv_co_flush_to_os) {
>>> +        ret = bs->drv->bdrv_co_flush_to_os(bs);
>>> +        if (ret < 0) {
>>> +            return ret;
>>> +        }
>>> +    }
>>> +
>>> +    if (bs->drv->bdrv_co_flush_to_disk) {
>>>         return bs->drv->bdrv_co_flush_to_disk(bs);
>>>     } else if (bs->drv->bdrv_aio_flush) {
>>>         BlockDriverAIOCB *acb;
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index f7f73fe..5c784ee 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>>> @@ -1105,7 +1105,7 @@ fail:
>>>     return ret;
>>>  }
>>>
>>> -static int qcow2_co_flush(BlockDriverState *bs)
>>> +static int qcow2_co_flush_to_os(BlockDriverState *bs)
>>>  {
>>>     BDRVQcowState *s = bs->opaque;
>>>     int ret;
>>> @@ -1124,6 +1124,11 @@ static int qcow2_co_flush(BlockDriverState *bs)
>>>     }
>>>     qemu_co_mutex_unlock(&s->lock);
>>>
>>> +    return 0;
>>> +}
>>> +
>>> +static int qcow2_co_flush_to_disk(BlockDriverState *bs)
>>> +{
>>>     return bdrv_co_flush(bs->file);
>>>  }
>>>
>>> @@ -1245,7 +1250,8 @@ static BlockDriver bdrv_qcow2 = {
>>>
>>>     .bdrv_co_readv          = qcow2_co_readv,
>>>     .bdrv_co_writev         = qcow2_co_writev,
>>> -    .bdrv_co_flush_to_disk  = qcow2_co_flush,
>>> +    .bdrv_co_flush_to_os    = qcow2_co_flush_to_os,
>>> +    .bdrv_co_flush_to_disk  = qcow2_co_flush_to_disk,
>>>
>>>     .bdrv_co_discard        = qcow2_co_discard,
>>>     .bdrv_truncate          = qcow2_truncate,
>>> diff --git a/block_int.h b/block_int.h
>>> index 5aadc1f..1ec4921 100644
>>> --- a/block_int.h
>>> +++ b/block_int.h
>>> @@ -93,6 +93,13 @@ struct BlockDriver {
>>>      */
>>>     int coroutine_fn (*bdrv_co_flush_to_disk)(BlockDriverState *bs);
>>>
>>> +    /*
>>> +     * Flushes all internal caches to the OS. The data may still sit in a
>>> +     * writeback cache of the host OS, but it will survive a crash of the qemu
>> Sorry, How to use the data in cache of host OS to survive  acrash of the qemu?
>
> Just use the image file. Start a new qemu instance or whatever and
> you'll get the right data.
thanks.

>
> Kevin
>



-- 
Regards,

Zhi Yong Wu

[Qemu-devel] [PATCH v2 3/3] block: Make cache=unsafe flush to the OS

[Kevin Wolf]

cache=unsafe completely ignored bdrv_flush, because flushing the host disk
costs a lot of performance. However, this means that qcow2 images (and
potentially any other format) can lose data even after the guest has issued a
flush if the qemu process crashes/is killed. In case of a host crash, data loss
is certainly expected with cache=unsafe, but if just the qemu process dies this
is a bit too unsafe.

Now that we have two separate flush functions, we can choose to flush
everythign to the OS, but don't enforce that it's physically written to the
disk.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index f81dd9f..ba8a2fd 100644
--- a/block.c
+++ b/block.c
@@ -2784,12 +2784,11 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
 {
     int ret;
 
-    if (bs->open_flags & BDRV_O_NO_FLUSH) {
-        return 0;
-    } else if (!bs->drv) {
+    if (!bs->drv) {
         return 0;
     }
 
+    /* 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);
         if (ret < 0) {
@@ -2797,6 +2796,11 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
         }
     }
 
+    /* But don't actually force it to the disk with cache=unsafe */
+    if (bs->open_flags & BDRV_O_NO_FLUSH) {
+        return 0;
+    }
+
     if (bs->drv->bdrv_co_flush_to_disk) {
         return bs->drv->bdrv_co_flush_to_disk(bs);
     } else if (bs->drv->bdrv_aio_flush) {
-- 
1.7.6.4

Re: [Qemu-devel] [PATCH v2 0/3] block: Write out internal caches even with cache=unsafe

[Paolo Bonzini]

On 11/10/2011 06:32 PM, Kevin Wolf wrote:
> Paolo, does this implement what you were suggesting?
>
> Kevin Wolf (3):
>    block: Rename bdrv_co_flush to bdrv_co_flush_to_disk
>    block: Introduce bdrv_co_flush_to_os
>    block: Make cache=unsafe flush to the OS
>
>   block.c           |   23 +++++++++++++++++++----
>   block/cow.c       |   22 ++++++++++++----------
>   block/qcow.c      |   18 ++++++++++--------
>   block/qcow2.c     |   14 ++++++++++----
>   block/raw-win32.c |   15 +++++++++------
>   block/raw.c       |    8 ++++----
>   block/rbd.c       |   10 +++++-----
>   block/vdi.c       |    2 +-
>   block/vmdk.c      |    4 ++--
>   block/vpc.c       |    8 +++++---
>   block_int.h       |   14 +++++++++++++-
>   11 files changed, 90 insertions(+), 48 deletions(-)
>

Yes, thanks!

Paolo