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

[Max Reitz]

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 all the
>>> parents for debugging. And I'm absolutely ok to do it with x-debug-.
>>> Then we shouldn't care about enum for role type now. So, it the patch ok
>>> for you with x-debug- prefix?
>> Actually, no, because I'm not sure whether using points for the IDs is a
>> good idea.  That may defeat ASLR, and that would be a problem even with
>> x-debug-.
> 
> Good point, agree.
> 
>>
>> So I'd prefer using e.g. a hash map to map pointers to freshly generated
>> 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.)

>> In any case, I'll take a further look at the patch later, but another
>> thing that I've just seen is that using the opaque pointers to identify
>> a parent is something that doesn't look like it's guaranteed to work.
> 
> Hm, isn't it a bug? Can you point to an example?

Ah, no, it's OK.  Well, kind of.  It's OK in the sense that it is unique
when set.  I didn't notice that you only use it for the non-node
parents, sorry.

Still, it probably would be better to just use the BdrvChild object, as
that should be unique as well, and it is obviously non-NULL.
(BdrvChild.opaque may be NULL, even though it isn't in practice.)

Max

signature.asc
Description: OpenPGP digital signature