OpenVZ / Virtuozzo kernel development (devel@openvz.org)
 help / color / mirror / Atom feed
From: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
To: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Cc: devel@openvz.org
Subject: Re: [Devel] [PATCH hci-8.1 v9 00/10] dm-qcow2: truncate unused tail of image on suspend
Date: Thu, 29 Oct 2026 11:55:26 +0100	[thread overview]
Message-ID: <7ed35b11-2711-400e-baf4-4968806ae73c@virtuozzo.com> (raw)
In-Reply-To: <20261028150645.3222391-1-alexander.ivanov@virtuozzo.com>

Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>

On 10/28/26 16:06, Alexander Ivanov wrote:
> Discard punches holes in place of freed clusters, but doesn't change
> the file size. On postsuspend dm-qcow2 truncates the image file only
> down to file_preallocated_area_start, so if clusters at the end of the
> file are freed, the file never shrinks.
> 
> This series makes postsuspend scan the refcount blocks backward and
> truncate the file to the end of the last used cluster. Close and
> reload to RO go through postsuspend too, so the file is shrunk in these
> cases as well.
> 
> Patch 1 adds qcow2_md_page_read_sync() helper, which is needed by the
> scan. Patch 2 is a small cleanup of duplicate prefixes in QC_ERR/QC_INFO
> messages. Patches 3-9 fix existing cases where the refcounts or the
> dirty bit on disk don't reflect real cluster usage, or where such an
> image is not flagged for check, which the scan relies on. Patch 10 does
> the actual truncation.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-148171
> 
> v9:
> - Patch 5: new patch.
> - Patch 6: also keep the dirty bit while wants_check is set, as
>    md_writeback_error is kept only in the target and is lost on table
>    reload or host restart. Rename kernel_dirty_bit to
>    kernel_dirty_bit_set. Retitled accordingly.
> - Patch 7: new patch.
> - Patches 8, 9: split from patch 10: setting md_writeback_error on a
>    late reftable relocation failure and the MD_LX_REVERTED /
>    lx_writeback_error tracking are separate fixes now.
> - Patch 10: rename qcow2_truncate_preallocations() to
>    qcow2_truncate_unused_tail() and update the dm-ploop comment that
>    refers to it. Move the rationale from code comments to the commit
>    message.
> 
> v8:
> - Patch 3: instead of extending the file in place_r2(), make
>    find_unused_cluster() return the position for the new refblock and
>    let allocate_cluster() prepare it the same way as a data cluster
>    (punch, extend, watermark update moved into prepare_unused_cluster())
>    before calling place_r2(). Retitled accordingly.
> 
> v7:
> - Patch 5: track whether the image was opened dirty in a new opened_dirty
>    flag instead of checking the parsed header, which is reread on resume
>    and may carry the bit set by the kernel itself (e.g. on the lower image
>    of a cancelled backward merge). A new kernel_dirty_bit flag tells
>    whether the bit on disk is the kernel's own.
> - Patch 6: use opened_dirty to skip the scan.
> 
> v6:
> - Add Reviewed-by lines to patches 1 and 2.
> - Patch 2: fix "pase" typo in a message touched by the patch.
> - Patches 3-5: new patches.
> - Patch 6: drop Reviewed-by, as the patch changed a lot.
> - Patch 6: mark L1/L2 pages, whose failed writeback reverted refcounts
>    of new clusters, with MD_LX_REVERTED and report it via the new
>    lx_writeback_error flag, which skips the scan and is included in
>    wants_check.
>    Set md_writeback_error if relocate_refcount_table() fails
>    after the new header is written, and clamp the scan start to
>    reftable_max_file_size.
>    Skip the scan for an image opened with the dirty bit set
>    when kernel_sets_dirty_bit is enabled.
>    Check the R1 entry before calc_r2_page_id(), so a corrupted
>    one fails the scan without WARN.
>    Reword the commit message to make clear that discard does
>    free the clusters but doesn't shrink the file.
> 
> v5:
> - Add Reviewed-by line.
> 
> v4:
> - Patch 1: document in the comment that the caller must ensure no read
>    of the page is in flight.
> - Patch 3: say in the commit message that close and reload to RO go
>    through postsuspend too.
> 
> v3:
> - Split from the common series with dm-ploop: dm-ploop patches are
>    sent as a separate series.
> - Patch 1: rename qcow2_get_md_page_sync() to qcow2_md_page_read_sync(),
>    reword commit message and comment.
> - Patch 2: new patch.
> - Patch 3: start the scan from file_preallocated_area_start instead of
>    the end of the file and clamp the result to it, as the last cluster
>    may be partial; print the scan error with %pe and drop the duplicate
>    "dm-qcow2:" prefix from it.
> 
> v2:
> - Add dm-ploop support (patches 3-6), as requested in v1 review.
> - dm-qcow2: move get_md_page_sync() to a separate patch as
>    qcow2_md_page_read_sync() and use it in qcow2_parse_header() as well.
> - dm-qcow2: start the refcount scan below file_preallocated_area_start
>    instead of the end of the file, and truncate to the used end directly
>    instead of min(file_preallocated_area_start, used end).
> 
> Alexander Ivanov (10):
>    dm-qcow2: add qcow2_md_page_read_sync() helper
>    dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages
>    dm-qcow2: extend the file for a new refblock in allocate_cluster()
>    dm-qcow2: complete wbd before ending the data qio
>    dm-qcow2: flush lower metadata before clearing its dirty bit on merge
>      failure
>    dm-qcow2: keep the dirty bit of an image that needs check
>    dm-qcow2: fix md page cleanup on failed reftable relocation
>    dm-qcow2: flag the image for check if reftable relocation fails late
>    dm-qcow2: flag the image for check after a reverted L1/L2 writeback
>    dm-qcow2: truncate unused tail of image on suspend
> 
>   drivers/md/dm-ploop-target.c |   2 +-
>   drivers/md/dm-qcow2-cmd.c    |   5 +-
>   drivers/md/dm-qcow2-map.c    | 180 +++++++++++++++++++++++++----------
>   drivers/md/dm-qcow2-target.c | 119 +++++++++++++++++------
>   drivers/md/dm-qcow2.h        |  10 +-
>   5 files changed, 234 insertions(+), 82 deletions(-)
> 

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

  parent reply	other threads:[~2026-10-29 10:56 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-28 15:06 Alexander Ivanov
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 01/10] dm-qcow2: add qcow2_md_page_read_sync() helper Alexander Ivanov
2026-10-28 15:12   ` no-reply
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 02/10] dm-qcow2: drop duplicate prefix from QC_ERR/QC_INFO messages Alexander Ivanov
2026-10-28 15:10   ` no-reply
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 03/10] dm-qcow2: extend the file for a new refblock in allocate_cluster() Alexander Ivanov
2026-10-28 15:11   ` no-reply
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 04/10] dm-qcow2: complete wbd before ending the data qio Alexander Ivanov
2026-10-28 15:14   ` no-reply
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 05/10] dm-qcow2: flush lower metadata before clearing its dirty bit on merge failure Alexander Ivanov
2026-10-28 15:14   ` no-reply
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 06/10] dm-qcow2: keep the dirty bit of an image that needs check Alexander Ivanov
2026-10-28 15:16   ` no-reply
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 07/10] dm-qcow2: fix md page cleanup on failed reftable relocation Alexander Ivanov
2026-10-28 15:16   ` no-reply
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 08/10] dm-qcow2: flag the image for check if reftable relocation fails late Alexander Ivanov
2026-10-28 15:17   ` no-reply
2026-10-29  7:18     ` Alexander Ivanov
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 09/10] dm-qcow2: flag the image for check after a reverted L1/L2 writeback Alexander Ivanov
2026-10-28 15:19   ` no-reply
2026-10-29  7:27     ` Alexander Ivanov
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-28 15:06 ` [Devel] [PATCH hci-8.1 v9 10/10] dm-qcow2: truncate unused tail of image on suspend Alexander Ivanov
2026-10-28 15:21   ` no-reply
2026-11-03 17:50   ` Konstantin Khorenko
2026-10-29 10:55 ` Andrey Zhadchenko [this message]
2026-11-03 17:50 ` [Devel] [PATCH hci-8.1 v9 00/10] " Konstantin Khorenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7ed35b11-2711-400e-baf4-4968806ae73c@virtuozzo.com \
    --to=andrey.zhadchenko@virtuozzo.com \
    --cc=alexander.ivanov@virtuozzo.com \
    --cc=devel@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox