On 29.12.25 17:07, Denis V. Lunev wrote:
This helper is mandatory to be called before bdrv_co_flush(). In the
other case bdrv_co_flush() will be noop. This helper should be called
after actual write is completed for subsequent flush to perform some
work.
Actually this change is important, without it BDRV_REQ_FUA semantics is
broken completely. flush() is not called. This smells like potential
data loss if somebody relies on BDRV_REQ_FUA.
Signed-off-by: Denis V. Lunev <[email protected]>
CC: Andrey Drobyshev <[email protected]>
CC: Kevin Wolf <[email protected]>
CC: Hanna Reitz <[email protected]>
---
block/io.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
Definitely an improvement, but if possible, I would still prefer keeping
the increment it in bdrv_co_write_req_finish(), because that is where
all write requests eventually end up in, and that’s also where dirty
bitmaps are handled. It seems slightly error-prone to manually have to
call this every time the BDS data is modified.
Can we maybe instead pull the FUA emulation out into
bdrv_aligned_pwritev(), after bdrv_co_write_req_finish()?
I think we can only pull out the flush, not the FUA emulation detection
because for write-zeroes, we have to check either supported_zero_flags
or supported_write_flags, depending on whether we fall back or not. But
bdrv_driver_pwritev() and bdrv_co_do_pwrite_zeroes() could return an
need_emulate_fua flag via pointer parameter, and if set,
bdrv_aligned_pwritev() can then do the flush after
bdrv_co_write_req_finish(). (This would also allow dropping both “only
flush once at the end” blocks.)
Maybe it would then also make sense to move the bdrv_co_flush() into
bdrv_co_write_req_finish(), and add a need_flush parameter to it, so
that it can see whether the whole request (write + flush) was successful.
Hanna
