[Qemu-devel] [PATCH v4 0/3] block nodes graph visualization

[Qemu-devel] [PATCH v4 0/3] block nodes graph visualization

[Vladimir Sementsov-Ogievskiy]

Hi all!

On the way of backup schemes development (and in general any complicated
developments in Qemu block layer) it would be good to have an ability to print
out graph of block nodes with their permissions.

v4:
01: - improve documentation and graph node type names
    - parentheses in macro
    - s/hash/graph_nodes
    - add dbg_ prefix to graph functions
    - free leaked gr in dbg_graph_finalize()
    - use uintptr_t and "!= 0" for hash keys, add comment
      about reserved 0 key
    - add const specifier for permissions array and iterator,
      add static on permissions array size
    - fall back to blk_get_attached_dev_id() for block-backend
      nodes if blk_name() returns NULL 
02: - add a-b by Eduardo and r-b by Max
    - change graph node type names (rebase on 01 change)

v3: again, major rework, after long discussion with Max:
   - start creating graph looping through blk's and block jobs, don't use opaque
   - don't export pointers, generate ids instead
   (graphical representation didn't significantly changed, you can look at the
    picture, attached to v2 cover-letter)

v2: major rework: Identifying non-bds nodes by their description was a bad idea,
descriptions are not guaranteed to be different for different nodes. So, the only
way is use pointer to identify them. And to be unique, let's use pointers to
identify all the nodes in the graph. As additional benefit, we have pointers for
each node, which is good for debugging (imagine a gdb session).

Vladimir Sementsov-Ogievskiy (3):
  qapi: add x-debug-query-block-graph
  scripts: add render_block_graph function for QEMUMachine
  not-for-commit: example of new command usage for debugging

 qapi/block-core.json           | 108 ++++++++++++++++++++++++
 include/block/block.h          |   1 +
 include/sysemu/block-backend.h |   2 +
 block.c                        | 146 +++++++++++++++++++++++++++++++++
 block/block-backend.c          |   5 ++
 blockdev.c                     |   5 ++
 scripts/render_block_graph.py  | 120 +++++++++++++++++++++++++++
 tests/qemu-iotests/222         |   2 +
 8 files changed, 389 insertions(+)
 create mode 100755 scripts/render_block_graph.py

-- 
2.18.0

[Qemu-devel] [PATCH v4 1/3] qapi: add x-debug-query-block-graph

[Vladimir Sementsov-Ogievskiy]

Add a new command, returning block nodes (and their users) graph.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/block-core.json           | 108 ++++++++++++++++++++++++
 include/block/block.h          |   1 +
 include/sysemu/block-backend.h |   2 +
 block.c                        | 146 +++++++++++++++++++++++++++++++++
 block/block-backend.c          |   5 ++
 blockdev.c                     |   5 ++
 6 files changed, 267 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index cfb37f8c1d..1580bb95b8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1657,6 +1657,114 @@
 ##
 { 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ] }
 
+##
+# @BlockGraphNodeType:
+#
+# @block-backend: corresponds to BlockBackend
+#
+# @block-job: corresonds to BlockJob
+#
+# @block-driver: corresponds to BlockDriverState
+#
+# Since: 3.1
+##
+{ 'enum': 'BlockGraphNodeType',
+  'data': [ 'block-backend', 'block-job', 'block-driver' ] }
+
+##
+# @BlockGraphNode:
+#
+# @id: Block graph node identifier. This @id is generated only for
+#      x-debug-query-block-graph and does not relate to any other identifiers in
+#      Qemu.
+#
+# @type: Type of graph node. Can be one of block-backend, block-job or
+#        block-driver-state.
+#
+# @name: Human readable name of the node. Corresponds to node-name for
+#        block-driver-state nodes; is not guaranteed to be unique in the whole
+#        graph (with block-jobs and block-backends).
+#
+# Since: 3.1
+##
+{ 'struct': 'BlockGraphNode',
+  'data': { 'id': 'uint64', 'type': 'BlockGraphNodeType', 'name': 'str' } }
+
+##
+# @BlockPermission:
+#
+# Enum of base block permissions.
+#
+# @consistent-read: A user that has the "permission" of consistent reads is
+#                   guaranteed that their view of the contents of the block
+#                   device is complete and self-consistent, representing the
+#                   contents of a disk at a specific point.
+#                   For most block devices (including their backing files) this
+#                   is true, but the property cannot be maintained in a few
+#                   situations like for intermediate nodes of a commit block
+#                   job.
+#
+# @write: This permission is required to change the visible disk contents.
+#
+# @write-unchanged: This permission (which is weaker than BLK_PERM_WRITE) is
+#                   both enough and required for writes to the block node when
+#                   the caller promises that the visible disk content doesn't
+#                   change.
+#                   As the BLK_PERM_WRITE permission is strictly stronger,
+#                   either is sufficient to perform an unchanging write.
+#
+# @resize: This permission is required to change the size of a block node.
+#
+# @graph-mod: This permission is required to change the node that this
+#             BdrvChild points to.
+#
+# Since: 3.1
+##
+  { 'enum': 'BlockPermission',
+    'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize',
+              'graph-mod' ] }
+##
+# @BlockGraphEdge:
+#
+# Block Graph edge description for x-debug-query-block-graph.
+#
+# @parent: parent id
+#
+# @child: child id
+#
+# @name: name of the relation (examples are 'file' and 'backing')
+#
+# @perm: granted permissions for the parent operating on the child
+#
+# @shared-perm: permissions that can still be granted to other users of the
+#               child while it is still attached to this parent
+#
+# Since: 3.1
+##
+{ 'struct': 'BlockGraphEdge',
+  'data': { 'parent': 'uint64', 'child': 'uint64',
+            'name': 'str', 'perm': [ 'BlockPermission' ],
+            'shared-perm': [ 'BlockPermission' ] } }
+
+##
+# @BlockGraph:
+#
+# Block Graph - list of nodes and list of edges.
+#
+# Since: 3.1
+##
+{ 'struct': 'BlockGraph',
+  'data': { 'nodes': ['BlockGraphNode'], 'edges': ['BlockGraphEdge'] } }
+
+##
+# @x-debug-query-block-graph:
+#
+# Get the block graph.
+#
+# Since: 3.1
+##
+{ 'command': 'x-debug-query-block-graph', 'returns': 'BlockGraph' }
+
 ##
 # @drive-mirror:
 #
diff --git a/include/block/block.h b/include/block/block.h
index b189cf422e..31158cced0 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -445,6 +445,7 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag);
 const char *bdrv_get_format_name(BlockDriverState *bs);
 BlockDriverState *bdrv_find_node(const char *node_name);
 BlockDeviceInfoList *bdrv_named_nodes_list(Error **errp);
+BlockGraph *bdrv_get_block_graph(Error **errp);
 BlockDriverState *bdrv_lookup_bs(const char *device,
                                  const char *node_name,
                                  Error **errp);
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 830d873f24..32ca5c1b12 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -237,4 +237,6 @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
                                    int bytes, BdrvRequestFlags read_flags,
                                    BdrvRequestFlags write_flags);
 
+const BdrvChild *blk_root(BlockBackend *blk);
+
 #endif
diff --git a/block.c b/block.c
index 7710b399a3..90825f427f 100644
--- a/block.c
+++ b/block.c
@@ -4047,6 +4047,152 @@ BlockDeviceInfoList *bdrv_named_nodes_list(Error **errp)
     return list;
 }
 
+#define QAPI_LIST_ADD(list, element) do { \
+    typeof(list) _tmp = g_new(typeof(*(list)), 1); \
+    _tmp->value = (element); \
+    _tmp->next = (list); \
+    (list) = _tmp; \
+} while (0)
+
+typedef struct BlockGraphConstructor {
+    BlockGraph *graph;
+    GHashTable *graph_nodes;
+} BlockGraphConstructor;
+
+static BlockGraphConstructor *dbg_graph_new(void)
+{
+    BlockGraphConstructor *gr = g_new(BlockGraphConstructor, 1);
+
+    gr->graph = g_new0(BlockGraph, 1);
+    gr->graph_nodes = g_hash_table_new(NULL, NULL);
+
+    return gr;
+}
+
+static BlockGraph *dbg_graph_finalize(BlockGraphConstructor *gr)
+{
+    BlockGraph *graph = gr->graph;
+
+    g_hash_table_destroy(gr->graph_nodes);
+    g_free(gr);
+
+    return graph;
+}
+
+static uintptr_t dbg_graph_node_num(BlockGraphConstructor *gr, void *node)
+{
+    uintptr_t ret = (uintptr_t)g_hash_table_lookup(gr->graph_nodes, node);
+
+    if (ret != 0) {
+        return ret;
+    }
+
+    /* Start counting from 1, not 0, because 0 interferes with not-found (NULL)
+     * answer of g_hash_table_lookup.
+     */
+    ret = g_hash_table_size(gr->graph_nodes) + 1;
+    g_hash_table_insert(gr->graph_nodes, node, (void *)ret);
+
+    return ret;
+}
+
+static void dbg_graph_add_node(BlockGraphConstructor *gr, void *node,
+                               BlockGraphNodeType type, const char *name)
+{
+    BlockGraphNode *n;
+
+    n = g_new0(BlockGraphNode, 1);
+
+    n->id = dbg_graph_node_num(gr, node);
+    n->type = type;
+    n->name = g_strdup(name);
+
+    QAPI_LIST_ADD(gr->graph->nodes, n);
+}
+
+static void dbg_graph_add_edge(BlockGraphConstructor *gr, void *parent,
+                               const BdrvChild *child)
+{
+    typedef struct {
+        unsigned int flag;
+        BlockPermission num;
+    } PermissionMap;
+
+    static const PermissionMap permissions[] = {
+        { BLK_PERM_CONSISTENT_READ, BLOCK_PERMISSION_CONSISTENT_READ },
+        { BLK_PERM_WRITE,           BLOCK_PERMISSION_WRITE },
+        { BLK_PERM_WRITE_UNCHANGED, BLOCK_PERMISSION_WRITE_UNCHANGED },
+        { BLK_PERM_RESIZE,          BLOCK_PERMISSION_RESIZE },
+        { BLK_PERM_GRAPH_MOD,       BLOCK_PERMISSION_GRAPH_MOD },
+        { 0, 0 }
+    };
+    const PermissionMap *p;
+    BlockGraphEdge *edge;
+
+    QEMU_BUILD_BUG_ON(1UL << (ARRAY_SIZE(permissions) - 1) != BLK_PERM_ALL + 1);
+
+    edge = g_new0(BlockGraphEdge, 1);
+
+    edge->parent = dbg_graph_node_num(gr, parent);
+    edge->child = dbg_graph_node_num(gr, child->bs);
+    edge->name = g_strdup(child->name);
+
+    for (p = permissions; p->flag; p++) {
+        if (p->flag & child->perm) {
+            QAPI_LIST_ADD(edge->perm, p->num);
+        }
+        if (p->flag & child->shared_perm) {
+            QAPI_LIST_ADD(edge->shared_perm, p->num);
+        }
+    }
+
+    QAPI_LIST_ADD(gr->graph->edges, edge);
+}
+
+
+BlockGraph *bdrv_get_block_graph(Error **errp)
+{
+    BlockBackend *blk;
+    BlockJob *job;
+    BlockDriverState *bs;
+    BdrvChild *child;
+    BlockGraphConstructor *gr = dbg_graph_new();
+
+    for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
+        char *allocated_name = NULL;
+        const char *name = blk_name(blk);
+
+        if (!name) {
+            name = allocated_name = blk_get_attached_dev_id(blk);
+        }
+        dbg_graph_add_node(gr, blk, BLOCK_GRAPH_NODE_TYPE_BLOCK_BACKEND, name);
+        g_free(allocated_name);
+        if (blk_root(blk)) {
+            dbg_graph_add_edge(gr, blk, blk_root(blk));
+        }
+    }
+
+    for (job = block_job_next(NULL); job; job = block_job_next(job)) {
+        GSList *el;
+
+        dbg_graph_add_node(gr, job, BLOCK_GRAPH_NODE_TYPE_BLOCK_JOB,
+                           job->job.id);
+        for (el = job->nodes; el; el = el->next) {
+            dbg_graph_add_edge(gr, job, (BdrvChild *)el->data);
+        }
+    }
+
+    QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
+        dbg_graph_add_node(gr, bs, BLOCK_GRAPH_NODE_TYPE_BLOCK_DRIVER,
+                           bs->node_name);
+        QLIST_FOREACH(child, &bs->children, next) {
+            dbg_graph_add_edge(gr, bs, child);
+        }
+    }
+
+    return dbg_graph_finalize(gr);
+}
+
 BlockDriverState *bdrv_lookup_bs(const char *device,
                                  const char *node_name,
                                  Error **errp)
diff --git a/block/block-backend.c b/block/block-backend.c
index dc0cd57724..0146908229 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2248,3 +2248,8 @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
                               blk_out->root, off_out,
                               bytes, read_flags, write_flags);
 }
+
+const BdrvChild *blk_root(BlockBackend *blk)
+{
+    return blk->root;
+}
diff --git a/blockdev.c b/blockdev.c
index a8755bd908..925db6cf92 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3538,6 +3538,11 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
     return bdrv_named_nodes_list(errp);
 }
 
+BlockGraph *qmp_x_debug_query_block_graph(Error **errp)
+{
+    return bdrv_get_block_graph(errp);
+}
+
 BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
                              Error **errp)
 {
-- 
2.18.0

[Qemu-devel] [PATCH v4 2/3] scripts: add render_block_graph function for QEMUMachine

[Vladimir Sementsov-Ogievskiy]

Render block nodes graph with help of graphviz. This new function is
for debugging, so there is no sense to put it into qemu.py as a method
of QEMUMachine. Let's instead put it separately.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Acked-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 scripts/render_block_graph.py | 120 ++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100755 scripts/render_block_graph.py

diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py
new file mode 100755
index 0000000000..ed7e581b4f
--- /dev/null
+++ b/scripts/render_block_graph.py
@@ -0,0 +1,120 @@
+#!/usr/bin/env python
+#
+# Render Qemu Block Graph
+#
+# Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import sys
+import subprocess
+import json
+from graphviz import Digraph
+from qemu import MonitorResponseError
+
+
+def perm(arr):
+    s = 'w' if 'write' in arr else '_'
+    s += 'r' if 'consistent-read' in arr else '_'
+    s += 'u' if 'write-unchanged' in arr else '_'
+    s += 'g' if 'graph-mod' in arr else '_'
+    s += 's' if 'resize' in arr else '_'
+    return s
+
+
+def render_block_graph(qmp, filename, format='png'):
+    '''
+    Render graph in text (dot) representation into "@filename" and
+    representation in @format into "@filename.@format"
+    '''
+
+    bds_nodes = qmp.command('query-named-block-nodes')
+    bds_nodes = {n['node-name']: n for n in bds_nodes}
+
+    job_nodes = qmp.command('query-block-jobs')
+    job_nodes = {n['device']: n for n in job_nodes}
+
+    block_graph = qmp.command('x-debug-query-block-graph')
+
+    graph = Digraph(comment='Block Nodes Graph')
+    graph.format = format
+    graph.node('permission symbols:\l'
+               '  w - Write\l'
+               '  r - consistent-Read\l'
+               '  u - write - Unchanged\l'
+               '  g - Graph-mod\l'
+               '  s - reSize\l'
+               'edge label scheme:\l'
+               '  <child type>\l'
+               '  <perm>\l'
+               '  <shared_perm>\l', shape='none')
+
+    for n in block_graph['nodes']:
+        if n['type'] == 'block-driver':
+            info = bds_nodes[n['name']]
+            label = n['name'] + ' [' + info['drv'] + ']'
+            if info['drv'] == 'file':
+                label += '\n' + os.path.basename(info['file'])
+            shape = 'ellipse'
+        elif n['type'] == 'block-job':
+            info = job_nodes[n['name']]
+            label = info['type'] + ' job (' + n['name'] + ')'
+            shape = 'box'
+        else:
+            assert n['type'] == 'block-backend'
+            label = n['name'] if n['name'] else 'unnamed blk'
+            shape = 'box'
+
+        graph.node(str(n['id']), label, shape=shape)
+
+    for e in block_graph['edges']:
+        label = '%s\l%s\l%s\l' % (e['name'], perm(e['perm']),
+                                  perm(e['shared-perm']))
+        graph.edge(str(e['parent']), str(e['child']), label=label)
+
+    graph.render(filename)
+
+
+class LibvirtGuest():
+    def __init__(self, name):
+        self.name = name
+
+    def command(self, cmd):
+        # only supports qmp commands without parameters
+        m = {'execute': cmd}
+        ar = ['virsh', 'qemu-monitor-command', self.name, json.dumps(m)]
+
+        reply = json.loads(subprocess.check_output(ar))
+
+        if 'error' in reply:
+            raise MonitorResponseError(reply)
+
+        return reply['return']
+
+
+if __name__ == '__main__':
+    obj = sys.argv[1]
+    out = sys.argv[2]
+
+    if os.path.exists(obj):
+        # assume unix socket
+        qmp = QEMUMonitorProtocol(obj)
+        qmp.connect()
+    else:
+        # assume libvirt guest name
+        qmp = LibvirtGuest(obj)
+
+    render_block_graph(qmp, out)
-- 
2.18.0

[Qemu-devel] [PATCH v4 3/3] not-for-commit: example of new command usage for debugging

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/222 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
index 0ead56d574..91d88aa5c0 100644
--- a/tests/qemu-iotests/222
+++ b/tests/qemu-iotests/222
@@ -137,6 +137,8 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Cleanup ---')
     log('')
 
+    from render_block_graph import render_block_graph
+    render_block_graph(vm, '/tmp/out.dot')
     log(vm.qmp('block-job-cancel', device=src_node))
     log(vm.event_wait('BLOCK_JOB_CANCELLED'),
         filters=[iotests.filter_qmp_event])
-- 
2.18.0

Re: [Qemu-devel] [PATCH v4 1/3] qapi: add x-debug-query-block-graph

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1932 bytes --]

On 09.10.18 17:00, Vladimir Sementsov-Ogievskiy wrote:
> Add a new command, returning block nodes (and their users) graph.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  qapi/block-core.json           | 108 ++++++++++++++++++++++++
>  include/block/block.h          |   1 +
>  include/sysemu/block-backend.h |   2 +
>  block.c                        | 146 +++++++++++++++++++++++++++++++++
>  block/block-backend.c          |   5 ++
>  blockdev.c                     |   5 ++
>  6 files changed, 267 insertions(+)

[...]

> diff --git a/block.c b/block.c
> index 7710b399a3..90825f427f 100644
> --- a/block.c
> +++ b/block.c
> @@ -4047,6 +4047,152 @@ BlockDeviceInfoList *bdrv_named_nodes_list(Error **errp)

[...]

> +BlockGraph *bdrv_get_block_graph(Error **errp)
> +{
> +    BlockBackend *blk;
> +    BlockJob *job;
> +    BlockDriverState *bs;
> +    BdrvChild *child;
> +    BlockGraphConstructor *gr = dbg_graph_new();
> +
> +    for (blk = blk_all_next(NULL); blk; blk = blk_all_next(blk)) {
> +        char *allocated_name = NULL;
> +        const char *name = blk_name(blk);
> +
> +        if (!name) {

blk_name() never returns NULL, so you need to check for !*name here.

The rest looks OK, although I realized that maybe we want to use the
QAPI type name "BlockGraph" for a non-debug command in the future.  But
I think that as long as it's only this command providing that structure,
we should be free to rename it should the need arise.

(And introspection doesn't care about structure names, I think.)

Max

> +            name = allocated_name = blk_get_attached_dev_id(blk);
> +        }
> +        dbg_graph_add_node(gr, blk, BLOCK_GRAPH_NODE_TYPE_BLOCK_BACKEND, name);
> +        g_free(allocated_name);
> +        if (blk_root(blk)) {
> +            dbg_graph_add_edge(gr, blk, blk_root(blk));
> +        }
> +    }


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]