On Fri, Mar 01, 2024 at 11:17:10PM +0530, Prasad Pandit wrote: > Hello Petr, > > On Fri, 1 Mar 2024 at 14:46, <pet...@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 :|
