[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 3/7] block/dirty-bitmaps: add block_dirty_bit
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v3 3/7] block/dirty-bitmaps: add block_dirty_bitmap_check function |
Date: |
Fri, 1 Mar 2019 13:36:53 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 |
On 3/1/19 1:15 PM, John Snow wrote:
> Instead of checking against busy, inconsistent, or read only directly,
> use a check function with permissions bits that let us streamline the
> checks without reproducing them in many places.
>
> Included in this patch are permissions changes that simply add the
> inconsistent check to existing permissions call spots, without
> addressing existing bugs.
>
> In general, this means that busy+readonly checks become BDRV_BITMAP_DEFAULT,
> which checks against all three conditions. busy-only checks become
> BDRV_BITMAP_ALLOW_RO.
>
> Notably, remove allows inconsistent bitmaps, so it doesn't follow the pattern.
>
> Signed-off-by: John Snow <address@hidden>
> ---
> include/block/dirty-bitmap.h | 13 ++++++++-
> block/dirty-bitmap.c | 38 +++++++++++++++++++-------
> blockdev.c | 49 +++++++---------------------------
> migration/block-dirty-bitmap.c | 12 +++------
> nbd/server.c | 3 +--
> 5 files changed, 54 insertions(+), 61 deletions(-)
>
Diffstat proves its a win, even with the extra documentation for the new
function. Nice.
> +int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags,
> + Error **errp)
> +{
> + if ((flags & BDRV_BITMAP_BUSY) && bdrv_dirty_bitmap_busy(bitmap)) {
> + error_setg(errp, "Bitmap '%s' is currently in use by another"
> + " operation and cannot be used", bitmap->name);
Split before space,
> + return -1;
> + }
> +
> + if ((flags & BDRV_BITMAP_RO) && bdrv_dirty_bitmap_readonly(bitmap)) {
> + error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
> + bitmap->name);
> + return -1;
> + }
> +
> + if ((flags & BDRV_BITMAP_INCONSISTENT) &&
> + bdrv_dirty_bitmap_inconsistent(bitmap)) {
> + error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used",
> + bitmap->name);
> + error_append_hint(errp, "Try block-dirty-bitmap-remove to delete "
> + "this bitmap from disk");
split after space. Looks inconsistent within a single function (pardon
the pun :)
That's minor,
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[Qemu-block] [PATCH v3 6/7] block/dirty-bitmaps: disallow busy bitmaps as merge source, John Snow, 2019/03/01