Re: [Qemu-devel] [RFC PATCH RDMA support v4: 08/10] introduce QEMUFileRDMA

[Michael R. Hines]

On 03/19/2013 09:25 AM, Paolo Bonzini wrote:
> Yes, the drain needs to happen in a few places already:
>
> 1. During save_rdma_page (if the current "chunk" is full of pages)
> Ok, this is internal to RDMA so no problem.
>
> > 2. During the end of each iteration (now using qemu_fflush in my current
> > patch)
> Why?

This is because of downtime: You have to drain the queue anyway at the
very end, and if you don't drain it in advance after each iteration, then
the queue will have lots of bytes in it waiting for transmission and the
Virtual Machine will be stopped for a much longer period of time during
the last iteration waiting for RDMA card to finish transmission of all those
bytes.

If you wait till the last iteration to do this, then all of that waiting 
time gets
counted as downtime, causing the VCPUs to be unnecessarily stopped.


> > 3. And also during qemu_savem_state_complete(), also using qemu_fflush.
> This would be caught by put_buffer, but (2) would not.

I'm not sure this is good enough either - we don't want to flush
the queue *frequently*..... only when it's necessary for performance
.... we do want the queue to have some meat to it so the hardware
can write bytes as fast as possible.....

If we flush inside put_buffer (which is called very frequently): then
we have no way to distinquish *where* put buffer was called from
(either from qemu_savevm_state_complete() or from a device-level
function call that's using QEMUFile).

> > > Yes, I am suggesting to modify f->pos in save_rdma_page instead.
> > >
> > > Paolo
> > Would that not confuse the other QEMUFile users?
> > If I change that pointer (without actually putting bytes
> > in into QEMUFile), won't the f->pos pointer be
> > incorrectly updated?
> f->pos is never used directly by QEMUFile, it is almost an opaque value.
>   It is accumulated on every qemu_fflush (so that it can be passed to the
> ->put_buffer function), and returned by qemu_ftell; nothing else.
>
> If you make somehow save_rdma_page a new op, returning a value from that
> op and adding it to f->pos would be a good way to achieve this.

Ok, great - I'll take advantage of that........Thanks.