Re: [Qemu-devel] [PATCH 13/24] qcow2: add .bdrv_store_persistent_dirty_bitmaps()

[Max Reitz]

On 23.01.2017 13:10, Vladimir Sementsov-Ogievskiy wrote:
> Realize block bitmap storing interface, to allow qcow2 images store
> persistent bitmaps.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  block/qcow2-bitmap.c | 509 
> +++++++++++++++++++++++++++++++++++++++++++++++++--
>  block/qcow2.c        |   1 +
>  block/qcow2.h        |   1 +
>  3 files changed, 496 insertions(+), 15 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 3e8fd030d9..a2b857ea72 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -28,6 +28,7 @@
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "exec/log.h"
> +#include "qemu/cutils.h"
>  
>  #include "block/block_int.h"
>  #include "block/qcow2.h"
> @@ -43,6 +44,10 @@
>  #define BME_MIN_GRANULARITY_BITS 9
>  #define BME_MAX_NAME_SIZE 1023
>  
> +#if BME_MAX_TABLE_SIZE * 8ULL > INT_MAX
> +#error In the code bitmap table physical size assumed to fit into int
> +#endif
> +
>  /* Bitmap directory entry flags */
>  #define BME_RESERVED_FLAGS 0xfffffffcU
>  #define BME_FLAG_IN_USE 1
> @@ -67,13 +72,21 @@ typedef struct QEMU_PACKED Qcow2BitmapDirEntry {
>      /* name follows  */
>  } Qcow2BitmapDirEntry;
>  
> +typedef struct Qcow2BitmapTable {
> +    uint64_t offset;
> +    uint32_t size; /* number of 64bit entries */
> +    QSIMPLEQ_ENTRY(Qcow2BitmapTable) entry;
> +} Qcow2BitmapTable;
> +typedef QSIMPLEQ_HEAD(Qcow2BitmapTableList, Qcow2BitmapTable) 
> Qcow2BitmapTableList;
> +
>  typedef struct Qcow2Bitmap {
> -    uint64_t table_offset;
> -    uint32_t table_size;
> +    Qcow2BitmapTable table;

This doesn't hurt my reviewing too much, but it wouldn't make sense to
new reviewers. This should have been structured like this from the
beginning (i.e. patch 8).

Anyway, I don't mind and as long as nobody other starts reviewing, it's
fine. In the end, it'll be the same code anyway.

Reviewed-by: Max Reitz <address@hidden>

signature.asc
Description: OpenPGP digital signature