[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong refere
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS |
Date: |
Fri, 10 Nov 2017 17:22:24 +0100 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
Am 10.11.2017 um 17:13 hat Max Reitz geschrieben:
> On 2017-11-10 17:05, Kevin Wolf wrote:
> > Am 10.11.2017 um 16:23 hat Max Reitz geschrieben:
> >> On 2017-11-10 14:32, Fam Zheng wrote:
> >>> On Fri, 11/10 14:17, Kevin Wolf wrote:
> >>>> Do you actually need to keep references to all BDSes in the whole list
> >>>> while using the iterator or would it be enough to just keep a reference
> >>>> to the current one?
> >>>
> >>> To fix the bug we now see I think keeping the current is enough, but I
> >>> think
> >>> implementing just like this patch is also good with some future-proofing:
> >>> we
> >>> cannot know what will be wedged into the nexted aio_poll()'s over time
> >>> (and yes,
> >>> we should really reduce the number of them.)
> >>
> >> I don't really want to think about whether it's safe to only keep a
> >> reference to the current BDS. I can't imagine any case where destroying
> >> one root BDS leads to destroying another, but I'd rather be safe and not
> >> have to think about it. (Unless there is an important reason to only
> >> keep a strong reference to the current one.)
> >
> > Why would it be a problem if another BDS from the list went away? If it
> > is one that was already processed, we don't care, and if it was in the
> > yet unprocessed part of the list, we'll just never return it.
>
> You mean from bdrv_next() in its current form? Well, I know that when I
> just put a bdrv_ref()/bdrv_unref() pair around the drain, I got a
> segfault in blk_all_next() in bdrv_next(). I can investigate more, but
> that's pretty much what I mean by "I don't really want to think about it".
No, I mean a bdrv_next() that is modified to bdrv_ref() only what
it->blk/it->bs point to currently instead of allocating a whole list.
Kevin
> So that's why I want bdrv_next() to copy all BDS into another list
> instead of iterating through them on the fly. And if we do that, a
> disappearing BDS of course is an issue because we don't notice until
> we're trying to iterate over it, at which point we have a use-after-free.
>
> Max
signature.asc
Description: PGP signature
- [Qemu-block] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Max Reitz, 2017/11/09
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Eric Blake, 2017/11/09
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Fam Zheng, 2017/11/09
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Kevin Wolf, 2017/11/10
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Fam Zheng, 2017/11/10
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Max Reitz, 2017/11/10
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Fam Zheng, 2017/11/10
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Kevin Wolf, 2017/11/10
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Max Reitz, 2017/11/10
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS,
Kevin Wolf <=
- Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Max Reitz, 2017/11/10
Re: [Qemu-block] [PATCH for-2.11] block: Keep strong reference when draining all BDS, Stefan Hajnoczi, 2017/11/10