Re: [PATCH] migration/multifd: Document two places for mapped-ram

[Daniel P . Berrangé]

On Fri, Mar 01, 2024 at 11:17:10PM +0530, Prasad Pandit wrote:
> Hello Petr,
> 
> On Fri, 1 Mar 2024 at 14:46, <peterx@redhat.com> wrote:
> > +         * An explicitly close() on the channel here is normally not
> 
> explicitly -> explicit
> 
> > +         * required, but can be helpful for "file:" iochannels, where it
> > +         * will include an fdatasync() to make sure the data is flushed to
> > +         * the disk backend.
> 
> * an fdatasync() -> fdatasync()
> 
> * qio_channel_close
>     -> ioc_klass->io_close = qio_channel_file_close;
>      -> qemu_close(fioc->fd)
>       -> close(fd);
> 
> It does not seem to call fdatasync() before close(fd);
> 
>     - qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC, ...)

The documented behaviour reliant on another pending patch:

https://lists.nongnu.org/archive/html/qemu-devel/2024-02/msg07046.html

> 
> Maybe the qio_channel..() calls above should include the 'O_DSYNC'
> flag as well? But that will do fdatasync() work at each write(2) call
> I think, not sure if that is okay.



> 
> > +         *
> > +         * The object_unref() cannot guarantee that because: (1) finalize()
> > +         * of the iochannel is only triggered on the last reference, and
> > +         * it's not guaranteed that we always hold the last refcount when
> > +         * reaching here, and, (2) even if finalize() is invoked, it only
> > +         * does a close(fd) without data flush.
> > +         */
> 
> * object_unref
>     -> object_finalize
>       -> object_deinit
>         -> type->instance_finalize
>          -> qio_channel_file_finalize
>           -> qemu_close(ioc->fd);
> 
> * I hope I'm looking at the right code here. (Sorry if I'm not)

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 :|