| Message ID | 20210705091549.178335-8-vsementsov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: support zstd | expand |
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) {
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) {
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(-)