[Devel] [PATCH hci-8.1 v12 12/12] dm-qcow2: fix WARN in qrq_endio() on flush failing with ENOSPC

[no-reply at sashiko.virtuozzo.com]

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 <alexander.ivanov at virtuozzo.com>

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/20261110075140.1475607-1-alexander.ivanov@virtuozzo.com?part=12