On 2013/4/18 16:54, Stefan Hajnoczi wrote:
On Wed, Apr 10, 2013 at 04:11:51PM +0800, Dong Xu Wang wrote:This patch does two things: 1. Rename and move qcow2-cache to block-cache. 2. Add a type enum to parameterize BLKDEBUG_EVENT() for L2, refcount, and bitmaps. It's hard to review #2 since all lines are changed in the diff by #1. In the future, please split patches that move code into two. The first patch should simply move and rename - it should not include code changes. The second patch should contain code changes (i.e. adding the cache type enum and parameterizing BLKDEBUG_EVENT()). BTW, when splitting the patch into two for easy reviewing it also becomes questionable whether to reassign authorship since 90+% of the code is unchanged and simply moved.
Okay.
+ if (c->table_type == BLOCK_TABLE_REF) { + BLKDBG_EVENT(bs->file, BLKDBG_REFBLOCK_UPDATE_PART); + } else if (c->table_type == BLOCK_TABLE_L2) { + BLKDBG_EVENT(bs->file, BLKDBG_L2_UPDATE); + } else if (c->table_type == BLOCK_TABLE_BITMAP) { + BLKDBG_EVENT(bs->file, BLKDBG_COW_WRITE);BLKDBG_COW_WRITE is for writing out sectors from the backing file. It is not for updating the allocation bitmap.
Okay.
Please pick an appropriate constant or define your own if none exists.+ if (read_from_disk) { + if (c->table_type == BLOCK_TABLE_L2) { + BLKDBG_EVENT(bs->file, BLKDBG_L2_LOAD); + } else if (c->table_type == BLOCK_TABLE_BITMAP) { + BLKDBG_EVENT(bs->file, BLKDBG_COW_READ);Same here.
