20.08.2018 20:13, Max Reitz wrote:
On 2018-08-20 19:04, Vladimir Sementsov-Ogievskiy wrote:
20.08.2018 19:35, Max Reitz wrote:
On 2018-08-20 17:13, Vladimir Sementsov-Ogievskiy wrote:
20.08.2018 16:44, Max Reitz wrote:
On 2018-08-20 12:20, Vladimir Sementsov-Ogievskiy wrote:
My goal is to get graphviz representation of block graph with
parents for debugging. And I'm absolutely ok to do it with
Then we shouldn't care about enum for role type now. So, it the
for you with x-debug- prefix?
Actually, no, because I'm not sure whether using points for the IDs
good idea. That may defeat ASLR, and that would be a problem
Good point, agree.
So I'd prefer using e.g. a hash map to map pointers to freshly
IDs (just incrementing integers).
(By the way, that would also improve the speed of checking whether a
certain node needs to be added to the @nodes list still.)
Ah, no, it's OK. Well, kind of. It's OK in the sense that it is
In any case, I'll take a further look at the patch later, but
thing that I've just seen is that using the opaque pointers to
a parent is something that doesn't look like it's guaranteed to
Hm, isn't it a bug? Can you point to an example?
when set. I didn't notice that you only use it for the non-node
hm, no, I use opaque for all. So, it can be zero? In what case? In this
case, I cant get its parent?
As far as I can see, you only use it for BLOCK_GRAPH_NODE_TYPE_OTHER,
and you only do it when the parent is indeed not a BDS (because BDS are
always children of some sort, so they'll be automatically accounted for
Ah, but you set edge->parent = (uint64_t)child->opaque. I see, because
you assume it's always going to point to the BDS.
... or to some non-bds parent
Yeah, generally, you can't get the parent. That's the whole point. I
suppose in practice your code works, but that's not how it's supposed to
be. You generally cannot go from child to parent, only through the
interface defined through BdrvChildRole (which uses the opaque pointer).
As I said, a safe way to do this would be to enumerate all possible
block graph roots; those being (as far as I know) all BlockBackends and
all monitor-owned BDSs (see bdrv_close_all(), which handles exactly
those two cases). Then you can walk down from the roots through the
hmm, what about block jobs here?
Still, it probably would be better to just use the BdrvChild
that should be unique as well, and it is obviously non-NULL.
(BdrvChild.opaque may be NULL, even though it isn't in practice.)
but BdrvChild corresponds to edge in a graph, not to the node. I need
identificators for nodes..
But an edge identifies two nodes. All edges have at least one BDS on
Since you can identify all BDS through the BDS itself, if you have a
non-BDS node, you can use the edge to identify it (because the other end
of the edge is going to be a BDS which you can identify by itself).
But non-bds node may have several children, therefore several
BdrvChild's with different pointers.. And only opque point should be
the same for them.
Or, well, if you take my suggestion and walk down the trees starting at
the roots, you'll see that the only non-BDS nodes in the block graph are
BlockBackends. And you can just use their addresses to identify them
(just internally, of course; externally you'll still want to generate a
As I understand, I'll not see block jobs in this way to be non-bds