Re: [Qemu-block] [PATCH v2 6/6] block/dirty-bitmaps: move comment block

[Vladimir Sementsov-Ogievskiy]

14.02.2019 2:23, John Snow wrote:
> Simply move the big status enum comment block to above the status
> function, and document it as being deprecated. The whole confusing
> block can get deleted in three releases time.
> 
> Signed-off-by: John Snow <address@hidden>


Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>

But I think, it's OK to remove it now. It mostly unrelated to code now, so,
is it needed? And it is unrelated to deprecation. As I understand, user-seen
information is DirtyBitmapStatus declaration in qapi, and it is deprecated
and to be removed in three releases.

> ---
>   block/dirty-bitmap.c | 36 +++++++++++++++++++-----------------
>   1 file changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 8ab048385a..fc390cae94 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -28,22 +28,6 @@
>   #include "block/block_int.h"
>   #include "block/blockjob.h"
>   
> -/**
> - * A BdrvDirtyBitmap can be in four possible user-visible states:
> - * (1) Active:   successor is NULL, and disabled is false: full r/w mode
> - * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
> - *               guest writes are dropped, but monitor writes are possible,
> - *               through commands like merge and clear.
> - * (3) Frozen:   successor is not NULL.
> - *               A frozen bitmap cannot be renamed, deleted, cleared, set,
> - *               enabled, merged to, etc. A frozen bitmap can only abdicate()
> - *               or reclaim().
> - *               In this state, the anonymous successor bitmap may be either
> - *               Active and recording writes from the guest (e.g. backup 
> jobs),
> - *               but it can be Disabled and not recording writes.
> - * (4) Locked:   Whether Active or Disabled, the user cannot modify this 
> bitmap
> - *               in any way from the monitor.
> - */
>   struct BdrvDirtyBitmap {
>       QemuMutex *mutex;
>       HBitmap *bitmap;            /* Dirty bitmap implementation */
> @@ -205,7 +189,25 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
>       return !bitmap->disabled;
>   }
>   
> -/* Called with BQL taken.  */
> +/**
> + * bdrv_dirty_bitmap_status: This API is now deprecated.
> + * Called with BQL taken.
> + *
> + * A BdrvDirtyBitmap can be in four possible user-visible states:
> + * (1) Active:   successor is NULL, and disabled is false: full r/w mode
> + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
> + *               guest writes are dropped, but monitor writes are possible,
> + *               through commands like merge and clear.
> + * (3) Frozen:   successor is not NULL.
> + *               A frozen bitmap cannot be renamed, deleted, cleared, set,
> + *               enabled, merged to, etc. A frozen bitmap can only abdicate()
> + *               or reclaim().
> + *               In this state, the anonymous successor bitmap may be either
> + *               Active and recording writes from the guest (e.g. backup 
> jobs),
> + *               but it can be Disabled and not recording writes.
> + * (4) Locked:   Whether Active or Disabled, the user cannot modify this 
> bitmap
> + *               in any way from the monitor.
> + */
>   DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
>   {
>       if (bdrv_dirty_bitmap_has_successor(bitmap)) {
> 


-- 
Best regards,
Vladimir