Re: [Qemu-block] [Qemu-devel] [RFC PATCH] qemu-io: add drain/undrain cmd

[Fam Zheng]

On Mon, 05/15 13:58, Peter Lieven wrote:
> Am 15.05.2017 um 13:53 schrieb Fam Zheng:
> > On Mon, 05/15 13:26, Peter Lieven wrote:
> > > Am 15.05.2017 um 12:50 schrieb Fam Zheng:
> > > > On Mon, 05/15 12:02, Peter Lieven wrote:
> > > > > Hi Block developers,
> > > > > 
> > > > > I would like to add a feature to Qemu to drain all traffic from a 
> > > > > block so that
> > > > > I can take external snaphosts without the risk to that in the middle 
> > > > > of a write
> > > > > operation. Its meant for cases where where QGA freeze/thaw is not 
> > > > > available.
> > > > > 
> > > > > For me its enough to have this through qemu-io, but Kevin asked me to 
> > > > > check
> > > > > if its not worth to have a stable API for it and present it via 
> > > > > QMP/HMP.
> > > > > 
> > > > > What are your thoughts?
> > > > For debugging purpose or a "hacky" usage where you know what you are 
> > > > doing, it
> > > > may be fine to have this. The only issue is it should be a separate 
> > > > flag, like
> > > > BlockJob.user_paused.
> > > How can I add, remove such a flag?
> > Like bs->user_drained. Set it in "drain" command, then increment
> > bs->quiesce_counter if toggled; vise versa.
> 
> Ah okay. You wouldn't use bdrv_drained_begin/end? Because in these functions
> the counter is incremented already.

You're right, calling bdrv_drained_begin() is better.

> 
> 
> 
> > 
> > > > What happens from guest perspective? In the case of virtio, the request 
> > > > queue is
> > > > not handled and -ETIMEDOUT may happen. With IDE, I/O commands are still 
> > > > handled,
> > > > the command is not effective (or rather the implementation is not 
> > > > complete).
> > > That it only works with virtio is fine. However, the thing it does not 
> > > work correctly
> > > apply then also to all other users of the drained_begin/end functions, 
> > > right?
> > > As for the timeout I only plan to drain the device for about 1 second.
> > It didn't matter because for IDE, the invariant (staying quiesced as long as
> > necessary) is already ensured by BQL.  Virtio is different because it 
> > supports
> > ioeventfd and data plane.
> 
> Okay understood. So my use of bdrv_drained_begin/end is more an abuse of
> these functions?

Sort of. But it's not unreasonable to "extend" bdrv_drained_begin/end to cover
IDE, I just haven't thought about "how".

> 
> Do you have another idea how to achieve what I want? I was thinking of 
> throttle
> the I/O to zero. It would be enough to do this for writes, reading doesn't 
> hurt in
> my case.

Maybe add a block filter on top of the drained node, drain it when doing so,
then queue all further requests with a CoQueue until "undrain".  (It is then not
quite to "drain" but to "halt" or "pause", though.)

Fam