On 02/03/2017 04:40 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add bitmap extension as specified in docs/specs/qcow2.txt.
> For now, just mirror extension header into Qcow2 state and check
> constraints.
>
> For now, disable image resize if it has bitmaps. It will be fixed later.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
> block/qcow2.c | 127
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> block/qcow2.h | 24 +++++++++++
> 2 files changed, 145 insertions(+), 6 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 96fb8a8f16..d263ab1ed7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -63,6 +63,7 @@ typedef struct {
> #define QCOW2_EXT_MAGIC_END 0
> #define QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
> #define QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
> +#define QCOW2_EXT_MAGIC_BITMAPS 0x23852875
>
> static int qcow2_probe(const uint8_t *buf, int buf_size, const char
> *filename)
> {
> @@ -86,12 +87,17 @@ static int qcow2_probe(const uint8_t *buf, int buf_size,
> const char *filename)
> */
> static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> uint64_t end_offset, void **p_feature_table,
> - Error **errp)
> + bool *need_update_header, Error **errp)
> {
> BDRVQcow2State *s = bs->opaque;
> QCowExtension ext;
> uint64_t offset;
> int ret;
> + Qcow2BitmapHeaderExt bitmaps_ext;
> +
> + if (need_update_header != NULL) {
> + *need_update_header = false;
> + }
>
> #ifdef DEBUG_EXT
> printf("qcow2_read_extensions: start=%ld end=%ld\n", start_offset,
> end_offset);
> @@ -162,6 +168,85 @@ static int qcow2_read_extensions(BlockDriverState *bs,
> uint64_t start_offset,
> }
> break;
>
> + case QCOW2_EXT_MAGIC_BITMAPS:
> + if (ext.len != sizeof(bitmaps_ext)) {
> + error_setg_errno(errp, -ret, "bitmaps_ext: "
> + "Invalid extension length");
> + return -EINVAL;
> + }
> +
> + if (!(s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS)) {
> + fprintf(stderr,
> + "WARNING: a program lacking bitmap support modified "
> + "this file, so all bitmaps are now considered "
> + "inconsistent. Some clusters may be leaked, run "
> + "'qemu-img check -r' on the image file to fix.");
> + if (need_update_header != NULL) {
> + *need_update_header = true;
> + }
> + break;
> + }
> +
> + ret = bdrv_pread(bs->file, offset, &bitmaps_ext, ext.len);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "bitmaps_ext: "
> + "Could not read ext header");
> + return ret;
> + }
> +
> + if (bitmaps_ext.reserved32 != 0) {
> + error_setg_errno(errp, -ret, "bitmaps_ext: "
> + "Reserved field is not zero");
> + return -EINVAL;
> + }
> +
> + be32_to_cpus(&bitmaps_ext.nb_bitmaps);
> + be64_to_cpus(&bitmaps_ext.bitmap_directory_size);
> + be64_to_cpus(&bitmaps_ext.bitmap_directory_offset);
> +
> + if (bitmaps_ext.nb_bitmaps > QCOW2_MAX_BITMAPS) {
> + error_setg(errp,
> + "bitmaps_ext: File %s has %" PRIu32 " bitmaps, "
> + "exceeding the QEMU supported maximum of %d",
> + bs->filename, bitmaps_ext.nb_bitmaps,
> + QCOW2_MAX_BITMAPS);
> + return -EINVAL;
> + }
> +
> + if (bitmaps_ext.nb_bitmaps == 0) {
> + error_setg(errp, "found bitmaps extension with zero
> bitmaps");
> + return -EINVAL;
> + }
> +
> + if (bitmaps_ext.bitmap_directory_offset & (s->cluster_size - 1))
> {
> + error_setg(errp, "bitmaps_ext: "
> + "invalid bitmap directory offset");
> + return -EINVAL;
> + }
> +
> + if (bitmaps_ext.bitmap_directory_size >
> + QCOW2_MAX_BITMAP_DIRECTORY_SIZE) {
> + error_setg(errp, "bitmaps_ext: "
> + "bitmap directory size (%" PRIu64 ")
> exceeds "
> + "the maximum supported size (%d)",
> + bitmaps_ext.bitmap_directory_size,
> + QCOW2_MAX_BITMAP_DIRECTORY_SIZE);
> + return -EINVAL;
> + }
> +
> + s->nb_bitmaps = bitmaps_ext.nb_bitmaps;
> + s->bitmap_directory_offset =
> + bitmaps_ext.bitmap_directory_offset;
> + s->bitmap_directory_size =
> + bitmaps_ext.bitmap_directory_size;
> +
> +#ifdef DEBUG_EXT
> + printf("Qcow2: Got bitmaps extension: "
> + "offset=%" PRIu64 " nb_bitmaps=%" PRIu32 "\n",
> + s->bitmap_directory_offset, s->nb_bitmaps);
> +#endif
> + break;
> +
> default:
> /* unknown magic - save it in case we need to rewrite the header
> */
> {
> @@ -824,6 +909,7 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> Error *local_err = NULL;
> uint64_t ext_end;
> uint64_t l1_vm_state_index;
> + bool need_update_header = false;
>
> ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
> if (ret < 0) {
> @@ -929,7 +1015,7 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> if (s->incompatible_features & ~QCOW2_INCOMPAT_MASK) {
> void *feature_table = NULL;
> qcow2_read_extensions(bs, header.header_length, ext_end,
> - &feature_table, NULL);
> + &feature_table, NULL, NULL);
> report_unsupported_feature(errp, feature_table,
> s->incompatible_features &
> ~QCOW2_INCOMPAT_MASK);
> @@ -1116,7 +1202,7 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
>
> /* read qcow2 extensions */
> if (qcow2_read_extensions(bs, header.header_length, ext_end, NULL,
> - &local_err)) {
> + &need_update_header, &local_err)) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
> goto fail;
> @@ -1151,9 +1237,13 @@ static int qcow2_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto fail;
> }
>
> - /* Clear unknown autoclear feature bits */
> - if (!bs->read_only && !(flags & BDRV_O_INACTIVE) &&
> s->autoclear_features) {
> - s->autoclear_features = 0;
> + if (!need_update_header) {
> + /* Clear unknown autoclear feature bits */
> + need_update_header = s->autoclear_features & ~QCOW2_AUTOCLEAR_MASK;
> + }
> +
> + if (need_update_header && !bs->read_only && !(flags & BDRV_O_INACTIVE)) {
> + s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
> ret = qcow2_update_header(bs);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Could not update qcow2 header");
> @@ -1953,6 +2043,24 @@ int qcow2_update_header(BlockDriverState *bs)
> buflen -= ret;
> }
>
> + if (s->nb_bitmaps > 0) {
> + Qcow2BitmapHeaderExt bitmaps_header = {
> + .nb_bitmaps = cpu_to_be32(s->nb_bitmaps),
> + .bitmap_directory_size =
> + cpu_to_be64(s->bitmap_directory_size),
> + .bitmap_directory_offset =
> + cpu_to_be64(s->bitmap_directory_offset)
> + };
> + ret = header_ext_add(buf, QCOW2_EXT_MAGIC_BITMAPS,
> + &bitmaps_header, sizeof(bitmaps_header),
> + buflen);
> + if (ret < 0) {
> + goto fail;
> + }
> + buf += ret;
> + buflen -= ret;
> + }
> +
> /* Keep unknown header extensions */
> QLIST_FOREACH(uext, &s->unknown_header_ext, next) {
> ret = header_ext_add(buf, uext->magic, uext->data, uext->len,
> buflen);
> @@ -2528,6 +2636,13 @@ static int qcow2_truncate(BlockDriverState *bs,
> int64_t offset)
> return -ENOTSUP;
> }
>
> + /* cannot proceed if image has bitmaps */
> + if (s->nb_bitmaps) {
> + /* TODO: resize bitmaps in the image */
> + error_report("Can't resize an image which has bitmaps");
> + return -ENOTSUP;
> + }
> +
> /* shrinking is currently not supported */
> if (offset < bs->total_sectors * 512) {
> error_report("qcow2 doesn't support shrinking images yet");
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 182341483a..861b5011dd 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -52,6 +52,10 @@
> * space for snapshot names and IDs */
> #define QCOW_MAX_SNAPSHOTS_SIZE (1024 * QCOW_MAX_SNAPSHOTS)
>
> +/* Bitmap header extension constraints */
> +#define QCOW2_MAX_BITMAPS 65535
> +#define QCOW2_MAX_BITMAP_DIRECTORY_SIZE (1024 * QCOW2_MAX_BITMAPS)
> +
> /* indicate that the refcount of the referenced cluster is exactly one. */
> #define QCOW_OFLAG_COPIED (1ULL << 63)
> /* indicate that the cluster is compressed (they never have the copied flag)
> */
> @@ -195,6 +199,15 @@ enum {
> QCOW2_COMPAT_FEAT_MASK = QCOW2_COMPAT_LAZY_REFCOUNTS,
> };
>
> +/* Autoclear feature bits */
> +enum {
> + QCOW2_AUTOCLEAR_BITMAPS_BITNR = 0,
> + QCOW2_AUTOCLEAR_BITMAPS =
> + 1 << QCOW2_AUTOCLEAR_BITMAPS_BITNR,
> +
> + QCOW2_AUTOCLEAR_MASK = QCOW2_AUTOCLEAR_BITMAPS,
> +};
> +
> enum qcow2_discard_type {
> QCOW2_DISCARD_NEVER = 0,
> QCOW2_DISCARD_ALWAYS,
> @@ -222,6 +235,13 @@ typedef uint64_t Qcow2GetRefcountFunc(const void
> *refcount_array,
> typedef void Qcow2SetRefcountFunc(void *refcount_array,
> uint64_t index, uint64_t value);
>
> +typedef struct Qcow2BitmapHeaderExt {
> + uint32_t nb_bitmaps;
> + uint32_t reserved32;
> + uint64_t bitmap_directory_size;
> + uint64_t bitmap_directory_offset;
> +} QEMU_PACKED Qcow2BitmapHeaderExt;
> +
> typedef struct BDRVQcow2State {
> int cluster_bits;
> int cluster_size;
> @@ -263,6 +283,10 @@ typedef struct BDRVQcow2State {
> unsigned int nb_snapshots;
> QCowSnapshot *snapshots;
>
> + uint32_t nb_bitmaps;
> + uint64_t bitmap_directory_size;
> + uint64_t bitmap_directory_offset;
> +
> int flags;
> int qcow_version;
> bool use_lazy_refcounts;
>
Reviewed-by: John Snow <js...@redhat.com>
