Re: [Qemu-devel] [PATCH v20 10/30] block/dirty-bitmap: add readonly field to BdrvDirtyBitmap

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2172 bytes --]

On 2017-06-02 13:21, Vladimir Sementsov-Ogievskiy wrote:
> It will be needed in following commits for persistent bitmaps.
> If bitmap is loaded from read-only storage (and we can't mark it
> "in use" in this storage) corresponding BdrvDirtyBitmap should be
> read-only.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  block/dirty-bitmap.c         | 32 ++++++++++++++++++++++++++++++++
>  block/io.c                   |  8 ++++++++
>  blockdev.c                   |  6 ++++++
>  include/block/dirty-bitmap.h |  4 ++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index f25428868c..1c9ffb292a 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -45,6 +45,12 @@ struct BdrvDirtyBitmap {
>      bool disabled;              /* Bitmap is disabled. It skips all writes to
>                                     the device */
>      int active_iterators;       /* How many iterators are active */
> +    bool readonly;              /* Bitmap is read-only and may be changed only
> +                                   by deserialize* functions. This field blocks
> +                                   any changing operations on owning image
> +                                   (writes and discards), if bitmap is readonly
> +                                   such operations must fail and not change
> +                                   image or this bitmap */

"This fields blocks any modifying operations on the respective image
(writes and discards). If the bitmap is read-only, such operations must
fail and not change the image or this bitmap."

Or, what I'd think to be more natural:

"This fields also prevents the respective image from being modified
(i.e. blocks writes and discards). Such operations must fail and both
the image and this bitmap must remain unchanged while this flag is set."

Sorry to be so fussy about grammar and the like, but I'd just like the
comments which land in the code to be easily and clearly understandable...

But just nit picking, so:

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]