| Message ID | 20170904101800.22945-1-pbutsykin@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2: move qcow2_store_persistent_dirty_bitmaps() before cache flushing | expand |
On 09/04/2017 05:18 AM, Pavel Butsykin wrote: > After calling qcow2_inactivate(), all qcow2 caches must be flushed, but this > may not happen, because the last call qcow2_store_persistent_dirty_bitmaps() > can lead to marking l2/refcont cache as dirty. > > Let's move qcow2_store_persistent_dirty_bitmaps() before the caсhe flushing > to fix it. > > Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> > --- > block/qcow2.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > Should this cc: qemu-stable? Reviewed-by: Eric Blake <eblake@redhat.com>
On 05.09.2017 22:30, Eric Blake wrote: > On 09/04/2017 05:18 AM, Pavel Butsykin wrote: >> After calling qcow2_inactivate(), all qcow2 caches must be flushed, but this >> may not happen, because the last call qcow2_store_persistent_dirty_bitmaps() >> can lead to marking l2/refcont cache as dirty. >> >> Let's move qcow2_store_persistent_dirty_bitmaps() before the caсhe flushing >> to fix it. >> >> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> >> --- >> block/qcow2.c | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> > > Should this cc: qemu-stable? The latest stable branch (2.8?) doesn't contain the persistent dirty bitmap. > Reviewed-by: Eric Blake <eblake@redhat.com> >
Am 06.09.2017 um 10:19 hat Pavel Butsykin geschrieben: > On 05.09.2017 22:30, Eric Blake wrote: > > On 09/04/2017 05:18 AM, Pavel Butsykin wrote: > > > After calling qcow2_inactivate(), all qcow2 caches must be flushed, but this > > > may not happen, because the last call qcow2_store_persistent_dirty_bitmaps() > > > can lead to marking l2/refcont cache as dirty. > > > > > > Let's move qcow2_store_persistent_dirty_bitmaps() before the caсhe flushing > > > to fix it. > > > > > > Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> > > > --- > > > block/qcow2.c | 16 ++++++++-------- > > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > > > > Should this cc: qemu-stable? > > The latest stable branch (2.8?) doesn't contain the persistent dirty bitmap. Cc: qemu-stable would now be for qemu 2.10.1, which I think does need the fix. I'm adding the tag. Thanks, applied to the block branch. Kevin
diff --git a/block/qcow2.c b/block/qcow2.c index a3679c69e8..dcf49084c5 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2037,6 +2037,14 @@ static int qcow2_inactivate(BlockDriverState *bs) int ret, result = 0; Error *local_err = NULL; + qcow2_store_persistent_dirty_bitmaps(bs, &local_err); + if (local_err != NULL) { + result = -EINVAL; + error_report_err(local_err); + error_report("Persistent bitmaps are lost for node '%s'", + bdrv_get_device_or_node_name(bs)); + } + ret = qcow2_cache_flush(bs, s->l2_table_cache); if (ret) { result = ret; @@ -2051,14 +2059,6 @@ static int qcow2_inactivate(BlockDriverState *bs) strerror(-ret)); } - qcow2_store_persistent_dirty_bitmaps(bs, &local_err); - if (local_err != NULL) { - result = -EINVAL; - error_report_err(local_err); - error_report("Persistent bitmaps are lost for node '%s'", - bdrv_get_device_or_node_name(bs)); - } - if (result == 0) { qcow2_mark_clean(bs); }
After calling qcow2_inactivate(), all qcow2 caches must be flushed, but this may not happen, because the last call qcow2_store_persistent_dirty_bitmaps() can lead to marking l2/refcont cache as dirty. Let's move qcow2_store_persistent_dirty_bitmaps() before the caсhe flushing to fix it. Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com> --- block/qcow2.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)