Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dm-qcow2: retried flush after fsync ENOSPC can succeed because errseq reports the error once --
commit cfe92a6a11a8e762b7903b1085d1bba233f67647 Author: Alexander Ivanov <[email protected]> dm-qcow2: fix WARN in qrq_endio() on flush failing with ENOSPC This patch relaxes the WARN_ON_ONCE in qrq_endio() so that a REQ_OP_FLUSH completing with BLK_STS_NOSPC no longer triggers the warning. The flush is still delayed and resubmitted through qcow2_try_delay_enospc(). > diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c > --- a/drivers/md/dm-qcow2-map.c > +++ b/drivers/md/dm-qcow2-map.c [ ... ] > @@ -4491,7 +4491,8 @@ static void qrq_endio(struct qcow2_target *tgt, struct > qio *qio, > * callers receive -ENOSPC synchronous without intermediaries. > */ > if (unlikely(bi_status == BLK_STS_NOSPC)) { > - WARN_ON_ONCE(!op_is_write(qio->bi_op)); > + WARN_ON_ONCE(!op_is_write(qio->bi_op) && > + qio->bi_op != REQ_OP_FLUSH); > if (qcow2_try_delay_enospc(tgt, qrq, qio)) > return; > } [Severity: Medium] The commit message says "Delaying and resubmitting the flush is already correct". Is that accurate? The flush ENOSPC originates in do_qcow2_fsync_work(): if (fsync_ret == -EAGAIN) fsync_ret = vfs_fsync(qcow2->file, 0); end_qios(&flush_qios, errno_to_blk_status(fsync_ret)); On -ENOSPC, qrq_endio() calls qcow2_try_delay_enospc(), which requeues the qio on tgt->enospc_qios. After the timer fires, qcow2_submit_embedded_qio() sends it back to the fsync worker: WARN_ON_ONCE(qio->bi_op != REQ_OP_FLUSH); queue_list_id = QLIST_FLUSH; worker = &qcow2->fsync_worker; The worker then calls vfs_fsync() again on the same struct file. Writeback errors are reported through errseq. As documented in file_check_and_advance_wb_err(), fsync reports errors that occurred since the last fsync, and the call advances file->f_wb_err: err = errseq_check_and_advance(&mapping->wb_err, &file->f_wb_err); Can the retried vfs_fsync() on the same qcow2->file therefore return 0 on ext4 or xfs, which the commit message names for "a failed delayed write"? If so, the flush that originally failed with -ENOSPC would complete with BLK_STS_OK. The upper layer could then treat earlier writes as durable when they were not. A retried write resends its data, but a retried flush resends nothing. Is there something in dm-qcow2 that retains the earlier failure, or that otherwise makes the retry safe? Could the commit message explain why resubmitting the flush is correct, given the errseq behaviour? -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=12 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
