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 <vsementsov@virtuozzo.com>
> ---
> 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 <mreitz@redhat.com>
> + 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) {