[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH] migration/block-dirty-bitmap: Sile
From: |
John Snow |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH] migration/block-dirty-bitmap: Silence coverity CID 1390625 |
Date: |
Tue, 30 Apr 2019 19:08:24 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
On 11/20/18 10:15 AM, Vladimir Sementsov-Ogievskiy wrote:
> 16.11.2018 21:43, John Snow wrote:
>> Coverity warns that backing_bs() could give us a NULL pointer, which
>> we then use without checking that it isn't.
>>
>> In our loop condition, we check bs && bs->drv as a point of habit, but
>> by nature of the block graph, we cannot have null bs pointers here.
>>
>> This loop skips only implicit nodes, which always have children, so
>> this loop should never encounter a null value.
>
I let this drop again :)
> You mean, always have backing (not file for ex.)? Should we at least add a
> comment
> near "bool implicit;" that the node must have backing..
>
> Do we have filters, using 'file' child instead of backing, will we want to
> auto insert them, and therefore mark them with implicit=true?
>
I actually have no idea. I guess this is the sort of thing we actually
really want a dedicated kind of API for. "Find first non-filter" seems
like a common use case that we'd want.
[But maybe I'll avoid this problem.]
> And one more thing:
> So, it's looks like a wrong way to search for all block-nodes, instead of
> looping through backing chain to the first not-implicit bds, we must
> recursively explore the whole block graph, to find _all_ the bitmaps.
>
Looking at this again after not having done so for so long -- I guess
that bdrv_first/bdrv_next only iterate over *top level* BDSes and not
any children thereof. You're right, even the method here isn't quite
correct. We want to find ALL nodes, wherever they are.
query_named_block_nodes uses an implementation in block.c to accomplish
this because the API is not public.... or, it wasn't, but it looks like
we have bdrv_next_all_states now, and we could use this to just find ALL
of the bdrv nodes.
Ehm.... let me send something a little more RFC-caliber that should
address your concern (as well as Peter's) here.
--js
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-block] [Qemu-devel] [PATCH] migration/block-dirty-bitmap: Silence coverity CID 1390625,
John Snow <=