Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-13 Thread Kevin Wolf
Am 12.04.2018 um 22:44 hat Paolo Bonzini geschrieben: > On 12/04/2018 16:25, Kevin Wolf wrote: > > This is already the order we have there. What is probably different from > > what you envision is that after the parents have concluded, we still > > check that they are still quiescent in every itera

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Paolo Bonzini
On 12/04/2018 16:25, Kevin Wolf wrote: > This is already the order we have there. What is probably different from > what you envision is that after the parents have concluded, we still > check that they are still quiescent in every iteration. Yes, and that's the quadratic part. > What we could do

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Kevin Wolf
Am 12.04.2018 um 15:42 hat Paolo Bonzini geschrieben: > On 12/04/2018 15:27, Kevin Wolf wrote: > > Not sure I follow. Let's look at an example. Say, we have a block job > > BlockBackend as the root (because that uses proper layering, unlike > > devices which use aio_disable_external()), connected t

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Paolo Bonzini
On 12/04/2018 15:27, Kevin Wolf wrote: > Not sure I follow. Let's look at an example. Say, we have a block job > BlockBackend as the root (because that uses proper layering, unlike > devices which use aio_disable_external()), connected to a qcow2 node > over file. > > 1. The block job issues a req

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Kevin Wolf
Am 12.04.2018 um 14:02 hat Paolo Bonzini geschrieben: > On 12/04/2018 13:53, Kevin Wolf wrote: > >> The problem I have is that there is a direction through which I/O flows > >> (parent-to-child), so why can't draining follow that natural direction. > >> Having to check for the parents' I/O, while d

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Paolo Bonzini
On 12/04/2018 13:53, Kevin Wolf wrote: >> The problem I have is that there is a direction through which I/O flows >> (parent-to-child), so why can't draining follow that natural direction. >> Having to check for the parents' I/O, while draining the child, seems >> wrong. Perhaps we can't help it,

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Kevin Wolf
Am 12.04.2018 um 13:30 hat Paolo Bonzini geschrieben: > On 12/04/2018 13:11, Kevin Wolf wrote: > >> Well, there is one gotcha: bdrv_ref protects against disappearance, but > >> bdrv_ref/bdrv_unref are not thread-safe. Am I missing something else? > > > > Apart from the above, if we do an extra bdr

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Paolo Bonzini
On 12/04/2018 13:11, Kevin Wolf wrote: >> Well, there is one gotcha: bdrv_ref protects against disappearance, but >> bdrv_ref/bdrv_unref are not thread-safe. Am I missing something else? > > Apart from the above, if we do an extra bdrv_ref/unref we'd also have > to keep track of all the nodes that

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Kevin Wolf
Am 12.04.2018 um 12:12 hat Paolo Bonzini geschrieben: > On 12/04/2018 11:51, Kevin Wolf wrote: > > Am 12.04.2018 um 10:37 hat Paolo Bonzini geschrieben: > >> On 11/04/2018 18:39, Kevin Wolf wrote: > >>> +bool bdrv_drain_poll(BlockDriverState *bs, bool top_level) > >>> { > >>> /* Execute pendi

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Paolo Bonzini
On 12/04/2018 11:51, Kevin Wolf wrote: > Am 12.04.2018 um 10:37 hat Paolo Bonzini geschrieben: >> On 11/04/2018 18:39, Kevin Wolf wrote: >>> +bool bdrv_drain_poll(BlockDriverState *bs, bool top_level) >>> { >>> /* Execute pending BHs first and check everything else only after the >>> BHs >>>

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Kevin Wolf
Am 12.04.2018 um 10:37 hat Paolo Bonzini geschrieben: > On 11/04/2018 18:39, Kevin Wolf wrote: > > +bool bdrv_drain_poll(BlockDriverState *bs, bool top_level) > > { > > /* Execute pending BHs first and check everything else only after the > > BHs > > * have executed. */ > > -while

Re: [Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-12 Thread Paolo Bonzini
On 11/04/2018 18:39, Kevin Wolf wrote: > +bool bdrv_drain_poll(BlockDriverState *bs, bool top_level) > { > /* Execute pending BHs first and check everything else only after the BHs > * have executed. */ > -while (aio_poll(bs->aio_context, false)); > +if (top_level) { > +

[Qemu-devel] [PATCH 07/19] block: Really pause block jobs on drain

2018-04-11 Thread Kevin Wolf
We already requested that block jobs be paused in .bdrv_drained_begin, but no guarantee was made that the job was actually inactive at the point where bdrv_drained_begin() returned. This introduces a new callback BdrvChildRole.bdrv_drained_poll() and uses it to make bdrv_drain_poll() consider bloc