[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/6] block/dirty-bitmaps: add user_locked sta
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/6] block/dirty-bitmaps: add user_locked status checker |
Date: |
Wed, 3 Oct 2018 14:28:05 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
On 10/03/2018 08:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> 03.10.2018 02:02, John Snow wrote:
>> Instead of both frozen and qmp_locked checks, wrap it into one check.
>> frozen implies the bitmap is split in two (for backup), and shouldn't
>> be modified. qmp_locked implies it's being used by another operation,
>> like being exported over NBD. In both cases it means we shouldn't allow
>> the user to modify it in any meaningful way.
>>
>> Replace any usages where we check both frozen and qmp_locked with the
>> new check.
>>
>> Signed-off-by: John Snow <address@hidden>
>> ---
>> block/dirty-bitmap.c | 6 ++++++
>> blockdev.c | 29 ++++++++---------------------
>> include/block/dirty-bitmap.h | 1 +
>> migration/block-dirty-bitmap.c | 10 ++--------
>> 4 files changed, 17 insertions(+), 29 deletions(-)
>>
>> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
>> index 8ac933cf1c..85bc668f6a 100644
>> --- a/block/dirty-bitmap.c
>> +++ b/block/dirty-bitmap.c
>> @@ -176,6 +176,12 @@ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
>> return bitmap->successor;
>> }
>>
>> +/* Both conditions disallow user-modification via QMP. */
>> +bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) {
>> + return (bdrv_dirty_bitmap_frozen(bitmap) ||
>> + bdrv_dirty_bitmap_qmp_locked(bitmap));
>
> hmm, extra parentheses
I'm fond of them, but I'll take them out.
>
>> +}
>> +
>> void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool
>> qmp_locked)
>> {
>> qemu_mutex_lock(bitmap->mutex);
>
> [...]
>
>> --- a/migration/block-dirty-bitmap.c
>> +++ b/migration/block-dirty-bitmap.c
>> @@ -301,14 +301,8 @@ static int init_dirty_bitmap_migration(void)
>> goto fail;
>> }
>>
>> - if (bdrv_dirty_bitmap_frozen(bitmap)) {
>> - error_report("Can't migrate frozen dirty bitmap: '%s",
>> - bdrv_dirty_bitmap_name(bitmap));
>> - goto fail;
>> - }
>> -
>> - if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
>> - error_report("Can't migrate locked dirty bitmap: '%s",
>> + if (bdrv_dirty_bitmap_user_locked(bitmap)) {
>> + error_report("Can't migrate a bitmap that is in use: '%s'",
>
> "by another operation" like in other cases?
>
Oh, sure. Can't hurt.
>> bdrv_dirty_bitmap_name(bitmap));
>> goto fail;
>> }
>
> anyway,
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> --
> Best regards,
> Vladimir
>
Thanks, I'll just make these edits and trust that Eric is fine with it
as well.
--js