[Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[Alexander Ivanov]

On postsuspend we truncate the image file only down to
file_preallocated_area_start, i.e. we drop preallocations only.
Clusters at the end of the file freed by discard (refcount dropped
to zero) are never given back, so the file size never shrinks.

Add qcow2_calc_used_end(), which scans refcount blocks backward from
the end of the file and returns the end of the last cluster with
non-zero refcount. Refcount ranges with no allocated refblock are
skipped at once. Metadata pages are taken from md cache or read
synchronously. This is called on suspended device after md
writeback, so R1 and R2 can't change under us.

Truncate the file to min(file_preallocated_area_start, used end).
Skip the scan if md writeback failed, as refcounts on disk are stale
then, and fall back to the old behaviour on scan error.

https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-qcow2: block device over QCOW2 files driver

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c    | 77 ++++++++++++++++++++++++++++++++++++
 drivers/md/dm-qcow2-target.c | 12 +++++-
 drivers/md/dm-qcow2.h        |  1 +
 3 files changed, 89 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 78ee7cf5e881..39d465343d8b 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -2208,6 +2208,83 @@ int qcow2_truncate_safe(struct file *file, loff_t new_len)
 	return do_ftruncate(file, new_len, 0);
 }
 
+static int get_md_page_sync(struct qcow2 *qcow2, u64 id, struct md_page **md)
+{
+	int ret;
+
+	spin_lock_irq(&qcow2->md_pages_lock);
+	*md = qcow2_md_page_find_or_postpone(qcow2, id, NULL);
+	spin_unlock_irq(&qcow2->md_pages_lock);
+	if (*md)
+		return 0;
+
+	ret = qcow2_alloc_and_insert_md_page(qcow2, id, md);
+	if (ret)
+		return ret;
+
+	ret = qcow2_rw_page_sync(READ, qcow2, id, (*md)->page);
+	spin_lock_irq(&qcow2->md_pages_lock);
+	if (ret)
+		qcow2_md_page_erase(qcow2, *md);
+	else
+		(*md)->status |= MD_UPTODATE;
+	spin_unlock_irq(&qcow2->md_pages_lock);
+	if (ret)
+		qcow2_free_md_page(*md);
+	return ret;
+}
+
+/*
+ * Returns the end of the last cluster, which is used according
+ * to refcount table. Clusters freed by discard may leave the tail
+ * of file unused. This must be called on suspended device after
+ * md writeback, so nobody changes R1 and R2.
+ */
+loff_t qcow2_calc_used_end(struct qcow2 *qcow2)
+{
+	u32 clu_size = qcow2->clu_size;
+	struct qcow2_map_item r1, r2;
+	struct md_page *r1_md = NULL;
+	loff_t pos;
+	s32 i;
+	int ret;
+
+	pos = round_down(qcow2->file_size - 1, clu_size);
+	while (pos >= 0) {
+		if (calc_refcounters_map(qcow2, pos, &r1, &r2) < 0)
+			return -EIO;
+
+		if (!r1_md || r1_md->id != r1.page_id) {
+			ret = get_md_page_sync(qcow2, r1.page_id, &r1_md);
+			if (ret)
+				return ret;
+		}
+		r1.md = r1_md;
+
+		ret = calc_r2_page_id(qcow2, &r1, &r2);
+		if (ret == -ENOENT) {
+			/* No R2 cluster, so all clusters it covers are unused */
+			pos = (loff_t)r1.index * qcow2->refblock_entries * clu_size;
+			pos -= clu_size;
+			continue;
+		}
+
+		if (ret < 0)
+			return ret;
+
+		ret = get_md_page_sync(qcow2, r2.page_id, &r2.md);
+		if (ret)
+			return ret;
+
+		for (i = r2.index_in_page; i >= 0; i--, pos -= clu_size) {
+			if (get_r2_entry(qcow2, r2.md, i))
+				return pos + clu_size;
+		}
+	}
+
+	return -EIO;
+}
+
 static int truncate_prealloc_safe(struct qcow2 *qcow2, loff_t len, const char *func)
 {
 	loff_t prealloc_len, max_prealloc_len = qcow2->reftable_max_file_size;
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index be09615eb8e2..95c8b114d92f 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -1016,11 +1016,21 @@ static void qcow2_truncate_preallocations(struct dm_target *ti)
 {
 	struct qcow2_target *tgt = to_qcow2_target(ti);
 	struct qcow2 *qcow2 = top_qcow2_protected(ti);
-	loff_t end = qcow2->file_preallocated_area_start;
+	loff_t used_end, end = qcow2->file_preallocated_area_start;
 	int ret;
 
 	if (!(dm_table_get_mode(ti->table) & FMODE_WRITE))
 		return;
+
+	/* Refcounts on disk are stale after failed md writeback */
+	if (!tgt->md_writeback_error) {
+		used_end = qcow2_calc_used_end(qcow2);
+		if (used_end < 0)
+			QC_ERR(ti, "dm-qcow2: Can't find used end: %lld", used_end);
+		else if (used_end < end)
+			end = used_end;
+	}
+
 	if (end == qcow2->file_size)
 		return;
 
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index d9b8c38e093f..2c5eb7578316 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -409,6 +409,7 @@ void qcow2_ref_dec(struct qcow2_target *tgt, u8 ref_index);
 int qcow2_inflight_ref_switch(struct qcow2_target *tgt);
 void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2);
 int qcow2_truncate_safe(struct file *file, loff_t new_len);
+loff_t qcow2_calc_used_end(struct qcow2 *qcow2);
 
 void qcow2_merge_backward_work(struct work_struct *work);
 void qcow2_merge_backward_cancel(struct qcow2_target *tgt);
-- 
2.53.0

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

Re: [Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[Denis V. Lunev]

On 10/6/26 16:44, Alexander Ivanov wrote:
> On postsuspend we truncate the image file only down to
> file_preallocated_area_start, i.e. we drop preallocations only.
> Clusters at the end of the file freed by discard (refcount dropped
> to zero) are never given back, so the file size never shrinks.
>
> Add qcow2_calc_used_end(), which scans refcount blocks backward from
> the end of the file and returns the end of the last cluster with
> non-zero refcount. Refcount ranges with no allocated refblock are
> skipped at once. Metadata pages are taken from md cache or read
> synchronously. This is called on suspended device after md
> writeback, so R1 and R2 can't change under us.
>
> Truncate the file to min(file_preallocated_area_start, used end).
> Skip the scan if md writeback failed, as refcounts on disk are stale
> then, and fall back to the old behaviour on scan error.
>
> https://virtuozzo.atlassian.net/browse/VSTOR-148171
> Feature: dm-qcow2: block device over QCOW2 files driver
>
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>  drivers/md/dm-qcow2-map.c    | 77 ++++++++++++++++++++++++++++++++++++
>  drivers/md/dm-qcow2-target.c | 12 +++++-
>  drivers/md/dm-qcow2.h        |  1 +
>  3 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> index 78ee7cf5e881..39d465343d8b 100644
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
> @@ -2208,6 +2208,83 @@ int qcow2_truncate_safe(struct file *file, loff_t new_len)
>  	return do_ftruncate(file, new_len, 0);
>  }
>  
> +static int get_md_page_sync(struct qcow2 *qcow2, u64 id, struct md_page **md)
> +{
> +	int ret;
> +
> +	spin_lock_irq(&qcow2->md_pages_lock);
> +	*md = qcow2_md_page_find_or_postpone(qcow2, id, NULL);
> +	spin_unlock_irq(&qcow2->md_pages_lock);
> +	if (*md)
> +		return 0;
> +
> +	ret = qcow2_alloc_and_insert_md_page(qcow2, id, md);
> +	if (ret)
> +		return ret;
> +
> +	ret = qcow2_rw_page_sync(READ, qcow2, id, (*md)->page);
> +	spin_lock_irq(&qcow2->md_pages_lock);
> +	if (ret)
> +		qcow2_md_page_erase(qcow2, *md);
> +	else
> +		(*md)->status |= MD_UPTODATE;
> +	spin_unlock_irq(&qcow2->md_pages_lock);
> +	if (ret)
> +		qcow2_free_md_page(*md);
> +	return ret;
> +}
> +
> +/*
> + * Returns the end of the last cluster, which is used according
> + * to refcount table. Clusters freed by discard may leave the tail
> + * of file unused. This must be called on suspended device after
> + * md writeback, so nobody changes R1 and R2.
> + */
> +loff_t qcow2_calc_used_end(struct qcow2 *qcow2)
> +{
> +	u32 clu_size = qcow2->clu_size;
> +	struct qcow2_map_item r1, r2;
> +	struct md_page *r1_md = NULL;
> +	loff_t pos;
> +	s32 i;
> +	int ret;
> +
> +	pos = round_down(qcow2->file_size - 1, clu_size);
> +	while (pos >= 0) {
> +		if (calc_refcounters_map(qcow2, pos, &r1, &r2) < 0)
> +			return -EIO;
> +
> +		if (!r1_md || r1_md->id != r1.page_id) {
> +			ret = get_md_page_sync(qcow2, r1.page_id, &r1_md);
> +			if (ret)
> +				return ret;
> +		}
> +		r1.md = r1_md;
> +
> +		ret = calc_r2_page_id(qcow2, &r1, &r2);
> +		if (ret == -ENOENT) {
> +			/* No R2 cluster, so all clusters it covers are unused */
> +			pos = (loff_t)r1.index * qcow2->refblock_entries * clu_size;
> +			pos -= clu_size;
> +			continue;
> +		}
> +
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = get_md_page_sync(qcow2, r2.page_id, &r2.md);
> +		if (ret)
> +			return ret;
> +
> +		for (i = r2.index_in_page; i >= 0; i--, pos -= clu_size) {
> +			if (get_r2_entry(qcow2, r2.md, i))
> +				return pos + clu_size;
> +		}
> +	}
> +
> +	return -EIO;
> +}
> +
>  static int truncate_prealloc_safe(struct qcow2 *qcow2, loff_t len, const char *func)
>  {
>  	loff_t prealloc_len, max_prealloc_len = qcow2->reftable_max_file_size;
> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
> index be09615eb8e2..95c8b114d92f 100644
> --- a/drivers/md/dm-qcow2-target.c
> +++ b/drivers/md/dm-qcow2-target.c
> @@ -1016,11 +1016,21 @@ static void qcow2_truncate_preallocations(struct dm_target *ti)
>  {
>  	struct qcow2_target *tgt = to_qcow2_target(ti);
>  	struct qcow2 *qcow2 = top_qcow2_protected(ti);
> -	loff_t end = qcow2->file_preallocated_area_start;
> +	loff_t used_end, end = qcow2->file_preallocated_area_start;
>  	int ret;
>  
>  	if (!(dm_table_get_mode(ti->table) & FMODE_WRITE))
>  		return;
> +
> +	/* Refcounts on disk are stale after failed md writeback */
> +	if (!tgt->md_writeback_error) {
> +		used_end = qcow2_calc_used_end(qcow2);
> +		if (used_end < 0)
> +			QC_ERR(ti, "dm-qcow2: Can't find used end: %lld", used_end);
> +		else if (used_end < end)
> +			end = used_end;
> +	}
> +
>  	if (end == qcow2->file_size)
>  		return;
>  
> diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
> index d9b8c38e093f..2c5eb7578316 100644
> --- a/drivers/md/dm-qcow2.h
> +++ b/drivers/md/dm-qcow2.h
> @@ -409,6 +409,7 @@ void qcow2_ref_dec(struct qcow2_target *tgt, u8 ref_index);
>  int qcow2_inflight_ref_switch(struct qcow2_target *tgt);
>  void qcow2_flush_deferred_activity(struct qcow2_target *tgt, struct qcow2 *qcow2);
>  int qcow2_truncate_safe(struct file *file, loff_t new_len);
> +loff_t qcow2_calc_used_end(struct qcow2 *qcow2);
>  
>  void qcow2_merge_backward_work(struct work_struct *work);
>  void qcow2_merge_backward_cancel(struct qcow2_target *tgt);
ploop?
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[no-reply]

> On postsuspend we truncate the image file only down to
> file_preallocated_area_start, i.e. we drop preallocations only.
> Clusters at the end of the file freed by discard (refcount dropped
> to zero) are never given back, so the file size never shrinks.
> 
> Add qcow2_calc_used_end(), which scans refcount blocks backward from
> the end of the file and returns the end of the last cluster with
> non-zero refcount. Refcount ranges with no allocated refblock are
> skipped at once. Metadata pages are taken from md cache or read
> synchronously. This is called on suspended device after md
> writeback, so R1 and R2 can't change under us.
> 
> Truncate the file to min(file_preallocated_area_start, used end).
> Skip the scan if md writeback failed, as refcounts on disk are stale
> then, and fall back to the old behaviour on scan error.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-148171
> Feature: dm-qcow2: block device over QCOW2 files driver
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261006144428.341824-1-alexander.ivanov@virtuozzo.com?part=1


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

Re: [Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[Konstantin Khorenko]

> drivers/md/dm-qcow2: truncate unused tail of image on suspend
...
> Truncate the file to min(file_preallocated_area_start, used end).
> Skip the scan if md writeback failed, as refcounts on disk are stale
> then, and fall back to the old behaviour on scan error.

There is one more case of stale refcounts on disk: an image with
INCOMPATIBLE_FEATURES_DIRTY_BIT set. qcow2_check_convert_hdr() accepts it
(the check is commented out), and with qemu lazy refcounts that bit means
refcounts were not written for the last allocations, so they are too low.

Today such an image loses data only when the allocator hands out a
cluster whose refcount is zero but which L2 still references. After this
patch the first suspend truncates live clusters at the tail without a
single write to the device. The image is already invalid per spec, so
this is not a regression in the strict sense, but either the commit
message should say that a dirty image is not supported, or the dirty-bit
check in qcow2_check_convert_hdr() should come back.

> +static int get_md_page_sync(struct qcow2 *qcow2, u64 id, struct md_page **md)
> +{
> +     int ret;
> +
> +     spin_lock_irq(&qcow2->md_pages_lock);
> +     *md = qcow2_md_page_find_or_postpone(qcow2, id, NULL);
> +     spin_unlock_irq(&qcow2->md_pages_lock);
> +     if (*md)
> +             return 0;
> +
> +     ret = qcow2_alloc_and_insert_md_page(qcow2, id, md);
> +     if (ret)
> +             return ret;
> +
> +     ret = qcow2_rw_page_sync(READ, qcow2, id, (*md)->page);

qcow2_attach_file() does the same alloc + qcow2_rw_page_sync(READ) +
MD_UPTODATE sequence by hand for page 0, minus the erase on failure. If
this helper lives in dm-qcow2-target.c next to the other md page helpers
and is exported through dm-qcow2.h, that call site can use it as well.

> +     pos = round_down(qcow2->file_size - 1, clu_size);
> +     while (pos >= 0) {

Everything at or above file_preallocated_area_start is unused by
construction: allocate_cluster() moves it past every cluster it hands
out, and relocate_refcount_table() goes through truncate_prealloc_safe(),
which sets it to the requested length. Starting from file_size scans the
R2 entries covering the preallocated area (up to PREALLOC_SIZE) on every
suspend for nothing. Start from file_preallocated_area_start - 1 instead.

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[Denis V. Lunev]

On 10/6/26 17:52, Konstantin Khorenko wrote:
>> drivers/md/dm-qcow2: truncate unused tail of image on suspend
> ...
>> Truncate the file to min(file_preallocated_area_start, used end).
>> Skip the scan if md writeback failed, as refcounts on disk are stale
>> then, and fall back to the old behaviour on scan error.
> There is one more case of stale refcounts on disk: an image with
> INCOMPATIBLE_FEATURES_DIRTY_BIT set. qcow2_check_convert_hdr() accepts it
> (the check is commented out), and with qemu lazy refcounts that bit means
> refcounts were not written for the last allocations, so they are too low.
>
> Today such an image loses data only when the allocator hands out a
> cluster whose refcount is zero but which L2 still references. After this
> patch the first suspend truncates live clusters at the tail without a
> single write to the device. The image is already invalid per spec, so
> this is not a regression in the strict sense, but either the commit
> message should say that a dirty image is not supported, or the dirty-bit
> check in qcow2_check_convert_hdr() should come back.

Image should go clean into the kernel on first pass.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1] drivers/md/dm-qcow2: truncate unused tail of image on suspend

[Konstantin Khorenko]

On 10/6/26 17:56, Denis V. Lunev wrote:
> On 10/6/26 17:52, Konstantin Khorenko wrote:
>>> drivers/md/dm-qcow2: truncate unused tail of image on suspend
>> ...
>>> Truncate the file to min(file_preallocated_area_start, used end).
>>> Skip the scan if md writeback failed, as refcounts on disk are stale
>>> then, and fall back to the old behaviour on scan error.
>> There is one more case of stale refcounts on disk: an image with
>> INCOMPATIBLE_FEATURES_DIRTY_BIT set. qcow2_check_convert_hdr() accepts it
>> (the check is commented out), and with qemu lazy refcounts that bit means
>> refcounts were not written for the last allocations, so they are too low.
>>
>> Today such an image loses data only when the allocator hands out a
>> cluster whose refcount is zero but which L2 still references. After this
>> patch the first suspend truncates live clusters at the tail without a
>> single write to the device. The image is already invalid per spec, so
>> this is not a regression in the strict sense, but either the commit
>> message should say that a dirty image is not supported, or the dirty-bit
>> check in qcow2_check_convert_hdr() should come back.
> 
> Image should go clean into the kernel on first pass.

Well, i agree here, so please disregard my comment about dirty bit handling.

On the other hand i would like to note that the current design does not allow kernel to distinguish 2 situations:

1. the image is CLEAN (it might be checked / fixed after the crash, does not matter),
ploop utility marks it as DIRTY when sending to the kernel - intended behavior.

2. the image had DIRTY bit because the Node crashed, so the image was NOT fixed, but
someone issued "dmsetup create" manually, not via ploop util.


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