On 4/30/19 6:08 PM, John Snow wrote: > > > 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.]
Max has already tried to tackle that problem: https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg01713.html > >> 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. Max's series also tries to improve how we visit nodes when determining which bitmaps to find. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature