On 2017-11-10 18:25, Max Reitz wrote: > On one hand, it is a good idea for bdrv_next() to return a strong > reference because ideally nearly every pointer should be refcounted. > This fixes intermittent failure of iotest 194. > > On the other, it is absolutely necessary for bdrv_next() itself to keep > a strong reference to both the BB (in its first phase) and the BDS (at > least in the second phase) because when called the next time, it will > dereference those objects to get a link to the next one. Therefore, it > needs these objects to stay around until then. Just storing the pointer > to the next in the iterator is not really viable because that pointer > might become invalid as well. > > Both arguments taken together means we should probably just invoke > bdrv_ref() and blk_ref() in bdrv_next(). This means we have to assert > that bdrv_next() is always called from the main loop, but that was > probably necessary already before this patch and judging from the > callers, it also looks to actually be the case. > > Keeping these strong references means however that callers need to give > them up if they decide to abort the iteration early. They can do so > through the new bdrv_next_cleanup() function. > > Suggested-by: Kevin Wolf <kw...@redhat.com> > Signed-off-by: Max Reitz <mre...@redhat.com> > --- > v2: Instead of keeping the strong reference in bdrv_drain_all_*() only, > have them for all callers of bdrv_next() [Fam, Kevin] > (Completely different patch now, so no git-backport-diff included > here) > --- > include/block/block.h | 1 + > block.c | 3 +++ > block/block-backend.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- > block/snapshot.c | 6 ++++++ > migration/block.c | 1 + > 5 files changed, 57 insertions(+), 2 deletions(-)
Due to one supporter and otherwise lack of resistance: Applied to my block branch (https://github.com/XanClic/qemu/commits/block). Max
signature.asc
Description: OpenPGP digital signature