Re: [PATCH v2 28/29] migration: Add direct-io parameter

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Oct 25, 2023 at 11:48:08AM -0300, Fabiano Rosas wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
> > On Mon, Oct 23, 2023 at 05:36:07PM -0300, Fabiano Rosas wrote:
> >> Add the direct-io migration parameter that tells the migration code to
> >> use O_DIRECT when opening the migration stream file whenever possible.
> >> 
> >> This is currently only used for the secondary channels of fixed-ram
> >> migration, which can guarantee that writes are page aligned.
> >
> > When you say "secondary channels", I presume you're meaning that
> > the bulk memory regions will be written with O_DIRECT, while
> > general vmstate will use normal I/O on the main channel ?  If so,
> > could we explain that a little more directly.
> 
> Yes, the main channel writes via QEMUFile, so no O_DIRECT. The channels
> created via multifd_new_send_channel_create() have O_DIRECT enabled.
> 
> > Having a mixture of O_DIRECT and non-O_DIRECT I/O on the same
> > file is a little bit of an unusual situation. It will work for
> > us because we're writing to different regions of the file in
> > each case.
> >
> > Still I wonder if it would be sane in the outgoing case to
> > include a fsync() on the file in the main channel, to guarantee
> > that the whole saved file is on-media at completion ? Or perhaps
> > suggest in QAPI that mgmts might consider doing a fsync
> > themselves ?
> 
> I think that should be present in QIOChannelFile in general. Not even
> related to this series. I'll add it at qio_channel_file_close() unless
> you object.

Yes, looking at the places QIOChannelFile is used, I think they would
all benefit from fdatasync().  fsync() is probably too big of a hammer

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|