[Devel] [PATCH hci-8.1 0/3] dm-qcow2: keep images of a read-only table unmodified

[Devel] [PATCH hci-8.1 0/3] dm-qcow2: keep images of a read-only table unmodified

[Alexander Ivanov]

A read-only dm-qcow2 table does not protect its images. Writes to the
device pass through bio_check_ro() with a warning and allocate clusters
in the top image, an O_RDWR top image is truncated on .ctr and
.preresume, and merge_backward modifies the lower image. Since the
header is not updated for a read-only table, persistent bitmaps stay
marked consistent while the data under them changes.

Patch 1 makes an image writable only if both its file and the table
are writable. Patch 2 fails writes to a read-only table. Patch 3 keeps
the bitmaps autoclear bit clear if it is already clear in the image,
so that a later writable open does not revalidate stale bitmaps.

https://virtuozzo.atlassian.net/browse/VSTOR-138887

Alexander Ivanov (3):
  drivers/md/dm-qcow2: treat images of a read-only table as read-only
  drivers/md/dm-qcow2: fail writes to a read-only table
  drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps

 drivers/md/dm-qcow2-map.c    |  9 ++++++---
 drivers/md/dm-qcow2-target.c | 11 +++++++----
 drivers/md/dm-qcow2.h        |  3 ++-
 3 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.53.0

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

[Devel] [PATCH hci-8.1 1/3] drivers/md/dm-qcow2: treat images of a read-only table as read-only

[Alexander Ivanov]

Whether an image may be modified is decided by the mode of its file
only. Userspace may pass an O_RDWR top image to a read-only table, and
then dm-qcow2 truncates the unaligned image tail on every .ctr and
.preresume and considers the image writable on the I/O and discard
paths.

merge_backward is also allowed on a read-only table if the lower image
is opened for write. qcow2_set_image_file_features() checks the table
mode and skips the lower image, so the merge writes to it without the
dirty bit and without clearing the bitmaps autoclear bit. Enabled
persistent bitmaps of the lower image are left marked consistent
although the image data changed.

Consider an image writable only if both its file and the table are
writable, and use this check for the header update, the tail truncate
and the short read near EOF as well. merge_backward on a read-only
table now fails with -EACCES.

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

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c    | 4 +---
 drivers/md/dm-qcow2-target.c | 6 ++----
 drivers/md/dm-qcow2.h        | 3 ++-
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 78ee7cf5e881..54b3a345f927 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -1470,7 +1470,6 @@ static void md_page_read_complete(struct qio *qio)
 	struct md_page *md = qio->ext->md;
 	struct qcow2 *qcow2 = qio->qcow2;
 	int ret = qio->ret;
-	mode_t mode;
 
 	BUG_ON(qvec->bvec[0].bv_page != md->page);
 
@@ -1478,8 +1477,7 @@ static void md_page_read_complete(struct qio *qio)
 		/* Read near EOF? See qcow2_attach_file() */
 		loff_t pos = (md->id << PAGE_SHIFT) + ret;
 
-		mode = qcow2->file->f_mode;
-		if (pos == qcow2->file_size && !(mode & FMODE_WRITE)) {
+		if (pos == qcow2->file_size && !qcow2_file_is_writable(qcow2)) {
 			qcow2_zero_fill_page_from(md->page, ret);
 			ret = PAGE_SIZE;
 		}
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index be09615eb8e2..2d1e5d4e1b3a 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -623,13 +623,11 @@ int qcow2_set_image_file_features(struct qcow2 *qcow2, bool dirty)
 	u64 dirty_mask = cpu_to_be64(INCOMPATIBLE_FEATURES_DIRTY_BIT);
 	struct QCowHeader *raw_hdr;
 	struct md_page *md;
-	bool is_ro;
 
 	if (qcow2->hdr.version ==  2)
 		return 0;
 
-	is_ro = !(dm_table_get_mode(qcow2->tgt->ti->table) & FMODE_WRITE);
-	if (is_ro)
+	if (!qcow2_file_is_writable(qcow2))
 		return 0;
 
 	md = qcow2_md_page_find(qcow2, 0);
@@ -840,7 +838,7 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
 	int ret;
 
 	qcow2->file_size = i_size_read(file_inode(file));
-	if ((file->f_mode & FMODE_WRITE) && (qcow2->file_size & ~PAGE_MASK)) {
+	if (qcow2_file_is_writable(qcow2) && (qcow2->file_size & ~PAGE_MASK)) {
 		new_size = PAGE_ALIGN(qcow2->file_size);
 		ret = qcow2_truncate_safe(file, new_size);
 		if (ret) {
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index d9b8c38e093f..8f10c5812a1b 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -462,7 +462,8 @@ static inline bool qcow2_wants_check(struct qcow2_target *tgt)
 
 static inline bool qcow2_file_is_writable(struct qcow2 *qcow2)
 {
-	return qcow2->file->f_mode & FMODE_WRITE;
+	return (qcow2->file->f_mode & FMODE_WRITE) &&
+	       (dm_table_get_mode(qcow2->tgt->ti->table) & BLK_OPEN_WRITE);
 }
 
 static inline void remap_to_clu(struct qcow2 *qcow2, struct qio *qio, loff_t clu_pos)
-- 
2.53.0

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

[Devel] [PATCH hci-8.1 2/3] drivers/md/dm-qcow2: fail writes to a read-only table

[Alexander Ivanov]

bio_check_ro() only warns about a write to a read-only disk and lets it
through. dm-qcow2 then allocates clusters in the top image although
the table is read-only. The header is not updated for a read-only
table, so enabled persistent bitmaps stay marked consistent while the
image data changes under them.

Kill write requests in .clone_and_map when the table is read-only.

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

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-qcow2-map.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 54b3a345f927..26fdfce09aa7 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -4506,6 +4506,11 @@ int qcow2_clone_and_map(struct dm_target *ti, struct request *rq,
 	struct qcow2_rq *qrq;
 	struct qio *qio;
 
+	/* The block layer only warns about writes to a read-only disk */
+	if (op_is_write(req_op(rq)) &&
+	    !(dm_table_get_mode(ti->table) & BLK_OPEN_WRITE))
+		return DM_MAPIO_KILL;
+
 	qrq = mempool_alloc(tgt->qrq_pool, GFP_ATOMIC);
 	if (!qrq)
 		return DM_MAPIO_KILL;
-- 
2.53.0

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

[Devel] [PATCH hci-8.1 3/3] drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps

[Alexander Ivanov]

qcow2_check_convert_hdr() ignores the autoclear features stored in the
image, and qcow2_parse_bitmap_ext() sets the bitmaps bit whenever no
bitmap has the auto flag. If the bit was already clear on disk, a
writable open sets it again and marks the bitmaps extension consistent.
The qcow2 spec requires to treat such an extension as inconsistent,
since a program without bitmap support might have modified the image.

Keep the bitmaps bit only if it is set in the image.

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

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-qcow2-target.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index 2d1e5d4e1b3a..0285427dd795 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -835,6 +835,7 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
 	struct file *file = qcow2->file;
 	loff_t ext_start, ext_end;
 	struct md_page *md;
+	u64 raw_autoclear;
 	int ret;
 
 	qcow2->file_size = i_size_read(file_inode(file));
@@ -865,6 +866,7 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
 		max_len = upper->hdr.size;
 	}
 	ret = qcow2_check_convert_hdr(ti, raw_hdr, hdr, min_len, max_len);
+	raw_autoclear = be64_to_cpu(raw_hdr->autoclear_features);
 	kunmap(md->page);
 	if (ret < 0)
 		goto out;
@@ -914,6 +916,9 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
 			goto out;
 		}
 	}
+	/* Bitmaps invalidated before must not become consistent again */
+	if (!(raw_autoclear & QCOW2_AUTOCLEAR_BITMAPS))
+		hdr->autoclear_features &= ~QCOW2_AUTOCLEAR_BITMAPS;
 	ret = 0;
 out:
 	return ret;
-- 
2.53.0

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

Re: [Devel] [PATCH hci-8.1 2/3] drivers/md/dm-qcow2: fail writes to a read-only table

[no-reply]

> bio_check_ro() only warns about a write to a read-only disk and lets it
> through. dm-qcow2 then allocates clusters in the top image although
> the table is read-only. The header is not updated for a read-only
> table, so enabled persistent bitmaps stay marked consistent while the
> image data changes under them.
> 
> Kill write requests in .clone_and_map when the table is read-only.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-138887
> 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/20261023132008.2304901-1-alexander.ivanov@virtuozzo.com?part=2


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

Re: [Devel] [PATCH hci-8.1 1/3] drivers/md/dm-qcow2: treat images of a read-only table as read-only

[no-reply]

> Whether an image may be modified is decided by the mode of its file
> only. Userspace may pass an O_RDWR top image to a read-only table, and
> then dm-qcow2 truncates the unaligned image tail on every .ctr and
> .preresume and considers the image writable on the I/O and discard
> paths.
> 
> merge_backward is also allowed on a read-only table if the lower image
> is opened for write. qcow2_set_image_file_features() checks the table
> mode and skips the lower image, so the merge writes to it without the
> dirty bit and without clearing the bitmaps autoclear bit. Enabled
> persistent bitmaps of the lower image are left marked consistent
> although the image data changed.
> 
> Consider an image writable only if both its file and the table are
> writable, and use this check for the header update, the tail truncate
> and the short read near EOF as well. merge_backward on a read-only
> table now fails with -EACCES.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-138887
> 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/20261023132008.2304901-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 3/3] drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps

[no-reply]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dm-qcow2: qcow2_parse_header() reads autoclear_features from v2 headers
--

commit 023e7aaebe247a5bb8465f4b52b55cce93eb8686
Author: Alexander Ivanov <alexander.ivanov@virtuozzo.com>

drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps

This patch makes qcow2_parse_header() read autoclear_features from the raw
on-disk header. If the bitmaps bit is clear in the image, it clears that bit
in the parsed header after the extensions are parsed. The goal is to stop a
writable open from marking an inconsistent bitmaps extension consistent
again.

Link: https://virtuozzo.atlassian.net/browse/VSTOR-138887

> diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
> --- a/drivers/md/dm-qcow2-target.c
> +++ b/drivers/md/dm-qcow2-target.c

[ ... ]

> @@ -865,6 +866,7 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
>  		max_len = upper->hdr.size;
>  	}
>  	ret = qcow2_check_convert_hdr(ti, raw_hdr, hdr, min_len, max_len);
> +	raw_autoclear = be64_to_cpu(raw_hdr->autoclear_features);
>  	kunmap(md->page);
>  	if (ret < 0)
>  		goto out;

[Severity: Low]
Is it correct to read autoclear_features from raw_hdr unconditionally here?

qcow2_check_convert_hdr() returns early for version 2 images:

    if (hdr->version == 2)
        return 0;

so it never parses or zeroes autoclear_features for them. For a v2 image,
the field is not part of the header, and the bytes at that offset belong to
the extension area or padding. The new code then treats those bytes as
autoclear feature flags.

The v3 case looks similar. header_length is only checked against
sizeof(struct QCowHeader) on the compression path. A v3 image with a short
header_length would also have the bytes at that offset read as autoclear
flags.

The new code can only clear QCOW2_AUTOCLEAR_BITMAPS, never set it, and the
read stays inside the mapped page. The effect is therefore small.

Would it be better to do the read and the mask only when hdr->version >= 3?
That could go inside qcow2_check_convert_hdr(), next to the existing
hdr->autoclear_features = 0.

[ ... ]

> @@ -914,6 +916,9 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
>  			goto out;
>  		}
>  	}
> +	/* Bitmaps invalidated before must not become consistent again */
> +	if (!(raw_autoclear & QCOW2_AUTOCLEAR_BITMAPS))
> +		hdr->autoclear_features &= ~QCOW2_AUTOCLEAR_BITMAPS;
>  	ret = 0;
>  out:
>  	return ret;

[Severity: Low]
For a v2 image, does this decision depend on a meaningless raw_autoclear
value?

The mask is applied based on whatever bytes sit at offset 88 of the header
page, rather than on a real autoclear_features field. It should be
conditional on hdr->version >= 3, as noted above.

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

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

Re: [Devel] [PATCH hci-8.1 3/3] drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps

[Alexander Ivanov]

>> @@ -865,6 +866,7 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
>>   		max_len = upper->hdr.size;
>>   	}
>>   	ret = qcow2_check_convert_hdr(ti, raw_hdr, hdr, min_len, max_len);
>> +	raw_autoclear = be64_to_cpu(raw_hdr->autoclear_features);
>>   	kunmap(md->page);
>>   	if (ret < 0)
>>   		goto out;
> [Severity: Low]
> Is it correct to read autoclear_features from raw_hdr unconditionally here?
>
> qcow2_check_convert_hdr() returns early for version 2 images:
>
>      if (hdr->version == 2)
>          return 0;
>
> so it never parses or zeroes autoclear_features for them. For a v2 image,
> the field is not part of the header, and the bytes at that offset belong to
> the extension area or padding. The new code then treats those bytes as
> autoclear feature flags.
>
> The v3 case looks similar. header_length is only checked against
> sizeof(struct QCowHeader) on the compression path. A v3 image with a short
> header_length would also have the bytes at that offset read as autoclear
> flags.
>
> The new code can only clear QCOW2_AUTOCLEAR_BITMAPS, never set it, and the
> read stays inside the mapped page. The effect is therefore small.
>
> Would it be better to do the read and the mask only when hdr->version >= 3?
> That could go inside qcow2_check_convert_hdr(), next to the existing
> hdr->autoclear_features = 0.
No change needed: for v2 the value is never written back to disk, and a v3
header shorter than 104 bytes is invalid per the spec. The code can only
clear the bitmaps bit, never set it, so in the worst case bitmaps are 
treated
as inconsistent, which is safe.
>
> [ ... ]
>
>> @@ -914,6 +916,9 @@ static int qcow2_parse_header(struct dm_target *ti, struct qcow2 *qcow2,
>>   			goto out;
>>   		}
>>   	}
>> +	/* Bitmaps invalidated before must not become consistent again */
>> +	if (!(raw_autoclear & QCOW2_AUTOCLEAR_BITMAPS))
>> +		hdr->autoclear_features &= ~QCOW2_AUTOCLEAR_BITMAPS;
>>   	ret = 0;
>>   out:
>>   	return ret;
> [Severity: Low]
> For a v2 image, does this decision depend on a meaningless raw_autoclear
> value?
>
> The mask is applied based on whatever bytes sit at offset 88 of the header
> page, rather than on a real autoclear_features field. It should be
> conditional on hdr->version >= 3, as noted above.
For v2 the value is never used: hdr.autoclear_features is consumed only
by qcow2_set_image_file_features(), which returns early for v2, so whatever
bytes sit at offset 88 have no effect.
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH hci-8.1 2/3] drivers/md/dm-qcow2: fail writes to a read-only table

[Andrey Zhadchenko]

On 10/23/26 15:20, Alexander Ivanov wrote:
> bio_check_ro() only warns about a write to a read-only disk and lets it
> through. dm-qcow2 then allocates clusters in the top image although
> the table is read-only. The header is not updated for a read-only
> table, so enabled persistent bitmaps stay marked consistent while the
> image data changes under them.
> 
> Kill write requests in .clone_and_map when the table is read-only.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-138887
> Feature: dm-qcow2: block device over QCOW2 files driver
> 
> Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
> ---
>   drivers/md/dm-qcow2-map.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
> index 54b3a345f927..26fdfce09aa7 100644
> --- a/drivers/md/dm-qcow2-map.c
> +++ b/drivers/md/dm-qcow2-map.c
> @@ -4506,6 +4506,11 @@ int qcow2_clone_and_map(struct dm_target *ti, struct request *rq,
>   	struct qcow2_rq *qrq;
>   	struct qio *qio;
>   
> +	/* The block layer only warns about writes to a read-only disk */

Let's not litter the code with the useless comments. Killing write 
request on non-write device does not need any extra info.

> +	if (op_is_write(req_op(rq)) &&
> +	    !(dm_table_get_mode(ti->table) & BLK_OPEN_WRITE))
> +		return DM_MAPIO_KILL;
> +
>   	qrq = mempool_alloc(tgt->qrq_pool, GFP_ATOMIC);
>   	if (!qrq)
>   		return DM_MAPIO_KILL;

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

Re: [Devel] [PATCH hci-8.1 0/3] dm-qcow2: keep images of a read-only table unmodified

[Andrey Zhadchenko]

We probably don't need an extra respin to delete the single comment. 
@khorenko could you please remove it from patch 2/3 before applying?

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

On 10/23/26 15:20, Alexander Ivanov wrote:
> A read-only dm-qcow2 table does not protect its images. Writes to the
> device pass through bio_check_ro() with a warning and allocate clusters
> in the top image, an O_RDWR top image is truncated on .ctr and
> .preresume, and merge_backward modifies the lower image. Since the
> header is not updated for a read-only table, persistent bitmaps stay
> marked consistent while the data under them changes.
> 
> Patch 1 makes an image writable only if both its file and the table
> are writable. Patch 2 fails writes to a read-only table. Patch 3 keeps
> the bitmaps autoclear bit clear if it is already clear in the image,
> so that a later writable open does not revalidate stale bitmaps.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-138887
> 
> Alexander Ivanov (3):
>    drivers/md/dm-qcow2: treat images of a read-only table as read-only
>    drivers/md/dm-qcow2: fail writes to a read-only table
>    drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps
> 
>   drivers/md/dm-qcow2-map.c    |  9 ++++++---
>   drivers/md/dm-qcow2-target.c | 11 +++++++----
>   drivers/md/dm-qcow2.h        |  3 ++-
>   3 files changed, 15 insertions(+), 8 deletions(-)
> 

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

Re: [Devel] [PATCH hci-8.1 1/3] drivers/md/dm-qcow2: treat images of a read-only table as read-only

[Konstantin Khorenko]

> then dm-qcow2 truncates the unaligned image tail on every .ctr and
> .preresume and considers the image writable on the I/O and discard
> paths.

then dm-qcow2 pads the unaligned image tail on .ctr and .preresume and
considers the image writable on the discard and merge paths.

qcow2_truncate_safe() extends the file to PAGE_ALIGN(file_size), it
cuts nothing, and only while the tail is unaligned. The plain write
path never consults qcow2_file_is_writable(), it goes through
qio_may_modify_image(), which is true for every non-merge qio; the
helper gates only discard, prepare_backward_merge(), the merge
read-ahead and the header/padding sites.

> -             mode = qcow2->file->f_mode;
> -             if (pos == qcow2->file_size && !(mode & FMODE_WRITE)) {
> +             if (pos == qcow2->file_size && !qcow2_file_is_writable(qcow2)) {

Optional, while you are here: the comment above, "Read near EOF? See
qcow2_attach_file()", points at the wrong function. The padding that
makes a short read possible is in qcow2_parse_header().

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team


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

Re: [Devel] [PATCH hci-8.1 3/3] drivers/md/dm-qcow2: do not revalidate inconsistent persistent bitmaps

[Konstantin Khorenko]

> Keep the bitmaps bit only if it is set in the image.

Please add

Fixes: f913aa3f7a1d4 ("dm-qcow2: parse bitmap extension")

That commit replaced the raw read of autoclear_features with the
zero-then-set logic, so it is where a clear on-disk bit started to be
set again.

On the Sashiko thread: agreed with your reply, nothing to change here.
The driver does not check header_length >= 104 for v3 the way qemu
does; that is a separate one-liner, not for this series.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team


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

Re: [Devel] [PATCH hci-8.1 0/3] dm-qcow2: keep images of a read-only table unmodified

[Konstantin Khorenko]

Three things found while reviewing the series, none of them for it.
Please fix them separately.

- A v3 image with header_length < 104 is not rejected. The only check
  is header_length < sizeof(struct QCowHeader), and only when the
  compression bit is set. qemu fails such an image with "qcow2 header
  too short". For us the extension parser then starts inside the
  header.

- For a v2 image hdr->header_length is never set, so it stays 0 from
  kzalloc() and the extensions are parsed from offset 0 of the header
  instead of 72. qemu sets header_length = 72 for v2.

- The table mode is still tested against FMODE_WRITE in
  qcow2_check_convert_hdr(), qcow2_attach_file(),
  qcow2_truncate_preallocations(), qcow2_postsuspend() and
  qcow2_preresume(), while the series switched to BLK_OPEN_WRITE. Same
  value today, but dm_table_get_mode() returns blk_mode_t. The is_ro
  in qcow2_check_convert_hdr() has only a commented-out user and can go.


Please file appropriate tasks and fix them separately.

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team


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

Re: [Devel] [PATCH hci-8.1 0/3] dm-qcow2: keep images of a read-only table unmodified

[Alexander Ivanov]

On 10/29/26 12:42, Konstantin Khorenko wrote:
> Three things found while reviewing the series, none of them for it.
> Please fix them separately.
>
> - A v3 image with header_length < 104 is not rejected. The only check
>    is header_length < sizeof(struct QCowHeader), and only when the
>    compression bit is set. qemu fails such an image with "qcow2 header
>    too short". For us the extension parser then starts inside the
>    header.
>
> - For a v2 image hdr->header_length is never set, so it stays 0 from
>    kzalloc() and the extensions are parsed from offset 0 of the header
>    instead of 72. qemu sets header_length = 72 for v2.
>
> - The table mode is still tested against FMODE_WRITE in
>    qcow2_check_convert_hdr(), qcow2_attach_file(),
>    qcow2_truncate_preallocations(), qcow2_postsuspend() and
>    qcow2_preresume(), while the series switched to BLK_OPEN_WRITE. Same
>    value today, but dm_table_get_mode() returns blk_mode_t. The is_ro
>    in qcow2_check_convert_hdr() has only a commented-out user and can go.
>
>
> Please file appropriate tasks and fix them separately.

Created tasks:
https://virtuozzo.atlassian.net/browse/VSTOR-152393

https://virtuozzo.atlassian.net/browse/VSTOR-152394

https://virtuozzo.atlassian.net/browse/VSTOR-152395

> --
> Best regards,
>
> Konstantin Khorenko,
> Virtuozzo Linux Kernel Team
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel