Patchwork [V19,6/8] add debug event for add-cow

login
register
mail settings
Submitter Robert Wang
Date May 30, 2013, 10 a.m.
Message ID <1369908025-9556-7-git-send-email-wdongxu@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/247519/
State New
Headers show

Comments

Robert Wang - May 30, 2013, 10 a.m.
From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>

Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Dongxu Wang <wdongxu@linux.vnet.ibm.com>
---
 block/blkdebug.c      | 3 +++
 block/block-cache.c   | 4 ++--
 include/block/block.h | 3 +++
 3 files changed, 8 insertions(+), 2 deletions(-)
Stefan Hajnoczi - June 5, 2013, 1:35 p.m.
On Thu, May 30, 2013 at 06:00:23PM +0800, Dongxu Wang wrote:
> diff --git a/block/block-cache.c b/block/block-cache.c
> index f5d75d1..454269c 100644
> --- a/block/block-cache.c
> +++ b/block/block-cache.c
> @@ -125,7 +125,7 @@ static int block_cache_entry_flush(BlockDriverState *bs, BlockCache *c, int i)
>      } 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_EVENT(bs->file, BLKDBG_ADDCOW_WRITE);
>      }
>  
>      ret = bdrv_pwrite(bs->file, c->entries[i].offset,
> @@ -260,7 +260,7 @@ static int block_cache_do_get(BlockDriverState *bs, BlockCache *c,
>          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);
> +            BLKDBG_EVENT(bs->file, BLKDBG_ADDCOW_READ);
>          }
>  
>          ret = bdrv_pread(bs->file, offset, c->entries[i].table,

These patches are in the wrong order.  The previous patch should not use
the incorrect BLKDBG_COW_READ/BLKDBG_COW_WRITE values.

Put the blkdebug.c and block.h changes from this patch before the
previous patch so block-cache.c can use
BLKDBG_ADDCOW_READ/BLKDBG_ADDCOW_WRITE from the start.
Kevin Wolf - June 5, 2013, 1:46 p.m.
Am 30.05.2013 um 12:00 hat Dongxu Wang geschrieben:
> From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> 
> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> Signed-off-by: Dongxu Wang <wdongxu@linux.vnet.ibm.com>

One of these should surely be enough?

> ---
>  block/blkdebug.c      | 3 +++
>  block/block-cache.c   | 4 ++--
>  include/block/block.h | 3 +++
>  3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 71f99e4..2bd6a53 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -182,6 +182,9 @@ static const char *event_names[BLKDBG_EVENT_MAX] = {
>      [BLKDBG_CLUSTER_ALLOC]                  = "cluster_alloc",
>      [BLKDBG_CLUSTER_ALLOC_BYTES]            = "cluster_alloc_bytes",
>      [BLKDBG_CLUSTER_FREE]                   = "cluster_free",
> +
> +    [BLKDBG_ADDCOW_READ]                    = "add_cow_read",
> +    [BLKDBG_ADDCOW_WRITE]                   = "add_cow_write",
>  };
>  
>  static int get_event_by_name(const char *name, BlkDebugEvent *event)
> diff --git a/block/block-cache.c b/block/block-cache.c
> index f5d75d1..454269c 100644
> --- a/block/block-cache.c
> +++ b/block/block-cache.c
> @@ -125,7 +125,7 @@ static int block_cache_entry_flush(BlockDriverState *bs, BlockCache *c, int i)
>      } 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_EVENT(bs->file, BLKDBG_ADDCOW_WRITE);
>      }
>  
>      ret = bdrv_pwrite(bs->file, c->entries[i].offset,
> @@ -260,7 +260,7 @@ static int block_cache_do_get(BlockDriverState *bs, BlockCache *c,
>          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);
> +            BLKDBG_EVENT(bs->file, BLKDBG_ADDCOW_READ);
>          }

Doesn't this break qcow2 test cases that don't get the
BLKDBG_COW_READ/WRITE events any more now?

I guess you need to extend the cache creation function to also take
function pointers for the blkdebug events that should be generated in
the various places. Maybe it's enough data now that it would be worth
having a BlockCache struct somewhere (e.g. embedded in BDRVQcow2State)
and change the cache creation function to an init function that only
takes a pointer to this struct.

Kevin
Stefan Hajnoczi - June 6, 2013, 7:30 a.m.
On Wed, Jun 05, 2013 at 03:46:43PM +0200, Kevin Wolf wrote:
> Am 30.05.2013 um 12:00 hat Dongxu Wang geschrieben:
> > From: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> > 
> > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
> > Signed-off-by: Dongxu Wang <wdongxu@linux.vnet.ibm.com>
> 
> One of these should surely be enough?
> 
> > ---
> >  block/blkdebug.c      | 3 +++
> >  block/block-cache.c   | 4 ++--
> >  include/block/block.h | 3 +++
> >  3 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/blkdebug.c b/block/blkdebug.c
> > index 71f99e4..2bd6a53 100644
> > --- a/block/blkdebug.c
> > +++ b/block/blkdebug.c
> > @@ -182,6 +182,9 @@ static const char *event_names[BLKDBG_EVENT_MAX] = {
> >      [BLKDBG_CLUSTER_ALLOC]                  = "cluster_alloc",
> >      [BLKDBG_CLUSTER_ALLOC_BYTES]            = "cluster_alloc_bytes",
> >      [BLKDBG_CLUSTER_FREE]                   = "cluster_free",
> > +
> > +    [BLKDBG_ADDCOW_READ]                    = "add_cow_read",
> > +    [BLKDBG_ADDCOW_WRITE]                   = "add_cow_write",
> >  };
> >  
> >  static int get_event_by_name(const char *name, BlkDebugEvent *event)
> > diff --git a/block/block-cache.c b/block/block-cache.c
> > index f5d75d1..454269c 100644
> > --- a/block/block-cache.c
> > +++ b/block/block-cache.c
> > @@ -125,7 +125,7 @@ static int block_cache_entry_flush(BlockDriverState *bs, BlockCache *c, int i)
> >      } 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_EVENT(bs->file, BLKDBG_ADDCOW_WRITE);
> >      }
> >  
> >      ret = bdrv_pwrite(bs->file, c->entries[i].offset,
> > @@ -260,7 +260,7 @@ static int block_cache_do_get(BlockDriverState *bs, BlockCache *c,
> >          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);
> > +            BLKDBG_EVENT(bs->file, BLKDBG_ADDCOW_READ);
> >          }
> 
> Doesn't this break qcow2 test cases that don't get the
> BLKDBG_COW_READ/WRITE events any more now?

This event is only raised for BLKDBG_TABLE_BITMAP, which is new for
add-cow only.  qcow2 only uses L2 and refcount table types, not bitmap.

Patch

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 71f99e4..2bd6a53 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -182,6 +182,9 @@  static const char *event_names[BLKDBG_EVENT_MAX] = {
     [BLKDBG_CLUSTER_ALLOC]                  = "cluster_alloc",
     [BLKDBG_CLUSTER_ALLOC_BYTES]            = "cluster_alloc_bytes",
     [BLKDBG_CLUSTER_FREE]                   = "cluster_free",
+
+    [BLKDBG_ADDCOW_READ]                    = "add_cow_read",
+    [BLKDBG_ADDCOW_WRITE]                   = "add_cow_write",
 };
 
 static int get_event_by_name(const char *name, BlkDebugEvent *event)
diff --git a/block/block-cache.c b/block/block-cache.c
index f5d75d1..454269c 100644
--- a/block/block-cache.c
+++ b/block/block-cache.c
@@ -125,7 +125,7 @@  static int block_cache_entry_flush(BlockDriverState *bs, BlockCache *c, int i)
     } 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_EVENT(bs->file, BLKDBG_ADDCOW_WRITE);
     }
 
     ret = bdrv_pwrite(bs->file, c->entries[i].offset,
@@ -260,7 +260,7 @@  static int block_cache_do_get(BlockDriverState *bs, BlockCache *c,
         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);
+            BLKDBG_EVENT(bs->file, BLKDBG_ADDCOW_READ);
         }
 
         ret = bdrv_pread(bs->file, offset, c->entries[i].table,
diff --git a/include/block/block.h b/include/block/block.h
index 2989da6..3573e3e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -451,6 +451,9 @@  typedef enum {
     BLKDBG_CLUSTER_ALLOC_BYTES,
     BLKDBG_CLUSTER_FREE,
 
+    BLKDBG_ADDCOW_READ,
+    BLKDBG_ADDCOW_WRITE,
+
     BLKDBG_EVENT_MAX,
 } BlkDebugEvent;