Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] ignore bdrv_flush operation when no qcow2 cache item is dirty

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] ignore bdrv_flush operation when no qcow2 cache item is dirty

[Eric Blake]

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

[patches should always be sent to qemu-devel, even if qemu-block is also
in the to/cc list]

On 07/08/2015 01:26 AM, Qingshu Chen wrote:
> qcow2_cache_flush() writes dirty cache to the disk and invokes bdrv_flush()
> to make the data durable. But even if there is no dirty cache,
> qcow2_cache_flush() would invoke bdrv_flush().  In fact, bdrv_flush() will
> invoke fdatasync(), and it is an expensive operation. The patch will not
> invoke bdrv_flush if there is not dirty cache. The reason that I modify the
> return value of qcow2_cache_flush()  is qcow2_co_flush_to_os needs to know
> whether flush operation is called. Following is the patch:
> 
>>From 23f9f83da4178e8fbb53d2cffe128f5a2d3a239a Mon Sep 17 00:00:00 2001
> From: Qingshu Chen <qingshu.chen714@gmail.com>
> Date: Wed, 1 Jul 2015 14:45:23 +0800
> Subject: [PATCH 1/2] ignore bdrv_flush operation when no qcow2 cache item is
>  dirty
> Signed-off-by: Qingshu Chen <qingshu.chen714@gmail.com>

I didn't quickly find an associated 2/2 patch; are you sure you sent the
series correctly?

> 
> ---
>  block/qcow2-cache.c | 9 ++++++++-
>  block/qcow2.c       | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> index ed92a09..57c0601 100644
> --- a/block/qcow2-cache.c
> +++ b/block/qcow2-cache.c
> @@ -174,6 +174,7 @@ int qcow2_cache_flush(BlockDriverState *bs, Qcow2Cache
> *c)
>      int result = 0;
>      int ret;
>      int i;
> +    int flag = 0;

This is used as a bool, so declare it as such (bool flag = false;).

-- 
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: 604 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] ignore bdrv_flush operation when no qcow2 cache item is dirty

[Qingshu Chen]

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

I've made a mistake on the series. Following is the new patch:

>From ef1079b422eef40a802ca13e249795005efa441d Mon Sep 17 00:00:00 2001
From: Qingshu Chen <1150163259@qq.com>
Date: Tue, 21 Jul 2015 09:46:08 +0800
Subject: [PATCH] ignore bdrv_flush operation when no qcow2 cache item is
dirty
Signed-off-by: Qingshu Chen <qingshu.chen714@gmail.com>

---
 block/qcow2-cache.c | 16 ++++++++++++----
 block/qcow2.c       |  2 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 53b8afc..08d1884 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -174,23 +174,31 @@ int qcow2_cache_flush(BlockDriverState *bs,
Qcow2Cache *c)
     int result = 0;
     int ret;
     int i;
+    bool flag = false;

     trace_qcow2_cache_flush(qemu_coroutine_self(), c == s->l2_table_cache);

     for (i = 0; i < c->size; i++) {
-        ret = qcow2_cache_entry_flush(bs, c, i);
-        if (ret < 0 && result != -ENOSPC) {
-            result = ret;
+        if (c->entries[i].dirty && c->entries[i].offset) {
+            flag = true;
+            ret = qcow2_cache_entry_flush(bs, c, i);
+            if (ret < 0 && result != -ENOSPC) {
+                result = ret;
+            }
         }
     }

-    if (result == 0) {
+    if (result == 0 && flag) {
         ret = bdrv_flush(bs->file);
         if (ret < 0) {
             result = ret;
         }
     }

+    if (!flag && result >= 0) {
+        result = 1;
+    }
+
     return result;
 }

diff --git a/block/qcow2.c b/block/qcow2.c
index 76c331b..2e95cc1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2505,6 +2505,8 @@ static coroutine_fn int
qcow2_co_flush_to_os(BlockDriverState *bs)
     if (ret < 0) {
         qemu_co_mutex_unlock(&s->lock);
         return ret;
+    } else if (ret == 1) {
+        bdrv_flush(bs->file);
     }

     if (qcow2_need_accurate_refcounts(s)) {
--
1.9.1


2015-07-20 23:03 GMT+08:00 Eric Blake <eblake@redhat.com>:
>
> [patches should always be sent to qemu-devel, even if qemu-block is also
> in the to/cc list]
>
> On 07/08/2015 01:26 AM, Qingshu Chen wrote:
> > qcow2_cache_flush() writes dirty cache to the disk and invokes
bdrv_flush()
> > to make the data durable. But even if there is no dirty cache,
> > qcow2_cache_flush() would invoke bdrv_flush().  In fact, bdrv_flush()
will
> > invoke fdatasync(), and it is an expensive operation. The patch will not
> > invoke bdrv_flush if there is not dirty cache. The reason that I modify
the
> > return value of qcow2_cache_flush()  is qcow2_co_flush_to_os needs to
know
> > whether flush operation is called. Following is the patch:
> >
> >>From 23f9f83da4178e8fbb53d2cffe128f5a2d3a239a Mon Sep 17 00:00:00 2001
> > From: Qingshu Chen <qingshu.chen714@gmail.com>
> > Date: Wed, 1 Jul 2015 14:45:23 +0800
> > Subject: [PATCH 1/2] ignore bdrv_flush operation when no qcow2 cache
item is
> >  dirty
> > Signed-off-by: Qingshu Chen <qingshu.chen714@gmail.com>
>
> I didn't quickly find an associated 2/2 patch; are you sure you sent the
> series correctly?
>
> >
> > ---
> >  block/qcow2-cache.c | 9 ++++++++-
> >  block/qcow2.c       | 2 ++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> > index ed92a09..57c0601 100644
> > --- a/block/qcow2-cache.c
> > +++ b/block/qcow2-cache.c
> > @@ -174,6 +174,7 @@ int qcow2_cache_flush(BlockDriverState *bs,
Qcow2Cache
> > *c)
> >      int result = 0;
> >      int ret;
> >      int i;
> > +    int flag = 0;
>
> This is used as a bool, so declare it as such (bool flag = false;).
>
> --
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>



--





-------------------------------------------------------------------------------------------------------------
Qingshu Chen,
Institute of Parallel and Distributed Systems (IPADS),
School of Software,
Shanghai Jiao Tong University
---------------------------------------------------------------------------------------------------------------

[-- Attachment #2: Type: text/html, Size: 5305 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] ignore bdrv_flush operation when no qcow2 cache item is dirty

[Eric Blake]

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

On 07/20/2015 07:51 PM, Qingshu Chen wrote:
> I've made a mistake on the series. Following is the new patch:

When re-sending a patch, it's better to start a new thread and include a
'v2' in the subject line.

> 
>>From ef1079b422eef40a802ca13e249795005efa441d Mon Sep 17 00:00:00 2001
> From: Qingshu Chen <1150163259@qq.com>
> Date: Tue, 21 Jul 2015 09:46:08 +0800
> Subject: [PATCH] ignore bdrv_flush operation when no qcow2 cache item is
> dirty
> Signed-off-by: Qingshu Chen <qingshu.chen714@gmail.com>
> 
> ---
>  block/qcow2-cache.c | 16 ++++++++++++----
>  block/qcow2.c       |  2 ++
>  2 files changed, 14 insertions(+), 4 deletions(-)

>      if (qcow2_need_accurate_refcounts(s)) {
> --
> 1.9.1
> 
> 
> 2015-07-20 23:03 GMT+08:00 Eric Blake <eblake@redhat.com>:
>>
>> [patches should always be sent to qemu-devel, even if qemu-block is also
>> in the to/cc list]
>>
>> On 07/08/2015 01:26 AM, Qingshu Chen wrote:
>>> qcow2_cache_flush() writes dirty cache to the disk and invokes
> bdrv_flush()

Replying to an existing mail, and worse, keeping the old patch in the
reply, makes it harder for maintainers to extract your patch.

For more hints on patch submissions, see
http://wiki.qemu.org/Contribute/SubmitAPatch

-- 
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: 604 bytes --]