diff mbox

[2/3] block/qapi: Factor out bdrv_query_bds_stats()

Message ID 1456518142-9849-3-git-send-email-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kevin Wolf Feb. 26, 2016, 8:22 p.m. UTC
The new functions handles the data that is taken from the
BlockDriverState.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qapi.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

Comments

Max Reitz March 2, 2016, 4:47 p.m. UTC | #1
On 26.02.2016 21:22, Kevin Wolf wrote:
> The new functions handles the data that is taken from the
> BlockDriverState.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qapi.c | 31 ++++++++++++++++++++-----------
>  1 file changed, 20 insertions(+), 11 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Now that I see s->stats->wr_highest_offset in this function, thus
piquing my interest in s->stats... How about another patch which makes
bdrv_query_blk_stats() take the BlockDeviceStats pointer instead of
BlockStats?

Max
Kevin Wolf March 2, 2016, 4:52 p.m. UTC | #2
Am 02.03.2016 um 17:47 hat Max Reitz geschrieben:
> On 26.02.2016 21:22, Kevin Wolf wrote:
> > The new functions handles the data that is taken from the
> > BlockDriverState.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  block/qapi.c | 31 ++++++++++++++++++++-----------
> >  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 
> Now that I see s->stats->wr_highest_offset in this function, thus
> piquing my interest in s->stats... How about another patch which makes
> bdrv_query_blk_stats() take the BlockDeviceStats pointer instead of
> BlockStats?

Hm, doesn't work any more after patch 3. Which doesn't necessarily mean
that it's a bad thought, maybe s->device can be set in the caller
instead.

Kevin
Max Reitz March 2, 2016, 4:55 p.m. UTC | #3
On 02.03.2016 17:52, Kevin Wolf wrote:
> Am 02.03.2016 um 17:47 hat Max Reitz geschrieben:
>> On 26.02.2016 21:22, Kevin Wolf wrote:
>>> The new functions handles the data that is taken from the
>>> BlockDriverState.
>>>
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  block/qapi.c | 31 ++++++++++++++++++++-----------
>>>  1 file changed, 20 insertions(+), 11 deletions(-)
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>> Now that I see s->stats->wr_highest_offset in this function, thus
>> piquing my interest in s->stats... How about another patch which makes
>> bdrv_query_blk_stats() take the BlockDeviceStats pointer instead of
>> BlockStats?
> 
> Hm, doesn't work any more after patch 3.

I just noticed. :-)

>                                          Which doesn't necessarily mean
> that it's a bad thought, maybe s->device can be set in the caller
> instead.

Seems reasonable to me.

Max
diff mbox

Patch

diff --git a/block/qapi.c b/block/qapi.c
index c04f1d8..31ae879 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -355,6 +355,9 @@  static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info,
     qapi_free_BlockInfo(info);
 }
 
+static BlockStats *bdrv_query_stats(const BlockDriverState *bs,
+                                    bool query_backing);
+
 static void bdrv_query_blk_stats(BlockStats *s, BlockBackend *blk)
 {
     BlockAcctStats *stats = blk_get_stats(blk);
@@ -422,13 +425,9 @@  static void bdrv_query_blk_stats(BlockStats *s, BlockBackend *blk)
     }
 }
 
-static BlockStats *bdrv_query_stats(const BlockDriverState *bs,
-                                    bool query_backing)
+static void bdrv_query_bds_stats(BlockStats *s, const BlockDriverState *bs,
+                                 bool query_backing)
 {
-    BlockStats *s;
-
-    s = g_malloc0(sizeof(*s));
-
     if (bdrv_get_device_name(bs)[0]) {
         s->has_device = true;
         s->device = g_strdup(bdrv_get_device_name(bs));
@@ -439,11 +438,6 @@  static BlockStats *bdrv_query_stats(const BlockDriverState *bs,
         s->node_name = g_strdup(bdrv_get_node_name(bs));
     }
 
-    s->stats = g_malloc0(sizeof(*s->stats));
-    if (bs->blk) {
-        bdrv_query_blk_stats(s, bs->blk);
-    }
-
     s->stats->wr_highest_offset = bs->wr_highest_offset;
 
     if (bs->file) {
@@ -456,6 +450,21 @@  static BlockStats *bdrv_query_stats(const BlockDriverState *bs,
         s->backing = bdrv_query_stats(bs->backing->bs, query_backing);
     }
 
+}
+
+static BlockStats *bdrv_query_stats(const BlockDriverState *bs,
+                                    bool query_backing)
+{
+    BlockStats *s;
+
+    s = g_malloc0(sizeof(*s));
+    s->stats = g_malloc0(sizeof(*s->stats));
+
+    if (bs->blk) {
+        bdrv_query_blk_stats(s, bs->blk);
+    }
+    bdrv_query_bds_stats(s, bs, query_backing);
+
     return s;
 }