Re: [PATCH v2 14/15] block: Don't poll in bdrv_replace_child_noperm()

[Paolo Bonzini]

On 12/9/22 17:53, Paolo Bonzini wrote:
> On 11/18/22 18:41, Kevin Wolf wrote:
> > In order to make sure that bdrv_replace_child_noperm() doesn't have to
> > poll any more, get rid of the bdrv_parent_drained_begin_single() call.
> >
> > This is possible now because we can require that the parent is already
> > drained through the child in question when the function is called and we
> > don't call the parent drain callbacks more than once.
> >
> > The additional drain calls needed in callers cause the test case to run
> > its code in the drain handler too early (bdrv_attach_child() drains
> > now), so modify it to only enable the code after the test setup has
> > completed.
> >
> > Signed-off-by: Kevin Wolf<kwolf@redhat.com>
>
> I hate to bear bad news, but this breaks the Windows builds on github 
> (msys-32bit, msys-64bit) with an obscure but 100% reproducible
>
> 51/88 qemu:unit / test-bdrv-drain ERROR           1.30s   (exit status 
> 3221225477 or signal 3221225349 SIGinvalid)
>
> The exit status is 0xC0000005 aka a Windows SIGSEGV.  With some luck it 
> could be reproducible with Wine (but no gdb).

I am testing dropping this patch and squashing

diff --git a/block.c b/block.c
index 1a2a8d9de907..bb9c85222168 100644
--- a/block.c
+++ b/block.c
@@ -2870,7 +2870,9 @@ static void bdrv_replace_child_noperm(BdrvChild 
*child,
      */
     new_bs_quiesce_counter = (new_bs ? new_bs->quiesce_counter : 0);
     if (new_bs_quiesce_counter && !child->quiesced_parent) {
-        bdrv_parent_drained_begin_single(child, true);
+        bdrv_parent_drained_begin_single(child);
+        AIO_WAIT_WHILE(bdrv_child_get_parent_aio_context(child),
+                       bdrv_parent_drained_poll_single(c));
     }

     if (old_bs) {

in patch 15/15, which should at least allow me to keep a lot of the 
cleanups I had on top of this series.

Paolo