On 30.09.2016 12:53, Vladimir Sementsov-Ogievskiy wrote: > This flag means that the bitmap is now in use by the software or was not > successfully saved. In any way, with this flag set the bitmap data must > be considered inconsistent and should not be loaded. > > With current implementation this flag is never set, as we just remove > bitmaps from the image after loading. But it defined in qcow2 spec and > must be handled. Also, it can be used in future, if async schemes of > bitmap loading/saving are implemented. > > We also remove in-use bitmaps from the image on open. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > block/qcow2-bitmap.c | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-)
Don't you want to make use of this flag? It would appear useful to me if
you just marked autoload bitmaps as in_use instead of deleting them from
the image when it's opened and then overwrite them when the image is closed.
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index a5be25a..8cf40f0 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -28,6 +28,7 @@
> #include "qemu/osdep.h"
> #include "qapi/error.h"
> #include "qemu/cutils.h"
> +#include "exec/log.h"
>
> #include "block/block_int.h"
> #include "block/qcow2.h"
> @@ -44,7 +45,8 @@
> #define BME_MAX_NAME_SIZE 1023
>
> /* Bitmap directory entry flags */
> -#define BME_RESERVED_FLAGS 0xfffffffd
> +#define BME_RESERVED_FLAGS 0xfffffffc
> +#define BME_FLAG_IN_USE 1
This should be (1U << 0) to be consistent with the definition of
BME_FLAG_AUTO.
> #define BME_FLAG_AUTO (1U << 1)
>
> /* bits [1, 8] U [56, 63] are reserved */
> @@ -287,6 +289,11 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
> BdrvDirtyBitmap *bitmap = NULL;
> char *name = g_strndup(dir_entry_name_notcstr(entry), entry->name_size);
>
> + if (entry->flags & BME_FLAG_IN_USE) {
> + error_setg(errp, "Bitmap '%s' is in use", name);
> + goto fail;
> + }
> +
> ret = bitmap_table_load(bs, entry, &bitmap_table);
> if (ret < 0) {
> error_setg_errno(errp, -ret,
> @@ -533,6 +540,14 @@ int qcow2_read_bitmaps(BlockDriverState *bs, Error
> **errp)
> for_each_bitmap_dir_entry(e, dir, dir_size) {
> uint32_t fl = e->flags;
>
> + if (fl & BME_FLAG_IN_USE) {
> + qemu_log("qcow2 warning: "
> + "removing in_use bitmap '%.*s' for image %s.\n",
> + e->name_size, (char *)(e + 1),
> bdrv_get_device_or_node_name(bs));
I'm not sure whether qemu_log() is the right way to do this. I think
fprintf() to stderr might actually be more appropriate. qemu_log() looks
like it's mostly used for debugging purposes.
Also, this is not a warning. You are just doing it. You are not warning
someone about a cliff when he's already fallen off.
But you can actually make it a warning: Just leave the bitmap in the
image (maybe someone can do something with it?) and instead let qemu-img
check clean it up. The warning should then just be "Warning: Ignoring
in_use bitmap %.*s, use qemu-img check -r all to delete it".
Max
> +
> + continue;
> + }
> +
> if (fl & BME_FLAG_AUTO) {
> BdrvDirtyBitmap *bitmap = load_bitmap(bs, e, errp);
> if (bitmap == NULL) {
>
signature.asc
Description: OpenPGP digital signature
