Re: [PATCH v1 1/3] migration: Fix missing qemu_fflush() on buffer file in bg_migration_thread

[Peter Xu <peterx@redhat.com>]

On Wed, Mar 24, 2021 at 05:35:44PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > On Tue, Mar 23, 2021 at 08:21:43PM +0300, Andrey Gruzdev wrote:
> > > > For the long term I think we'd better have a helper:
> > > > 
> > > >          qemu_put_qio_channel_buffer(QEMUFile *file, QIOChannelBuffer *bioc)
> > > > 
> > > > So as to hide this flush operation, which is tricky. We'll have two users so
> > > > far:
> > > > 
> > > >          bg_migration_completion
> > > >          colo_do_checkpoint_transaction
> > > > 
> > > > IMHO it'll be nicer if you'd do it in this patch altogether!
> > > > 
> > > > Thanks,
> > > > 
> > > Sorry, can't get the idea, what's wrong with the fix.
> > 
> > I'm fine with the fix, but I've got one patch attached just to show what I
> > meant, so without any testing for sure..
> > 
> > Looks more complicated than I thought, but again I think we should hide that
> > buffer flush into another helper to avoid overlooking it.
> 
> I was wondering if I was missing the same fflush in postcopy, but I
> don't *think* so, although it's a bit round about; before sending the
> data I call:
> 
>   qemu_savevm_send_postcopy_run(fb)
> 
> and that calls qemu_savevm_command_send that ends in a fflish;  which is
> non-obvious.
> 
> While I'd leave that in there, it might be good to use that same thing.

Right, I was grepping qemu_put_buffer() previously, so as to easily got
qemu_savevm_send_packaged() overlooked..

Maybe I can make it a small patch series after the snapshot fixes got in.

Thanks,

-- 
Peter Xu