qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-devel] [PATCH 3/4] scripts/qemu: add render_block_graph method for QEMUMachine
Date: Fri, 17 Aug 2018 13:08:42 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

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 <address@hidden>
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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]