Re: [PATCH 07/14] qcow2: simple case support for downgrading of qcow2 images with zstd

2021-07-16 Thread Max Reitz

On 05.07.21 11:15, Vladimir Sementsov-Ogievskiy wrote:

If image doesn't have any compressed cluster we can easily switch to
zlib compression, which may allow to downgrade the image.

That's mostly needed to support IMGOPTS='compression_type=zstd' in some
iotests which do qcow2 downgrade.

While being here also fix checkpatch complain against '#' in printf
formatting.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/qcow2.c | 58 +--
  1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ee4530cdbd..bed3354474 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5221,6 +5221,38 @@ static int qcow2_load_vmstate(BlockDriverState *bs, 
QEMUIOVector *qiov,
  qiov->size, qiov, 0, 0);
  }
  
+static int qcow2_has_compressed_clusters(BlockDriverState *bs)

+{
+int64_t offset = 0;
+int64_t bytes = bdrv_getlength(bs);
+
+if (bytes < 0) {
+return bytes;
+}
+
+while (bytes != 0) {
+int ret;
+QCow2SubclusterType type;
+unsigned int cur_bytes = MIN(INT_MAX, bytes);
+uint64_t host_offset;
+
+ret = qcow2_get_host_offset(bs, offset, &cur_bytes, &host_offset,
+&type);
+if (ret < 0) {
+return ret;
+}
+
+if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
+return 1;
+}
+
+offset += cur_bytes;
+bytes -= cur_bytes;
+}
+
+return 0;
+}
+
  /*
   * Downgrades an image's version. To achieve this, any incompatible features
   * have to be removed.
@@ -5278,9 +5310,10 @@ static int qcow2_downgrade(BlockDriverState *bs, int 
target_version,
   * the first place; if that happens nonetheless, returning -ENOTSUP is the
   * best thing to do anyway */
  
-if (s->incompatible_features) {

+if (s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION) {
  error_setg(errp, "Cannot downgrade an image with incompatible features 
"
-   "%#" PRIx64 " set", s->incompatible_features);
+   "0x%" PRIx64 " set",
+   s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION);
  return -ENOTSUP;
  }
  
@@ -5298,6 +5331,27 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,

  return ret;
  }
  
+if (s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION) {

+ret = qcow2_has_compressed_clusters(bs);
+if (ret < 0) {
+error_setg(errp, "Failed to check block status");
+return -EINVAL;
+}
+if (ret) {
+error_setg(errp, "Cannot downgrade an image with zstd compression "


Perhaps s/zstd/non-zlib/?

Like, really “perhaps”.  Right now I think this is the better error 
message, it’s just that “non-zlib” is more technically correct and 
theoretically future-proof.



+   "type and existing compressed clusters");
+return -ENOTSUP;
+}
+/*
+ * No compressed clusters for now, so just chose default zlib
+ * compression.
+ */
+s->incompatible_features = 0;


Not wrong, though I’d prefer

s->incompatible_features &= ~QCOW2_INCOMPAT_COMPRESSION;

Anyway:

Reviewed-by: Max Reitz 


+s->compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
+}
+
+assert(s->incompatible_features == 0);
+
  s->qcow_version = target_version;
  ret = qcow2_update_header(bs);
  if (ret < 0) {





[PATCH 07/14] qcow2: simple case support for downgrading of qcow2 images with zstd

2021-07-05 Thread Vladimir Sementsov-Ogievskiy
If image doesn't have any compressed cluster we can easily switch to
zlib compression, which may allow to downgrade the image.

That's mostly needed to support IMGOPTS='compression_type=zstd' in some
iotests which do qcow2 downgrade.

While being here also fix checkpatch complain against '#' in printf
formatting.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 block/qcow2.c | 58 +--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ee4530cdbd..bed3354474 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5221,6 +5221,38 @@ static int qcow2_load_vmstate(BlockDriverState *bs, 
QEMUIOVector *qiov,
 qiov->size, qiov, 0, 0);
 }
 
+static int qcow2_has_compressed_clusters(BlockDriverState *bs)
+{
+int64_t offset = 0;
+int64_t bytes = bdrv_getlength(bs);
+
+if (bytes < 0) {
+return bytes;
+}
+
+while (bytes != 0) {
+int ret;
+QCow2SubclusterType type;
+unsigned int cur_bytes = MIN(INT_MAX, bytes);
+uint64_t host_offset;
+
+ret = qcow2_get_host_offset(bs, offset, &cur_bytes, &host_offset,
+&type);
+if (ret < 0) {
+return ret;
+}
+
+if (type == QCOW2_SUBCLUSTER_COMPRESSED) {
+return 1;
+}
+
+offset += cur_bytes;
+bytes -= cur_bytes;
+}
+
+return 0;
+}
+
 /*
  * Downgrades an image's version. To achieve this, any incompatible features
  * have to be removed.
@@ -5278,9 +5310,10 @@ static int qcow2_downgrade(BlockDriverState *bs, int 
target_version,
  * the first place; if that happens nonetheless, returning -ENOTSUP is the
  * best thing to do anyway */
 
-if (s->incompatible_features) {
+if (s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION) {
 error_setg(errp, "Cannot downgrade an image with incompatible features 
"
-   "%#" PRIx64 " set", s->incompatible_features);
+   "0x%" PRIx64 " set",
+   s->incompatible_features & ~QCOW2_INCOMPAT_COMPRESSION);
 return -ENOTSUP;
 }
 
@@ -5298,6 +5331,27 @@ static int qcow2_downgrade(BlockDriverState *bs, int 
target_version,
 return ret;
 }
 
+if (s->incompatible_features & QCOW2_INCOMPAT_COMPRESSION) {
+ret = qcow2_has_compressed_clusters(bs);
+if (ret < 0) {
+error_setg(errp, "Failed to check block status");
+return -EINVAL;
+}
+if (ret) {
+error_setg(errp, "Cannot downgrade an image with zstd compression "
+   "type and existing compressed clusters");
+return -ENOTSUP;
+}
+/*
+ * No compressed clusters for now, so just chose default zlib
+ * compression.
+ */
+s->incompatible_features = 0;
+s->compression_type = QCOW2_COMPRESSION_TYPE_ZLIB;
+}
+
+assert(s->incompatible_features == 0);
+
 s->qcow_version = target_version;
 ret = qcow2_update_header(bs);
 if (ret < 0) {
-- 
2.29.2