On 12/29/25 15:36, Denis V. Lunev wrote: > On 12/29/25 15:29, Denis V. Lunev wrote: >> On 12/19/25 19:08, Denis V. Lunev wrote: >>> qcow2_header_updated() is the final call during image close. This means >>> after this point we will have no IO operations on this file descriptor. >>> This assumption has been validated via 'strace' which clearly confirms >>> that this is very final write and there is no sync after this point. >>> >>> There is almost no problem when the image is residing in local >>> filesystem except that we will have image check if the chage will >>> not reach disk before powerloss, but with a network or distributed >>> filesystem we come to trouble. The change could be in flight and we >>> can miss this data on other node like during migration. >>> >>> The patch adds BDRV_REQ_FUA to the write request to do the trick. >>> >>> Signed-off-by: Denis V. Lunev <[email protected]> >>> CC: Kevin Wolf <[email protected]> >>> CC: Hanna Reitz <[email protected]> >>> --- >>> block/qcow2.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/block/qcow2.c b/block/qcow2.c >>> index e29810d86a..abbc4e82ba 100644 >>> --- a/block/qcow2.c >>> +++ b/block/qcow2.c >>> @@ -3252,7 +3252,7 @@ int qcow2_update_header(BlockDriverState *bs) >>> } >>> >>> /* Write the new header */ >>> - ret = bdrv_pwrite(bs->file, 0, s->cluster_size, header, 0); >>> + ret = bdrv_pwrite(bs->file, 0, s->cluster_size, header, BDRV_REQ_FUA); >>> if (ret < 0) { >>> goto fail; >>> } >> (attempt #2, mailer has rotten the mail. sorry). >> >> Thanks a lot for Andrey Drobyshev who has attempted to review and >> test this patch. >> >> The patch is non-working due to broken BDRV_REQ_FUA processing in QEMU >> code and this looks more severe than the original problem. bdrv_pwrite() >> endups in bdrv_aligned_pwritev() which internally calls >> bdrv_driver_pwritev() -> bdrv_co_flush() bdrv_co_write_req_finish() >> Please note, that bdrv_co_flush() is in reality noop until bs->write_gen >> is incremented, which happened only late inside >> bdrv_co_write_req_finish(). This means that BDRV_REQ_FUA request will be >> flushed only on the NEXT flush operation, which is definitely wrong. Den > (attempt #3, mailer has rotten the mail. sorry. Finally get it working) > > Thanks a lot for Andrey Drobyshev who has attempted to review and > test this patch. > > The patch is non-working due to broken BDRV_REQ_FUA processing in QEMU > code and this looks more severe than the original problem. > > bdrv_pwrite() endups in bdrv_aligned_pwritev() which internally calls > bdrv_driver_pwritev() -> bdrv_co_flush() > bdrv_co_write_req_finish() > Please note, that bdrv_co_flush() is in reality noop until bs->write_gen > is incremented, which happened only late inside bdrv_co_write_req_finish(). > > This means that BDRV_REQ_FUA request will be flushed only on the NEXT > flush operation, which is definitely wrong. > > Den > please disregard this patch. Fixes have been sent.
Den
