[Top][All Lists]

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

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

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH v2 1/3] qapi: add x-query-block-graph
Date: Mon, 20 Aug 2018 18:35:44 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

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.)


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

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