Re: [PATCH v2 5/6] block/dirty-bitmaps: unify qmp_locked and user_locked calls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



14.02.2019 2:23, John Snow wrote:
> These mean the same thing now. Unify them and rename the merged call
> bdrv_dirty_bitmap_busy to indicate semantically what we are describing,
> as well as help disambiguate from the various _locked and _unlocked
> versions of bitmap helpers that refer to mutex locks.
> 
> Signed-off-by: John Snow <jsnow@xxxxxxxxxx>
> Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>
> ---
>   block/dirty-bitmap.c           | 41 +++++++++++++++-------------------
>   blockdev.c                     | 18 +++++++--------
>   include/block/dirty-bitmap.h   |  5 ++---
>   migration/block-dirty-bitmap.c |  6 ++---
>   nbd/server.c                   |  6 ++---
>   5 files changed, 35 insertions(+), 41 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 2042c62602..8ab048385a 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -48,9 +48,9 @@ struct BdrvDirtyBitmap {
>       QemuMutex *mutex;
>       HBitmap *bitmap;            /* Dirty bitmap implementation */
>       HBitmap *meta;              /* Meta dirty bitmap */
> -    bool qmp_locked;            /* Bitmap is locked, it can't be modified
> -                                   through QMP */
> -    BdrvDirtyBitmap *successor; /* Anonymous child; implies user_locked state */
> +    bool busy;                  /* Bitmap is busy, it can't be modified through
> +                                   QMP */

better not "modified" but "used".. for example, export through NBD is not a modification.

> +    BdrvDirtyBitmap *successor; /* Anonymous child, if any. */

hm this comment change about successor relates more to previous patch, but I don't really care.

>       char *name;                 /* Optional non-empty unique ID */
>       int64_t size;               /* Size of the bitmap, in bytes */
>       bool disabled;              /* Bitmap is disabled. It ignores all writes to
> @@ -188,22 +188,17 @@ bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap)
>       return bitmap->successor;
>   }
>   


In comment for bdrv_dirty_bitmap_create_successor, there is "locked" word, which you forget to fix to "busy"
with at least this fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx>


-- 
Best regards,
Vladimir


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux