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

[Qemu-devel] [PATCH 0/4] 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 a way to print
out graph of block nodes with their permissions. Here is this way.

Instead of a lot of words I'll reply to this cover letter with example
graph, generated in patch 04.

Vladimir Sementsov-Ogievskiy (4):
  block: improve blk_root_get_parent_desc
  qapi: add x-query-block-nodes-relations
  scripts/qemu: add render_block_graph method for QEMUMachine
  not-for-commit: example of new command usage for debugging

 qapi/block-core.json   | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h  |  1 +
 block.c                | 57 ++++++++++++++++++++++++++++++++++++++++
 block/block-backend.c  |  2 +-
 blockdev.c             |  5 ++++
 scripts/qemu.py        | 53 +++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/222 |  1 +
 7 files changed, 189 insertions(+), 1 deletion(-)

-- 
2.11.1

[Qemu-devel] [PATCH 1/4] block: improve blk_root_get_parent_desc

[Vladimir Sementsov-Ogievskiy]

Make blk_root_get_parent_desc return different descriptions for
different blk's in worst case.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/block-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index fa120630be..e5707a0f8c 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -141,7 +141,7 @@ static char *blk_root_get_parent_desc(BdrvChild *child)
     } else {
         /* TODO Callback into the BB owner for something more detailed */
         g_free(dev_id);
-        return g_strdup("a block device");
+        return g_strdup_printf("a block device %p", blk);
     }
 }
 
-- 
2.11.1

[Qemu-devel] [PATCH 2/4] qapi: add x-query-block-nodes-relations

[Vladimir Sementsov-Ogievskiy]

Add new command, returning list of block nodes graph edges.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qapi/block-core.json  | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/block/block.h |  1 +
 block.c               | 57 +++++++++++++++++++++++++++++++++++++++++
 blockdev.c            |  5 ++++
 4 files changed, 134 insertions(+)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4c7a37afdc..ad7b62c49b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1630,6 +1630,77 @@
 { 'command': 'query-named-block-nodes', 'returns': [ 'BlockDeviceInfo' ] }
 
 ##
+# @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' ] }
+
+##
+# @BlockRelationInfo:
+#
+# Information about relation between block node and its parent.
+#
+# @parent: node name or some other parent name/description, if parent is not a
+#          block node.
+#
+# @parent-is-bds: parent is block node.
+#
+# @child: node name
+#
+# @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 this parent.
+#
+# Since: 3.1
+##
+{ 'struct': 'BlockRelationInfo',
+  'data': { 'parent': 'str', 'parent-is-bds': 'bool', 'child': 'str',
+            'name': 'str', 'perm': [ 'BlockPermission' ],
+            'shared-perm': [ 'BlockPermission' ] } }
+
+##
+# @x-query-block-nodes-relations:
+#
+# Get the block relations list.
+#
+# Returns: the list of BlockRelationInfo.
+#
+# Since: 3.1
+##
+{ 'command': 'x-query-block-nodes-relations', 'returns': [ 'BlockRelationInfo' ] }
+
+##
 # @drive-mirror:
 #
 # Start mirroring a block device's writes to a new destination. target
diff --git a/include/block/block.h b/include/block/block.h
index 4e0871aaf9..04136634c2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -448,6 +448,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);
+BlockRelationInfoList *bdrv_block_relations_list(Error **errp);
 BlockDriverState *bdrv_lookup_bs(const char *device,
                                  const char *node_name,
                                  Error **errp);
diff --git a/block.c b/block.c
index 6161dbe3eb..776fc714af 100644
--- a/block.c
+++ b/block.c
@@ -4003,6 +4003,63 @@ BlockDeviceInfoList *bdrv_named_nodes_list(Error **errp)
     return list;
 }
 
+BlockRelationInfoList *bdrv_block_relations_list(Error **errp)
+{
+    BlockRelationInfoList *list = NULL, *entry;
+    BlockDriverState *bs;
+    struct {
+        unsigned int flag;
+        BlockPermission num;
+    } 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 }
+    }, *p;
+
+    QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
+        BdrvChild *child;
+
+        QLIST_FOREACH(child, &bs->parents, next_parent) {
+            entry = g_new0(BlockRelationInfoList, 1);
+            BlockRelationInfo *info = g_new0(BlockRelationInfo, 1);
+
+            info->parent_is_bds = child->role->parent_is_bds;
+            info->parent = child->role->parent_is_bds ?
+                               g_strdup(bdrv_get_node_name(child->opaque)) :
+                               bdrv_child_user_desc(child);
+            info->child = g_strdup(bs->node_name);
+            assert(bs == child->bs);
+            info->name = g_strdup(child->name);
+
+            for (p = permissions; p->flag; p++) {
+                BlockPermissionList *en;
+
+                if (p->flag & child->perm) {
+                    en = g_new(BlockPermissionList, 1);
+                    en->value = p->num;
+                    en->next = info->perm;
+                    info->perm = en;
+                }
+                if (p->flag & child->shared_perm) {
+                    en = g_new(BlockPermissionList, 1);
+                    en->value = p->num;
+                    en->next = info->shared_perm;
+                    info->shared_perm = en;
+                }
+            }
+
+            entry->value = info;
+            entry->next = list;
+            list = entry;
+        }
+    }
+
+    return list;
+}
+
 BlockDriverState *bdrv_lookup_bs(const char *device,
                                  const char *node_name,
                                  Error **errp)
diff --git a/blockdev.c b/blockdev.c
index 72f5347df5..74bdc2e93e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3485,6 +3485,11 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
     return bdrv_named_nodes_list(errp);
 }
 
+BlockRelationInfoList *qmp_x_query_block_nodes_relations(Error **errp)
+{
+    return bdrv_block_relations_list(errp);
+}
+
 BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
                              Error **errp)
 {
-- 
2.11.1

[Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine

[Vladimir Sementsov-Ogievskiy]

Render block nodes graph with help of graphviz

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 scripts/qemu.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/scripts/qemu.py b/scripts/qemu.py
index f099ce7278..cff562c713 100644
--- a/scripts/qemu.py
+++ b/scripts/qemu.py
@@ -460,3 +460,56 @@ class QEMUMachine(object):
                                                  socket.SOCK_STREAM)
             self._console_socket.connect(self._console_address)
         return self._console_socket
+
+    def render_block_graph(self, filename):
+        '''
+        Render graph in text (dot) representation into "filename" and graphical
+        representation into pdf file "filename".pdf
+        '''
+
+        try:
+            from graphviz import Digraph
+        except ImportError:
+            print "Can't import graphviz. Please run 'pip install graphviz'"
+            return
+
+        nodes = self.qmp('query-named-block-nodes')['return']
+        edges = self.qmp('x-query-block-nodes-relations')['return']
+        node_names = []
+
+        graph = Digraph(comment='Block Nodes Graph')
+        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')
+
+        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
+
+        for n in nodes:
+            node_names.append(n['node-name'])
+            label = n['node-name'] + ' [' + n['drv'] + ']'
+            if n['drv'] == 'file':
+                label = '<%s<br/>%s>' % (label, os.path.basename(n['file']))
+            graph.node(n['node-name'], label)
+
+        for r in edges:
+            if r['parent'] not in node_names:
+                graph.node(r['parent'], shape='box')
+
+            label = '%s\l%s\l%s\l' % (r['name'], perm(r['perm']),
+                                      perm(r['shared-perm']))
+            graph.edge(r['parent'], r['child'], label=label)
+
+        graph.render(filename)
-- 
2.11.1

[Qemu-devel] [PATCH 4/4] 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/222 b/tests/qemu-iotests/222
index 0ead56d574..6e2d96cd72 100644
--- a/tests/qemu-iotests/222
+++ b/tests/qemu-iotests/222
@@ -137,6 +137,7 @@ with iotests.FilePath('base.img') as base_img_path, \
     log('--- Cleanup ---')
     log('')
 
+    vm.render_block_graph('/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.11.1

Re: [Qemu-devel] [PATCH 0/4] block nodes graph visualization

[Vladimir Sementsov-Ogievskiy]

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

Here is an image

16.08.2018 20:20, Vladimir Sementsov-Ogievskiy wrote:
> 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 a way to print
> out graph of block nodes with their permissions. Here is this way.
>
> Instead of a lot of words I'll reply to this cover letter with example
> graph, generated in patch 04.
>
> Vladimir Sementsov-Ogievskiy (4):
>    block: improve blk_root_get_parent_desc
>    qapi: add x-query-block-nodes-relations
>    scripts/qemu: add render_block_graph method for QEMUMachine
>    not-for-commit: example of new command usage for debugging
>
>   qapi/block-core.json   | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   include/block/block.h  |  1 +
>   block.c                | 57 ++++++++++++++++++++++++++++++++++++++++
>   block/block-backend.c  |  2 +-
>   blockdev.c             |  5 ++++
>   scripts/qemu.py        | 53 +++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/222 |  1 +
>   7 files changed, 189 insertions(+), 1 deletion(-)
>


-- 
Best regards,
Vladimir


[-- Attachment #2: out --]
[-- Type: image/png, Size: 149351 bytes --]

Re: [Qemu-devel] [PATCH 1/4] block: improve blk_root_get_parent_desc

[Vladimir Sementsov-Ogievskiy]

Hmm, now I think, that instead of this, it is better to use pointer as 
parent id for
nod-bds parents, to be sure they will not intersect. And add special 
field to
qapi block relation info "parent-description" for such parents.

Also I'm afraid that this patch may break iotests..

16.08.2018 20:20, Vladimir Sementsov-Ogievskiy wrote:
> Make blk_root_get_parent_desc return different descriptions for
> different blk's in worst case.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/block-backend.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index fa120630be..e5707a0f8c 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -141,7 +141,7 @@ static char *blk_root_get_parent_desc(BdrvChild *child)
>       } else {
>           /* TODO Callback into the BB owner for something more detailed */
>           g_free(dev_id);
> -        return g_strdup("a block device");
> +        return g_strdup_printf("a block device %p", blk);
>       }
>   }
>   


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine

[Eduardo Habkost]

On Thu, Aug 16, 2018 at 08:20:26PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Render block nodes graph with help of graphviz
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Thanks for your patch.  Comments below:

> ---
>  scripts/qemu.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/scripts/qemu.py b/scripts/qemu.py
> index f099ce7278..cff562c713 100644
> --- a/scripts/qemu.py
> +++ b/scripts/qemu.py
> @@ -460,3 +460,56 @@ class QEMUMachine(object):
>                                                   socket.SOCK_STREAM)
>              self._console_socket.connect(self._console_address)
>          return self._console_socket
> +
> +    def render_block_graph(self, filename):
> +        '''
> +        Render graph in text (dot) representation into "filename" and graphical
> +        representation into pdf file "filename".pdf
> +        '''
> +
> +        try:
> +            from graphviz import Digraph

I'm not convinced that this belongs to qemu.py, if most code
using the qemu.py module will never use it.  Do you see any
problem in moving this code to a scripts/render_block_graph.py
script?

> +        except ImportError:
> +            print "Can't import graphviz. Please run 'pip install graphviz'"

If you really want to make this part of qemu.py, I suggest
raising an appropriate exception instead of printing to stdout
directly.

(But just moving this code to a separate script would probably be
simpler)

Also, keep in mind that this module needs to work with both
Python 3 and Python 2, so you need to use print("message ...")
with parenthesis.


> +            return
> +
> +        nodes = self.qmp('query-named-block-nodes')['return']
> +        edges = self.qmp('x-query-block-nodes-relations')['return']

I suggest you use .command() instead of .qmp().  It handles
['return'] and ['error'] for you.


> +        node_names = []
> +
> +        graph = Digraph(comment='Block Nodes Graph')
> +        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')
> +
> +        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
> +
> +        for n in nodes:
> +            node_names.append(n['node-name'])
> +            label = n['node-name'] + ' [' + n['drv'] + ']'
> +            if n['drv'] == 'file':
> +                label = '<%s<br/>%s>' % (label, os.path.basename(n['file']))
> +            graph.node(n['node-name'], label)
> +
> +        for r in edges:
> +            if r['parent'] not in node_names:
> +                graph.node(r['parent'], shape='box')
> +
> +            label = '%s\l%s\l%s\l' % (r['name'], perm(r['perm']),
> +                                      perm(r['shared-perm']))
> +            graph.edge(r['parent'], r['child'], label=label)
> +
> +        graph.render(filename)

The rest of the code looks OK to me.

-- 
Eduardo

Re: [Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine

[Vladimir Sementsov-Ogievskiy]

17.08.2018 04:54, Eduardo Habkost wrote:
> On Thu, Aug 16, 2018 at 08:20:26PM +0300, Vladimir Sementsov-Ogievskiy wrote:
>> Render block nodes graph with help of graphviz
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Thanks for your patch.  Comments below:
>
>> ---
>>   scripts/qemu.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 53 insertions(+)
>>
>> diff --git a/scripts/qemu.py b/scripts/qemu.py
>> index f099ce7278..cff562c713 100644
>> --- a/scripts/qemu.py
>> +++ b/scripts/qemu.py
>> @@ -460,3 +460,56 @@ class QEMUMachine(object):
>>                                                    socket.SOCK_STREAM)
>>               self._console_socket.connect(self._console_address)
>>           return self._console_socket
>> +
>> +    def render_block_graph(self, filename):
>> +        '''
>> +        Render graph in text (dot) representation into "filename" and graphical
>> +        representation into pdf file "filename".pdf
>> +        '''
>> +
>> +        try:
>> +            from graphviz import Digraph
> I'm not convinced that this belongs to qemu.py, if most code
> using the qemu.py module will never use it.  Do you see any
> problem in moving this code to a scripts/render_block_graph.py
> script?

Not a problem, I can do it.. Just one thought:
Isn't it better to keep all function doing something with one vm as 
methods, not separate functions?

>
>> +        except ImportError:
>> +            print "Can't import graphviz. Please run 'pip install graphviz'"
> If you really want to make this part of qemu.py, I suggest
> raising an appropriate exception instead of printing to stdout
> directly.

Ok

> (But just moving this code to a separate script would probably be
> simpler)
>
> Also, keep in mind that this module needs to work with both
> Python 3 and Python 2, so you need to use print("message ...")
> with parenthesis.
>
>
>> +            return
>> +
>> +        nodes = self.qmp('query-named-block-nodes')['return']
>> +        edges = self.qmp('x-query-block-nodes-relations')['return']
> I suggest you use .command() instead of .qmp().  It handles
> ['return'] and ['error'] for you.

Cool, thanks

>
>
>> +        node_names = []
>> +
>> +        graph = Digraph(comment='Block Nodes Graph')
>> +        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')
>> +
>> +        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
>> +
>> +        for n in nodes:
>> +            node_names.append(n['node-name'])
>> +            label = n['node-name'] + ' [' + n['drv'] + ']'
>> +            if n['drv'] == 'file':
>> +                label = '<%s<br/>%s>' % (label, os.path.basename(n['file']))
>> +            graph.node(n['node-name'], label)
>> +
>> +        for r in edges:
>> +            if r['parent'] not in node_names:
>> +                graph.node(r['parent'], shape='box')
>> +
>> +            label = '%s\l%s\l%s\l' % (r['name'], perm(r['perm']),
>> +                                      perm(r['shared-perm']))
>> +            graph.edge(r['parent'], r['child'], label=label)
>> +
>> +        graph.render(filename)
> The rest of the code looks OK to me.
>


-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine

[Eduardo Habkost]

On Fri, Aug 17, 2018 at 01:08:42PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 17.08.2018 04:54, Eduardo Habkost wrote:
> > On Thu, Aug 16, 2018 at 08:20:26PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> > > Render block nodes graph with help of graphviz
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > Thanks for your patch.  Comments below:
> > 
> > > ---
> > >   scripts/qemu.py | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 53 insertions(+)
> > > 
> > > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > > index f099ce7278..cff562c713 100644
> > > --- a/scripts/qemu.py
> > > +++ b/scripts/qemu.py
> > > @@ -460,3 +460,56 @@ class QEMUMachine(object):
> > >                                                    socket.SOCK_STREAM)
> > >               self._console_socket.connect(self._console_address)
> > >           return self._console_socket
> > > +
> > > +    def render_block_graph(self, filename):
> > > +        '''
> > > +        Render graph in text (dot) representation into "filename" and graphical
> > > +        representation into pdf file "filename".pdf
> > > +        '''
> > > +
> > > +        try:
> > > +            from graphviz import Digraph
> > I'm not convinced that this belongs to qemu.py, if most code
> > using the qemu.py module will never use it.  Do you see any
> > problem in moving this code to a scripts/render_block_graph.py
> > script?
> 
> Not a problem, I can do it.. Just one thought:
> Isn't it better to keep all function doing something with one vm as methods,
> not separate functions?

I don't think so.  We don't need to create a new QEMUMachine
method every time we write a function that takes a QEMUMachine as
argument in our scripts.

There are cases when we're forced to create a method: when the
new code is tightly coupled to the QEMUMachine code and need
access to its private attributes.

There are cases where we probably want to place the code in
qemu.py (as a method): when we expect many users of qemu.py to
call it.

But in other cases, I don't see any problem with another
script/module having a regular function like:

  def my_special_purpose_function(vm, ...):
     ...


-- 
Eduardo