qcow2: Clean-up update_cluster_refcount().
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/4 qcow2: Clean-up update_cluster_refcount().
pièce jointe document texte brut (0001-Clean-up-update_cluster_refcount.patch) Move some parts to alloc_refcount_block() and block cache
checking to load_refcount_block().
--- block-qcow2.c | 71 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 28 deletions(-)
block-qcow2.c +43 -28
@@ -2169,6 +2169,10 @@ static int load_refcount_block(BlockDriv {BDRVQcowState *s = bs->opaque;int ret; + +if (refcount_block_offset == s->refcount_block_cache_offset) +return 0 ; + ret =bdrv_pread (s->hd, refcount_block_offset, s->refcount_block_cache, s->cluster_size);if (ret != s->cluster_size)
@@ -2189,11 +2193,8 @@ static int get_refcount(BlockDriverState refcount_block_offset = s->refcount_table[refcount_table_index];if (!refcount_block_offset)return 0 ; -if (refcount_block_offset != s->refcount_block_cache_offset) { -/* better than nothing: return allocated if read error */ -if (load_refcount_block (bs, refcount_block_offset) <0 ) -return 1 ; - } +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return 1 ; block_index = cluster_index & ((1 << (s->cluster_bits -REFCOUNT_SHIFT )) -1 );return be16_to_cpu (s->refcount_block_cache[block_index]);
@@ -2227,6 +2228,38 @@ static int64_t alloc_clusters_noref(Bloc } } +/* create a new refcount block */ + +static int64_t alloc_refcount_block (BlockDriverState *bs,int refcount_table_index) +{ +BDRVQcowState *s = bs->opaque; +int64_t offset; +int ret; +uint64_t data64; + +/* Note: we cannot update the refcount now to avoid recursion */ + + offset =alloc_clusters_noref (bs, s->cluster_size); +memset (s->refcount_block_cache,0 , s->cluster_size); + + ret =bdrv_pwrite (s->hd, offset, s->refcount_block_cache, s->cluster_size); +if (ret != s->cluster_size) +return -1 ; + + s->refcount_table[refcount_table_index] = offset; + data64 =cpu_to_be64 (offset); + ret =bdrv_pwrite (s->hd, s->refcount_table_offset + + refcount_table_index *sizeof (uint64_t ), + &data64,sizeof (data64)); +if (ret !=sizeof (data64)) +return -1 ; + + s->refcount_block_cache_offset = offset; +update_refcount (bs, offset, s->cluster_size,1 ); + +return offset; +} +static int64_t alloc_clusters (BlockDriverState *bs,int64_t size) {int64_t offset;
@@ -2359,9 +2392,8 @@ static int update_cluster_refcount(Block int addend) {BDRVQcowState *s = bs->opaque; -int64_t offset, refcount_block_offset; +int64_t refcount_block_offset;int ret, refcount_table_index, block_index, refcount; -uint64_t data64; refcount_table_index = cluster_index >> (s->cluster_bits -REFCOUNT_SHIFT );if (refcount_table_index >= s->refcount_table_size) {
@@ -2375,29 +2407,12 @@ static int update_cluster_refcount(Blockif (!refcount_block_offset) {if (addend <0 )return -EINVAL ; -/* create a new refcount block */ -/* Note: we cannot update the refcount now to avoid recursion */ - offset =alloc_clusters_noref (bs, s->cluster_size); -memset (s->refcount_block_cache,0 , s->cluster_size); - ret =bdrv_pwrite (s->hd, offset, s->refcount_block_cache, s->cluster_size); -if (ret != s->cluster_size) -return -EINVAL ; - s->refcount_table[refcount_table_index] = offset; - data64 =cpu_to_be64 (offset); - ret =bdrv_pwrite (s->hd, s->refcount_table_offset + - refcount_table_index *sizeof (uint64_t ), - &data64,sizeof (data64)); -if (ret !=sizeof (data64)) + refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); +if (refcount_block_offset <0 )return -EINVAL ; - - refcount_block_offset = offset; - s->refcount_block_cache_offset = offset; -update_refcount (bs, offset, s->cluster_size,1 ); }else { -if (refcount_block_offset != s->refcount_block_cache_offset) { -if (load_refcount_block (bs, refcount_block_offset) <0 ) -return -EIO ; - } +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return -EIO ; }/* we can update the count and save it */ block_index = cluster_index &
Quoted 26 lines · Laurent Vivier
pièce jointe document texte brut (0001-Clean-up-update_cluster_refcount.patch) Move some parts to
alloc_refcount_block()and block cache
checking toload_refcount_block().Signed-off-by Laurent Vivier <[email protected]>--- block-qcow2.c | 71 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 28 deletions(-)Index:
qemu/block-qcow2.c
===================================================================block-qcow2.c +4
-0@@ -2169,6 +2169,10 @@ static int load_refcount_block(BlockDriv {BDRVQcowState *s = bs->opaque;int ret; + +if (refcount_block_offset == s->refcount_block_cache_offset) +return 0 ; + ret =bdrv_pread (s->hd, refcount_block_offset, s->refcount_block_cache, s->cluster_size);if (ret != s->cluster_size)
block-qcow2.c static int get_refcount(BlockDriverState · line 2197
Quoted 10 lines
block-qcow2.c +2
-5@@ -2189,11 +2193,8 @@ static int get_refcount(BlockDriverState refcount_block_offset = s->refcount_table[refcount_table_index];if (!refcount_block_offset)return 0 ; -if (refcount_block_offset != s->refcount_block_cache_offset) { -/* better than nothing: return allocated if read error */ -if (load_refcount_block (bs, refcount_block_offset) <0 ) -return 1 ; - } +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return 1 ;
Please retain the comment saying that 1 is not the correct value but we default to allocated for safety if the block could not be read. I don't think this is obvious.
Otherwise the patch looks fine to me.
Kevin
Quoted 33 lines · Kevin Wolf
Quoted 32 lines · Laurent Vivier
pièce jointe document texte brut (0001-Clean-up-update_cluster_refcount.patch) Move some parts to
alloc_refcount_block()and block cache
checking toload_refcount_block().Signed-off-by Laurent Vivier <[email protected]>--- block-qcow2.c | 71 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 28 deletions(-)Index:
qemu/block-qcow2.c
===================================================================block-qcow2.c +4
-3@@ -2169,6 +2169,10 @@ static int load_refcount_block(BlockDriv {BDRVQcowState *s = bs->opaque;int ret; + +if (refcount_block_offset == s->refcount_block_cache_offset) +return 0 ; + ret =bdrv_pread (s->hd, refcount_block_offset, s->refcount_block_cache, s->cluster_size);if (ret != s->cluster_size)@@ -2189,11 +2193,8 @@ static int get_refcount(BlockDriverState refcount_block_offset = s->refcount_table[refcount_table_index];if (!refcount_block_offset)return 0 ; -if (refcount_block_offset != s->refcount_block_cache_offset) { -/* better than nothing: return allocated if read error */ -if (load_refcount_block (bs, refcount_block_offset) <0 )
Quoted 8 lines
block-qcow2.c static int get_refcount(BlockDriverState · line 2197
Quoted 4 lines
block-qcow2.c +2
-2-return 1 ; - } +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return 1 ;Please retain the comment saying that 1 is not the correct value but we default to allocated for safety if the block could not be read. I don't think this is obvious.
I totally agree with you, but it's just a code move, I don't want to change anything except the line numbers.
I think this kind of change must have its own patch.
Regards,
Laurent
Signature · 3 lines
------------------ [email protected] ------------------ "Tout ce qui est impossible reste à accomplir" Jules Verne "Things are only impossible until they're not" Jean-Luc Picard
Quoted 42 lines · Laurent Vivier
Quoted 32 lines · Kevin Wolf
Quoted 31 lines · Laurent Vivier
pièce jointe document texte brut (0001-Clean-up-update_cluster_refcount.patch) Move some parts to
alloc_refcount_block()and block cache
checking toload_refcount_block().Signed-off-by Laurent Vivier <[email protected]>--- block-qcow2.c | 71 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 28 deletions(-)Index:
qemu/block-qcow2.c
===================================================================block-qcow2.c +4
-2@@ -2169,6 +2169,10 @@ static int load_refcount_block(BlockDriv {BDRVQcowState *s = bs->opaque;int ret; + +if (refcount_block_offset == s->refcount_block_cache_offset) +return 0 ; + ret =bdrv_pread (s->hd, refcount_block_offset, s->refcount_block_cache, s->cluster_size);if (ret != s->cluster_size)@@ -2189,11 +2193,8 @@ static int get_refcount(BlockDriverState refcount_block_offset = s->refcount_table[refcount_table_index];if (!refcount_block_offset)return 0 ; -if (refcount_block_offset != s->refcount_block_cache_offset) { -/* better than nothing: return allocated if read error */ Quoted 8 lines
block-qcow2.c static int get_refcount(BlockDriverState · line 2197
Quoted 5 lines
block-qcow2.c +2
-3-if (load_refcount_block (bs, refcount_block_offset) <0 ) -return 1 ; - } +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return 1 ;Please retain the comment saying that 1 is not the correct value but we default to allocated for safety if the block could not be read. I don't think this is obvious.
Quoted 4 lines
I totally agree with you, but it's just a code move, I don't want to change anything except the line numbers.
I think this kind of change must have its own patch.
No, it's a move which drops a comment and it shouldn't do that. ;-)
Quoted 1 line
- /* better than nothing: return allocated if read error */
Kevin
Quoted 43 lines · Kevin Wolf
Quoted 42 lines · Laurent Vivier
Quoted 40 lines · Kevin Wolf
Quoted 26 lines · Laurent Vivier
pièce jointe document texte brut (0001-Clean-up-update_cluster_refcount.patch) Move some parts to
alloc_refcount_block()and block cache
checking toload_refcount_block().Signed-off-by Laurent Vivier <[email protected]>--- block-qcow2.c | 71 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 43 insertions(+), 28 deletions(-)Index:
qemu/block-qcow2.c
===================================================================block-qcow2.c +4
-0@@ -2169,6 +2169,10 @@ static int load_refcount_block(BlockDriv {BDRVQcowState *s = bs->opaque;int ret; + +if (refcount_block_offset == s->refcount_block_cache_offset) +return 0 ; + ret =bdrv_pread (s->hd, refcount_block_offset, s->refcount_block_cache, s->cluster_size);if (ret != s->cluster_size)block-qcow2.c static int get_refcount(BlockDriverState · line 2197
Quoted 10 lines
block-qcow2.c +2
-5@@ -2189,11 +2193,8 @@ static int get_refcount(BlockDriverState refcount_block_offset = s->refcount_table[refcount_table_index];if (!refcount_block_offset)return 0 ; -if (refcount_block_offset != s->refcount_block_cache_offset) { -/* better than nothing: return allocated if read error */ -if (load_refcount_block (bs, refcount_block_offset) <0 ) -return 1 ; - } +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return 1 ;Please retain the comment saying that 1 is not the correct value but we default to allocated for safety if the block could not be read. I don't think this is obvious.
Quoted 8 lines
Quoted 4 lines
I totally agree with you, but it's just a code move, I don't want to change anything except the line numbers.
I think this kind of change must have its own patch.
No, it's a move which drops a comment and it shouldn't do that. ;-)
Quoted 1 line
- /* better than nothing: return allocated if read error */
Oops...
Laurent
Signature · 3 lines
------------------ [email protected] ------------------ "Tout ce qui est impossible reste à accomplir" Jules Verne "Things are only impossible until they're not" Jean-Luc Picard
PATCH 2/4 qcow2: Allow update_cluster_refcount() to update several clusters refcount.
In reply to a message not in the archivepièce jointe document texte brut (0002-Allow-update_cluster_refcount-to-update-several-cl.patch) To improve performance when the qcow2 file is empty, this patch allows update_cluster_refcount() to update refcount of several clusters.
--- block-qcow2.c | 107 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 39 deletions(-)
block-qcow2.c +68 -39
@@ -159,6 +159,7 @@ static void refcount_close(BlockDriverStstatic int get_refcount (BlockDriverState *bs,int64_t cluster_index);static int update_cluster_refcount (BlockDriverState *bs,int64_t cluster_index, +int nb_clusters,int addend); static void update_refcount(BlockDriverState *bs, int64_t offset, int64_t length,
@@ -1711,7 +1712,7 @@ static int update_snapshot_refcount(Bloc refcount =2 ; }else {if (addend !=0 ) { - refcount =update_cluster_refcount (bs, offset >> s->cluster_bits, addend); + refcount =update_cluster_refcount (bs, offset >> s->cluster_bits,1 , addend); }else { refcount =get_refcount (bs, offset >> s->cluster_bits); }
@@ -1733,7 +1734,7 @@ static int update_snapshot_refcount(Bloc }if (addend !=0 ) { - refcount =update_cluster_refcount (bs, l2_offset >> s->cluster_bits, addend); + refcount =update_cluster_refcount (bs, l2_offset >> s->cluster_bits,1 , addend); }else { refcount =get_refcount (bs, l2_offset >> s->cluster_bits); }
@@ -2292,14 +2293,14 @@ static int64_t alloc_bytes(BlockDriverStif (free_in_cluster ==0 ) s->free_byte_offset =0 ;if ((offset & (s->cluster_size -1 )) !=0 ) -update_cluster_refcount (bs, offset >> s->cluster_bits,1 ); +update_cluster_refcount (bs, offset >> s->cluster_bits,1 ,1 ); }else { offset =alloc_clusters (bs, s->cluster_size); cluster_offset = s->free_byte_offset & ~(s->cluster_size -1 );if ((cluster_offset + s->cluster_size) == offset) {/* we are lucky: contiguous data */ offset = s->free_byte_offset; -update_cluster_refcount (bs, offset >> s->cluster_bits,1 ); +update_cluster_refcount (bs, offset >> s->cluster_bits,1 ,1 ); s->free_byte_offset += size; }else { s->free_byte_offset = offset;
@@ -2389,46 +2390,77 @@ static int grow_refcount_table(BlockDriv/* XXX: cache several refcount block clusters ? */ static int update_cluster_refcount (BlockDriverState *bs,int64_t cluster_index, +int nb_clusters,int addend) {BDRVQcowState *s = bs->opaque;int64_t refcount_block_offset; -int ret, refcount_table_index, block_index, refcount; +int ret, refcount_table_index, refcount_table_last_index, block_index, refcount; +int nb_block_index; +int refcount_cache_size; - refcount_table_index = cluster_index >> (s->cluster_bits -REFCOUNT_SHIFT ); -if (refcount_table_index >= s->refcount_table_size) { +if (nb_clusters ==0 ) +return 0 ; + + refcount_table_last_index = (cluster_index + nb_clusters -1 ) >> + (s->cluster_bits -REFCOUNT_SHIFT ); + +/* grow the refcount table if needed */ + +if (refcount_table_last_index >= s->refcount_table_size) {if (addend <0 )return -EINVAL ; - ret =grow_refcount_table (bs, refcount_table_index +1 ); + ret =grow_refcount_table (bs, refcount_table_last_index +1 );if (ret <0 )return ret; } - refcount_block_offset = s->refcount_table[refcount_table_index]; -if (!refcount_block_offset) { -if (addend <0 ) -return -EINVAL ; - refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); -if (refcount_block_offset <0 ) -return -EINVAL ; - }else { -if (load_refcount_block (bs, refcount_block_offset) <0 ) + +while (nb_clusters) { + refcount_table_index = cluster_index >> + (s->cluster_bits -REFCOUNT_SHIFT ); + refcount_block_offset = s->refcount_table[refcount_table_index]; + +if (!refcount_block_offset) { +if (addend <0 ) +return -EINVAL ; + refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); +if (refcount_block_offset <0 ) +return -EINVAL ; + }else { +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return -EIO ; + } + +/* we can update the count and save it */ + + refcount_cache_size =1 << (s->cluster_bits -REFCOUNT_SHIFT ); + nb_block_index =0 ; + block_index = cluster_index & (refcount_cache_size -1 ); + refcount =0 ; +while (nb_clusters && + block_index + nb_block_index < refcount_cache_size) { + + refcount =be16_to_cpu ( + s->refcount_block_cache[block_index + nb_block_index]); + refcount += addend; +if (refcount <0 || refcount >0xffff ) +return -EINVAL ; +if (refcount ==0 && + cluster_index + nb_block_index < s->free_cluster_index) { + s->free_cluster_index = cluster_index + nb_block_index; + } + s->refcount_block_cache[block_index + nb_block_index] = +cpu_to_be16 (refcount); + nb_block_index++; + nb_clusters--; + } +if (bdrv_pwrite (s->hd, + refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), + s->refcount_block_cache + block_index, + nb_block_index *sizeof (uint16_t )) != + nb_block_index *sizeof (uint16_t ))return -EIO ; } -/* we can update the count and save it */ - block_index = cluster_index & - ((1 << (s->cluster_bits -REFCOUNT_SHIFT )) -1 ); - refcount =be16_to_cpu (s->refcount_block_cache[block_index]); - refcount += addend; -if (refcount <0 || refcount >0xffff ) -return -EINVAL ; -if (refcount ==0 && cluster_index < s->free_cluster_index) { - s->free_cluster_index = cluster_index; - } - s->refcount_block_cache[block_index] =cpu_to_be16 (refcount); -if (bdrv_pwrite (s->hd, - refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), - &s->refcount_block_cache[block_index],2 ) !=2 ) -return -EIO ;return refcount; }
@@ -2437,7 +2469,7 @@ static void update_refcount(BlockDriverS int addend) {BDRVQcowState *s = bs->opaque; -int64_t start, last, cluster_offset; +int64_t start, last;#ifdef DEBUG_ALLOC2 printf("update_refcount: offset=%lld size=%lld addend=%d\n",
@@ -2445,12 +2477,9 @@ static void update_refcount(BlockDriverS#endif if (length <=0 )return ; - start = offset & ~(s->cluster_size -1 ); - last = (offset + length -1 ) & ~(s->cluster_size -1 ); -for (cluster_offset = start; cluster_offset <= last; - cluster_offset += s->cluster_size) { -update_cluster_refcount (bs, cluster_offset >> s->cluster_bits, addend); - } + start = offset >> s->cluster_bits; + last = (offset + length) >> s->cluster_bits; +update_cluster_refcount (bs, start, last - start +1 , addend); }#ifdef DEBUG_ALLOC
Re: [Qemu-devel] [PATCH 2/4] qcow2: Allow update_cluster_refcount() to update several clusters refcount.
Quoted 94 lines · Laurent Vivier
pièce jointe document texte brut (0002-Allow-update_cluster_refcount-to-update-several-cl.patch) To improve performance when the qcow2 file is empty, this patch allows
update_cluster_refcount()to update refcount of several clusters.Signed-off-by Laurent Vivier <[email protected]>--- block-qcow2.c | 107 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 39 deletions(-)Index:
qemu/block-qcow2.c
===================================================================block-qcow2.c +19
-12@@ -159,6 +159,7 @@ static void refcount_close(BlockDriverStstatic int get_refcount (BlockDriverState *bs,int64_t cluster_index);static int update_cluster_refcount (BlockDriverState *bs,int64_t cluster_index, +int nb_clusters,int addend); static void update_refcount(BlockDriverState *bs, int64_t offset, int64_t length,@@ -1711,7 +1712,7 @@ static int update_snapshot_refcount(Bloc refcount =2 ; }else {if (addend !=0 ) { - refcount =update_cluster_refcount (bs, offset >> s->cluster_bits, addend); + refcount =update_cluster_refcount (bs, offset >> s->cluster_bits,1 , addend); }else { refcount =get_refcount (bs, offset >> s->cluster_bits); }@@ -1733,7 +1734,7 @@ static int update_snapshot_refcount(Bloc }if (addend !=0 ) { - refcount =update_cluster_refcount (bs, l2_offset >> s->cluster_bits, addend); + refcount =update_cluster_refcount (bs, l2_offset >> s->cluster_bits,1 , addend); }else { refcount =get_refcount (bs, l2_offset >> s->cluster_bits); }@@ -2292,14 +2293,14 @@ static int64_t alloc_bytes(BlockDriverStif (free_in_cluster ==0 ) s->free_byte_offset =0 ;if ((offset & (s->cluster_size -1 )) !=0 ) -update_cluster_refcount (bs, offset >> s->cluster_bits,1 ); +update_cluster_refcount (bs, offset >> s->cluster_bits,1 ,1 ); }else { offset =alloc_clusters (bs, s->cluster_size); cluster_offset = s->free_byte_offset & ~(s->cluster_size -1 );if ((cluster_offset + s->cluster_size) == offset) {/* we are lucky: contiguous data */ offset = s->free_byte_offset; -update_cluster_refcount (bs, offset >> s->cluster_bits,1 ); +update_cluster_refcount (bs, offset >> s->cluster_bits,1 ,1 ); s->free_byte_offset += size; }else { s->free_byte_offset = offset;@@ -2389,46 +2390,77 @@ static int grow_refcount_table(BlockDriv/* XXX: cache several refcount block clusters ? */ static int update_cluster_refcount (BlockDriverState *bs,int64_t cluster_index, +int nb_clusters,int addend) {BDRVQcowState *s = bs->opaque;int64_t refcount_block_offset; -int ret, refcount_table_index, block_index, refcount; +int ret, refcount_table_index, refcount_table_last_index, block_index, refcount; +int nb_block_index; +int refcount_cache_size; - refcount_table_index = cluster_index >> (s->cluster_bits -REFCOUNT_SHIFT ); -if (refcount_table_index >= s->refcount_table_size) { +if (nb_clusters ==0 ) +return 0 ; + + refcount_table_last_index = (cluster_index + nb_clusters -1 ) >> + (s->cluster_bits -REFCOUNT_SHIFT ); + +/* grow the refcount table if needed */ + +if (refcount_table_last_index >= s->refcount_table_size) {if (addend <0 )return -EINVAL ; - ret =grow_refcount_table (bs, refcount_table_index +1 ); + ret =grow_refcount_table (bs, refcount_table_last_index +1 );if (ret <0 )return ret; } - refcount_block_offset = s->refcount_table[refcount_table_index]; -if (!refcount_block_offset) { -if (addend <0 ) -return -EINVAL ;
block-qcow2.c static int grow_refcount_table(BlockDriv · line 2421
Quoted 10 lines
block-qcow2.c +5
-5- refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); -if (refcount_block_offset <0 ) -return -EINVAL ; - }else { -if (load_refcount_block (bs, refcount_block_offset) <0 ) + +while (nb_clusters) { + refcount_table_index = cluster_index >> + (s->cluster_bits -REFCOUNT_SHIFT ); + refcount_block_offset = s->refcount_table[refcount_table_index];
I guess (comment? ;-)) this outer loop is for handling requests which span multiple refcount blocks? If so, am I missing something or is refcount_block_offset the same for each iteration because cluster_index never changes?
Quoted 6 lines
block-qcow2.c +6
-0+ +if (!refcount_block_offset) { +if (addend <0 ) +return -EINVAL ; + refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); +if (refcount_block_offset <0 )
block-qcow2.c static int grow_refcount_table(BlockDriv · line 2437
Quoted 10 lines
block-qcow2.c +10
-0+return -EINVAL ; + }else { +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return -EIO ; + } + +/* we can update the count and save it */ + + refcount_cache_size =1 << (s->cluster_bits -REFCOUNT_SHIFT ); + nb_block_index =0 ;
I have to admit that nb_block_index is a name where I can't image what the variable might be for. Seems to be a counter for the changed refcounts in the current refcount block, and block_index seems to be the first index to be touched in this block. What about first_index and cur_refcount or something like that? (I don't like these suggestions too much, either. Maybe someone has better names.)
block-qcow2.c static int grow_refcount_table(BlockDriv · line 2447
Quoted 10 lines
block-qcow2.c +10
-0+ block_index = cluster_index & (refcount_cache_size -1 ); + refcount =0 ; +while (nb_clusters && + block_index + nb_block_index < refcount_cache_size) { + + refcount =be16_to_cpu ( + s->refcount_block_cache[block_index + nb_block_index]); + refcount += addend; +if (refcount <0 || refcount >0xffff ) +return -EINVAL ;
Here we possibly abort in the middle of the operation. If it fails somewhere in the fifth refcount block, what happens with the four already updated blocks?
Quoted 5 lines
block-qcow2.c +5
-0+if (refcount ==0 && + cluster_index + nb_block_index < s->free_cluster_index) { + s->free_cluster_index = cluster_index + nb_block_index; + } + s->refcount_block_cache[block_index + nb_block_index] =
block-qcow2.c static int grow_refcount_table(BlockDriv · line 2462
Quoted 10 lines
block-qcow2.c +9
-0+cpu_to_be16 (refcount); + nb_block_index++; + nb_clusters--; + } +if (bdrv_pwrite (s->hd, + refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), + s->refcount_block_cache + block_index, + nb_block_index *sizeof (uint16_t )) != + nb_block_index *sizeof (uint16_t ))return -EIO ;
Same here.
Quoted 30 lines
block-qcow2.c +1
-16} -/* we can update the count and save it */ - block_index = cluster_index & - ((1 << (s->cluster_bits -REFCOUNT_SHIFT )) -1 ); - refcount =be16_to_cpu (s->refcount_block_cache[block_index]); - refcount += addend; -if (refcount <0 || refcount >0xffff ) -return -EINVAL ; -if (refcount ==0 && cluster_index < s->free_cluster_index) { - s->free_cluster_index = cluster_index; - } - s->refcount_block_cache[block_index] =cpu_to_be16 (refcount); -if (bdrv_pwrite (s->hd, - refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), - &s->refcount_block_cache[block_index],2 ) !=2 ) -return -EIO ;return refcount; }@@ -2437,7 +2469,7 @@ static void update_refcount(BlockDriverS int addend) {BDRVQcowState *s = bs->opaque; -int64_t start, last, cluster_offset; +int64_t start, last;#ifdef DEBUG_ALLOC2 printf("update_refcount: offset=%lld size=%lld addend=%d\n",@@ -2445,12 +2477,9 @@ static void update_refcount(BlockDriverS#endif
block-qcow2.c static void update_refcount(BlockDriverS · line 2481
Quoted 10 lines
block-qcow2.c +2
-6if (length <=0 )return ; - start = offset & ~(s->cluster_size -1 ); - last = (offset + length -1 ) & ~(s->cluster_size -1 ); -for (cluster_offset = start; cluster_offset <= last; - cluster_offset += s->cluster_size) { -update_cluster_refcount (bs, cluster_offset >> s->cluster_bits, addend); - } + start = offset >> s->cluster_bits; + last = (offset + length) >> s->cluster_bits;
Off by one for length % cluster_size == 0?
block-qcow2.c static void update_refcount(BlockDriverS · line 2485
Quoted 5 lines
block-qcow2.c +1
-0+update_cluster_refcount (bs, start, last - start +1 , addend); }#ifdef DEBUG_ALLOC
Kevin
Re: [Qemu-devel] [PATCH 2/4] qcow2: Allow update_cluster_refcount() to update several clusters refcount.
Quoted 100 lines · Kevin Wolf
Quoted 99 lines · Laurent Vivier
pièce jointe document texte brut (0002-Allow-update_cluster_refcount-to-update-several-cl.patch) To improve performance when the qcow2 file is empty, this patch allows
update_cluster_refcount()to update refcount of several clusters.Signed-off-by Laurent Vivier <[email protected]>--- block-qcow2.c | 107 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 68 insertions(+), 39 deletions(-)Index:
qemu/block-qcow2.c
===================================================================block-qcow2.c +19
-17@@ -159,6 +159,7 @@ static void refcount_close(BlockDriverStstatic int get_refcount (BlockDriverState *bs,int64_t cluster_index);static int update_cluster_refcount (BlockDriverState *bs,int64_t cluster_index, +int nb_clusters,int addend); static void update_refcount(BlockDriverState *bs, int64_t offset, int64_t length,@@ -1711,7 +1712,7 @@ static int update_snapshot_refcount(Bloc refcount =2 ; }else {if (addend !=0 ) { - refcount =update_cluster_refcount (bs, offset >> s->cluster_bits, addend); + refcount =update_cluster_refcount (bs, offset >> s->cluster_bits,1 , addend); }else { refcount =get_refcount (bs, offset >> s->cluster_bits); }@@ -1733,7 +1734,7 @@ static int update_snapshot_refcount(Bloc }if (addend !=0 ) { - refcount =update_cluster_refcount (bs, l2_offset >> s->cluster_bits, addend); + refcount =update_cluster_refcount (bs, l2_offset >> s->cluster_bits,1 , addend); }else { refcount =get_refcount (bs, l2_offset >> s->cluster_bits); }@@ -2292,14 +2293,14 @@ static int64_t alloc_bytes(BlockDriverStif (free_in_cluster ==0 ) s->free_byte_offset =0 ;if ((offset & (s->cluster_size -1 )) !=0 ) -update_cluster_refcount (bs, offset >> s->cluster_bits,1 ); +update_cluster_refcount (bs, offset >> s->cluster_bits,1 ,1 ); }else { offset =alloc_clusters (bs, s->cluster_size); cluster_offset = s->free_byte_offset & ~(s->cluster_size -1 );if ((cluster_offset + s->cluster_size) == offset) {/* we are lucky: contiguous data */ offset = s->free_byte_offset; -update_cluster_refcount (bs, offset >> s->cluster_bits,1 ); +update_cluster_refcount (bs, offset >> s->cluster_bits,1 ,1 ); s->free_byte_offset += size; }else { s->free_byte_offset = offset;@@ -2389,46 +2390,77 @@ static int grow_refcount_table(BlockDriv/* XXX: cache several refcount block clusters ? */ static int update_cluster_refcount (BlockDriverState *bs,int64_t cluster_index, +int nb_clusters,int addend) {BDRVQcowState *s = bs->opaque;int64_t refcount_block_offset; -int ret, refcount_table_index, block_index, refcount; +int ret, refcount_table_index, refcount_table_last_index, block_index, refcount; +int nb_block_index; +int refcount_cache_size; - refcount_table_index = cluster_index >> (s->cluster_bits -REFCOUNT_SHIFT ); -if (refcount_table_index >= s->refcount_table_size) { +if (nb_clusters ==0 ) +return 0 ; + + refcount_table_last_index = (cluster_index + nb_clusters -1 ) >> + (s->cluster_bits -REFCOUNT_SHIFT ); + +/* grow the refcount table if needed */ + +if (refcount_table_last_index >= s->refcount_table_size) {if (addend <0 )return -EINVAL ; - ret =grow_refcount_table (bs, refcount_table_index +1 ); + ret =grow_refcount_table (bs, refcount_table_last_index +1 );if (ret <0 )return ret; } - refcount_block_offset = s->refcount_table[refcount_table_index]; -if (!refcount_block_offset) { -if (addend <0 ) -return -EINVAL ; - refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); -if (refcount_block_offset <0 ) -return -EINVAL ; - }else { -if (load_refcount_block (bs, refcount_block_offset) <0 )
Quoted 8 lines
block-qcow2.c static int grow_refcount_table(BlockDriv · line 2421
Quoted 5 lines
block-qcow2.c +5
-0+ +while (nb_clusters) { + refcount_table_index = cluster_index >> + (s->cluster_bits -REFCOUNT_SHIFT ); + refcount_block_offset = s->refcount_table[refcount_table_index];I guess (comment? ;-)) this outer loop is for handling requests which span multiple refcount blocks? If so, am I missing something or is
Yes,
Quoted 2 lines
refcount_block_offsetthe same for each iteration becausecluster_indexnever changes?
You're right, there is a missing "cluster_index++" at the end of this loop.
Quoted 35 lines
Quoted 6 lines
block-qcow2.c +6
-0+ +if (!refcount_block_offset) { +if (addend <0 ) +return -EINVAL ; + refcount_block_offset =alloc_refcount_block (bs, refcount_table_index); +if (refcount_block_offset <0 )block-qcow2.c static int grow_refcount_table(BlockDriv · line 2437
Quoted 10 lines
block-qcow2.c +10
-0+return -EINVAL ; + }else { +if (load_refcount_block (bs, refcount_block_offset) <0 ) +return -EIO ; + } + +/* we can update the count and save it */ + + refcount_cache_size =1 << (s->cluster_bits -REFCOUNT_SHIFT ); + nb_block_index =0 ;I have to admit that
nb_block_indexis a name where I can't image what the variable might be for. Seems to be a counter for the changed refcounts in the current refcount block, andblock_indexseems to be the first index to be touched in this block. What aboutfirst_indexandcur_refcountor something like that? (I don't like these suggestions too much, either. Maybe someone has better names.)block-qcow2.c static int grow_refcount_table(BlockDriv · line 2447
Quoted 10 lines
block-qcow2.c +10
-0+ block_index = cluster_index & (refcount_cache_size -1 ); + refcount =0 ; +while (nb_clusters && + block_index + nb_block_index < refcount_cache_size) { + + refcount =be16_to_cpu ( + s->refcount_block_cache[block_index + nb_block_index]); + refcount += addend; +if (refcount <0 || refcount >0xffff ) +return -EINVAL ;
Quoted 3 lines
Here we possibly abort in the middle of the operation. If it fails somewhere in the fifth refcount block, what happens with the four already updated blocks?
Yes, you're right. I think we have at least to save refcount we have updated.
Quoted 17 lines
Quoted 6 lines
block-qcow2.c +5
-0+if (refcount ==0 && + cluster_index + nb_block_index < s->free_cluster_index) { + s->free_cluster_index = cluster_index + nb_block_index; + } + s->refcount_block_cache[block_index + nb_block_index] =block-qcow2.c static int grow_refcount_table(BlockDriv · line 2462
Quoted 10 lines
block-qcow2.c +9
-0+cpu_to_be16 (refcount); + nb_block_index++; + nb_clusters--; + } +if (bdrv_pwrite (s->hd, + refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), + s->refcount_block_cache + block_index, + nb_block_index *sizeof (uint16_t )) != + nb_block_index *sizeof (uint16_t ))return -EIO ;
Quoted 1 line
Same here.
I think we are not worst than the original behavior. I don't know how to manage this better.
Quoted 26 lines
Quoted 26 lines
block-qcow2.c +1
-16} -/* we can update the count and save it */ - block_index = cluster_index & - ((1 << (s->cluster_bits -REFCOUNT_SHIFT )) -1 ); - refcount =be16_to_cpu (s->refcount_block_cache[block_index]); - refcount += addend; -if (refcount <0 || refcount >0xffff ) -return -EINVAL ; -if (refcount ==0 && cluster_index < s->free_cluster_index) { - s->free_cluster_index = cluster_index; - } - s->refcount_block_cache[block_index] =cpu_to_be16 (refcount); -if (bdrv_pwrite (s->hd, - refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), - &s->refcount_block_cache[block_index],2 ) !=2 ) -return -EIO ;return refcount; }@@ -2437,7 +2469,7 @@ static void update_refcount(BlockDriverS int addend) {BDRVQcowState *s = bs->opaque; -int64_t start, last, cluster_offset; +int64_t start, last;
Quoted 16 lines
Quoted 4 lines
block-qcow2.c +0
-0#ifdef DEBUG_ALLOC2 printf("update_refcount: offset=%lld size=%lld addend=%d\n",@@ -2445,12 +2477,9 @@ static void update_refcount(BlockDriverS#endif block-qcow2.c static void update_refcount(BlockDriverS · line 2481
Quoted 10 lines
block-qcow2.c +2
-6if (length <=0 )return ; - start = offset & ~(s->cluster_size -1 ); - last = (offset + length -1 ) & ~(s->cluster_size -1 ); -for (cluster_offset = start; cluster_offset <= last; - cluster_offset += s->cluster_size) { -update_cluster_refcount (bs, cluster_offset >> s->cluster_bits, addend); - } + start = offset >> s->cluster_bits; + last = (offset + length) >> s->cluster_bits;Off by one for length %
cluster_size== 0?
Explain, please (but notice the "<=" in the "for (...)").
Quoted 8 lines
block-qcow2.c static void update_refcount(BlockDriverS · line 2485
Quoted 5 lines
block-qcow2.c +1
-0+update_cluster_refcount (bs, start, last - start +1 , addend); }#ifdef DEBUG_ALLOC Kevin
Signature · 3 lines
------------------ [email protected] ------------------ "Tout ce qui est impossible reste à accomplir" Jules Verne "Things are only impossible until they're not" Jean-Luc Picard
Re: [Qemu-devel] [PATCH 2/4] qcow2: Allow update_cluster_refcount() to update several clusters refcount.
Quoted 32 lines · Laurent Vivier
Quoted 10 lines
block-qcow2.c static int grow_refcount_table(BlockDriv · line 2447
Quoted 10 lines
block-qcow2.c +10
-0+ block_index = cluster_index & (refcount_cache_size -1 ); + refcount =0 ; +while (nb_clusters && + block_index + nb_block_index < refcount_cache_size) { + + refcount =be16_to_cpu ( + s->refcount_block_cache[block_index + nb_block_index]); + refcount += addend; +if (refcount <0 || refcount >0xffff ) +return -EINVAL ;Quoted 3 lines
Here we possibly abort in the middle of the operation. If it fails somewhere in the fifth refcount block, what happens with the four already updated blocks?
Yes, you're right. I think we have at least to save refcount we have updated.
Quoted 15 lines
Quoted 15 lines
block-qcow2.c +14
-0+if (refcount ==0 && + cluster_index + nb_block_index < s->free_cluster_index) { + s->free_cluster_index = cluster_index + nb_block_index; + } + s->refcount_block_cache[block_index + nb_block_index] = +cpu_to_be16 (refcount); + nb_block_index++; + nb_clusters--; + } +if (bdrv_pwrite (s->hd, + refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), + s->refcount_block_cache + block_index, + nb_block_index *sizeof (uint16_t )) != + nb_block_index *sizeof (uint16_t ))return -EIO ;
Quoted 4 lines
Quoted 1 line
Same here.
I think we are not worst than the original behavior. I don't know how to manage this better.
Right, this hasn't been optimal before either. I noticed later that update_refcount didn't even check the return value. So the difference is that you abort while the old implementation tries the next block.
I'm unsure which behaviour is the better one. Actually, both don't feel quite right. But being not worse in this aspect than the original still makes this patch an improvement, so...
Quoted 42 lines
Quoted 26 lines
Quoted 26 lines
}block-qcow2.c +1
-16-/* we can update the count and save it */ - block_index = cluster_index & - ((1 << (s->cluster_bits -REFCOUNT_SHIFT )) -1 ); - refcount =be16_to_cpu (s->refcount_block_cache[block_index]); - refcount += addend; -if (refcount <0 || refcount >0xffff ) -return -EINVAL ; -if (refcount ==0 && cluster_index < s->free_cluster_index) { - s->free_cluster_index = cluster_index; - } - s->refcount_block_cache[block_index] =cpu_to_be16 (refcount); -if (bdrv_pwrite (s->hd, - refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), - &s->refcount_block_cache[block_index],2 ) !=2 ) -return -EIO ;return refcount; }@@ -2437,7 +2469,7 @@ static void update_refcount(BlockDriverS int addend) {BDRVQcowState *s = bs->opaque; -int64_t start, last, cluster_offset; +int64_t start, last;Quoted 15 lines
Quoted 4 lines
block-qcow2.c +0
-0#ifdef DEBUG_ALLOC2 printf("update_refcount: offset=%lld size=%lld addend=%d\n",@@ -2445,12 +2477,9 @@ static void update_refcount(BlockDriverS#endif block-qcow2.c static void update_refcount(BlockDriverS · line 2481
Quoted 10 lines
block-qcow2.c +2
-6if (length <=0 )return ; - start = offset & ~(s->cluster_size -1 ); - last = (offset + length -1 ) & ~(s->cluster_size -1 ); -for (cluster_offset = start; cluster_offset <= last; - cluster_offset += s->cluster_size) { -update_cluster_refcount (bs, cluster_offset >> s->cluster_bits, addend); - } + start = offset >> s->cluster_bits; + last = (offset + length) >> s->cluster_bits;Off by one for length %
cluster_size== 0?
Quoted 1 line
Explain, please (but notice the "<=" in the "for (...)").
I didn't even look at the old code yesterday, but the difference is that you dropped the - 1 for last.
Let's do it by example: Assume cluster_size = 0x1000 for simplicity and we get offset = 0x1000 and length = 0x2000 (i.e. the last affected byte would be 0x2fff). The old code correctly produces start = 0x1000 and last = 0x2000 whereas you get start = 1 and last = 3.
Kevin
Re: [Qemu-devel] [PATCH 2/4] qcow2: Allow update_cluster_refcount() to update several clusters refcount.
Quoted 1 line · Kevin Wolf
Laurent Vivier schrieb:
[...]
Quoted 26 lines
Quoted 24 lines
Quoted 8 lines
Quoted 8 lines
block-qcow2.c +1
-1@@ -2437,7 +2469,7 @@ static void update_refcount(BlockDriverS int addend) {BDRVQcowState *s = bs->opaque; -int64_t start, last, cluster_offset; +int64_t start, last;Quoted 15 lines
Quoted 4 lines
block-qcow2.c +0
-0#ifdef DEBUG_ALLOC2 printf("update_refcount: offset=%lld size=%lld addend=%d\n",@@ -2445,12 +2477,9 @@ static void update_refcount(BlockDriverS#endif block-qcow2.c static void update_refcount(BlockDriverS · line 2481
Quoted 10 lines
block-qcow2.c +2
-6if (length <=0 )return ; - start = offset & ~(s->cluster_size -1 ); - last = (offset + length -1 ) & ~(s->cluster_size -1 ); -for (cluster_offset = start; cluster_offset <= last; - cluster_offset += s->cluster_size) { -update_cluster_refcount (bs, cluster_offset >> s->cluster_bits, addend); - } + start = offset >> s->cluster_bits; + last = (offset + length) >> s->cluster_bits;Off by one for length %
cluster_size== 0?Quoted 1 line
Explain, please (but notice the "<=" in the "for (...)").
Quoted 7 lines
I didn't even look at the old code yesterday, but the difference is that you dropped the - 1 for last.
Let's do it by example: Assume
cluster_size=0x1000for simplicity and we get offset =0x1000and length =0x2000(i.e. the last affected byte would be0x2fff). The old code correctly produces start =0x1000and last =0x2000whereas you get start = 1 and last = 3.
Yes, "last" must be "(offset + length - 1) >> s->cluster_bits"
Thank you,
Laurent
Signature · 3 lines
------------------ [email protected] ------------------ "Tout ce qui est impossible reste à accomplir" Jules Verne "Things are only impossible until they're not" Jean-Luc Picard
PATCH 3/4 qcow2: Align I/O access to l2 table and refcount block.
In reply to a message not in the archivepièce jointe document texte brut (0003-Align-I-O-access-to-l2-table-and-refcount-block.patch) When used with O_DIRECT, to align I/O access (memory and offset) avoids to have to do a read before doing a write at the block-raw level (to align access at this level).
--- block-qcow2.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-)
block-qcow2.c +29 -11
@@ -255,7 +255,8 @@ static int qcow_open(BlockDriverState *bbe64_to_cpus (&s->l1_table[i]); }/* alloc L2 cache */ - s->l2_cache =qemu_malloc (s->l2_size *L2_CACHE_SIZE *sizeof (uint64_t )); + s->l2_cache =qemu_memalign (512 , + s->l2_size *L2_CACHE_SIZE *sizeof (uint64_t ));if (!s->l2_cache) goto fail; s->cluster_cache =qemu_malloc (s->cluster_size);
@@ -865,7 +866,7 @@ static uint64_t alloc_cluster_offset(Blo int *num) {BDRVQcowState *s = bs->opaque; -int l2_index, ret; +int l2_index, ret, aligned_index, aligned_size;uint64_t l2_offset, *l2_table, cluster_offset;int nb_available, nb_clusters, i, j;uint64_t start_sect, current;
@@ -988,11 +989,18 @@ static uint64_t alloc_cluster_offset(Blo (i << s->cluster_bits)) | QCOW_OFLAG_COPIED); +/* l2 table is part of l2_cache +* size of l2_cache is s->l2_size * L2_CACHE_SIZE * sizeof(uint64_t) +* and s->l2_bits is (s->cluster_bits - 3) = 12 - 3 = 9; +* and s->l2_size = 1 << s->l2_bits, so l2_cache is aligned on 512 boundary. +*/ + aligned_index = l2_index & ~63 ; + aligned_size = (l2_index - aligned_index + nb_clusters +63 ) & ~63ULL ; + aligned_size *=sizeof (uint64_t );if (bdrv_pwrite (s->hd, - l2_offset + l2_index *sizeof (uint64_t ), - l2_table + l2_index, - nb_clusters *sizeof (uint64_t )) != - nb_clusters *sizeof (uint64_t )) + l2_offset + aligned_index *sizeof (uint64_t ), + l2_table + aligned_index, + aligned_size) != aligned_size)return 0 ; out:
@@ -2137,7 +2145,7 @@ static int refcount_init(BlockDriverStatBDRVQcowState *s = bs->opaque;int ret, refcount_table_size2, i; - s->refcount_block_cache =qemu_malloc (s->cluster_size); + s->refcount_block_cache =qemu_memalign (512 , s->cluster_size);if (!s->refcount_block_cache) goto fail; refcount_table_size2 = s->refcount_table_size *sizeof (uint64_t );
@@ -2398,6 +2406,7 @@ static int update_cluster_refcount(Blockint ret, refcount_table_index, refcount_table_last_index, block_index, refcount;int nb_block_index;int refcount_cache_size; +int aligned_index, aligned_size;if (nb_clusters ==0 )return 0 ;
@@ -2454,11 +2463,20 @@ static int update_cluster_refcount(Block nb_block_index++; nb_clusters--; } +/* +* size of refcount_block_cache is s->cluster_size (4096) +* so we can align access on a 512 boundary +*/ + aligned_index = block_index & ~((512 >>REFCOUNT_SHIFT ) -1 ); + aligned_size = (block_index - aligned_index + nb_block_index + + ((512 >>REFCOUNT_SHIFT ) -1 )) & + ~((512 >>REFCOUNT_SHIFT ) -1 ); + aligned_size *=sizeof (uint16_t );if (bdrv_pwrite (s->hd, - refcount_block_offset + (block_index <<REFCOUNT_SHIFT ), - s->refcount_block_cache + block_index, - nb_block_index *sizeof (uint16_t )) != - nb_block_index *sizeof (uint16_t )) + refcount_block_offset + + (aligned_index <<REFCOUNT_SHIFT ), + s->refcount_block_cache + aligned_index, + aligned_size) != aligned_size)return -EIO ; }return refcount;
PATCH 4/4 qcow2: detect if no disk cache
In reply to a message not in the archivepièce jointe document texte brut (0004-qcow2-detect-if-no-disk-cache.patch)
To reduce the number of bytes read and written, disable the forced I/O alignment when the file is not open with O_DIRECT.
--- block-qcow2.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-)
block-qcow2.c +18 -6
@@ -147,6 +147,7 @@ typedef struct BDRVQcowState {int snapshots_size;int nb_snapshots;QCowSnapshot *snapshots; +int disk_nocache; } BDRVQcowState;static int decompress_cluster (BDRVQcowState *s,uint64_t cluster_offset);
@@ -193,6 +194,7 @@ static int qcow_open(BlockDriverState *b ret =bdrv_file_open (&s->hd, filename, flags);if (ret <0 )return ret; + s->disk_nocache = (flags &BDRV_O_NOCACHE ) !=0 ;if (bdrv_pread (s->hd,0 , &header,sizeof (header)) !=sizeof (header)) goto fail;be32_to_cpus (&header.magic);
@@ -994,8 +996,13 @@ static uint64_t alloc_cluster_offset(Blo* and s->l2_bits is (s->cluster_bits - 3) = 12 - 3 = 9; * and s->l2_size = 1 << s->l2_bits, so l2_cache is aligned on 512 boundary. */ - aligned_index = l2_index & ~63 ; - aligned_size = (l2_index - aligned_index + nb_clusters +63 ) & ~63ULL ; +if (s->disk_nocache) { + aligned_index = l2_index & ~63 ; + aligned_size = (l2_index - aligned_index + nb_clusters +63 ) & ~63ULL ; + }else { + aligned_index = l2_index; + aligned_size = nb_clusters; + } aligned_size *=sizeof (uint64_t ); if (bdrv_pwrite(s->hd, l2_offset + aligned_index * sizeof(uint64_t),
@@ -2467,10 +2474,15 @@ static int update_cluster_refcount(Block* size of refcount_block_cache is s->cluster_size (4096) * so we can align access on a 512 boundary */ - aligned_index = block_index & ~((512 >>REFCOUNT_SHIFT ) -1 ); - aligned_size = (block_index - aligned_index + nb_block_index + - ((512 >>REFCOUNT_SHIFT ) -1 )) & - ~((512 >>REFCOUNT_SHIFT ) -1 ); +if (s->disk_nocache) { + aligned_index = block_index & ~((512 >>REFCOUNT_SHIFT ) -1 ); + aligned_size = (block_index - aligned_index + nb_block_index + + ((512 >>REFCOUNT_SHIFT ) -1 )) & + ~((512 >>REFCOUNT_SHIFT ) -1 ); + }else { + aligned_index = block_index; + aligned_size = nb_block_index; + } aligned_size *=sizeof (uint16_t ); if (bdrv_pwrite(s->hd, refcount_block_offset +