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

2018-04-13 Thread Kevin Wolf
Am 13.04.2018 um 13:05 hat Paolo Bonzini geschrieben: > On 13/04/2018 10:01, Kevin Wolf wrote: > >> Or bs->quiescent, for the sake of bikeshedding. > > Yes, that sounds better. > > > > The only problem with the proposal as I made it is that it's wrong. We > > can't keep bs->quiescent until

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

2018-04-13 Thread Paolo Bonzini
On 13/04/2018 10:01, Kevin Wolf wrote: >> Or bs->quiescent, for the sake of bikeshedding. > Yes, that sounds better. > > The only problem with the proposal as I made it is that it's wrong. We > can't keep bs->quiescent until bdrv_do_drained_end() because the caller > can issue new requests and

Re: [Qemu-block] [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

Re: [Qemu-block] [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

Re: [Qemu-block] [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

Re: [Qemu-block] [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

Re: [Qemu-block] [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

Re: [Qemu-block] [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-block] [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

Re: [Qemu-block] [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

Re: [Qemu-block] [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

Re: [Qemu-block] [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-block] [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-block] [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) { > +