From: Markus Armbruster <armbru@redhat.com>
References: <1431460381-8530-1-git-send-email-jsnow@redhat.com>
	<55525D43.6060402@redhat.com>
Date: Fri, 22 May 2015 10:52:03 +0200
In-Reply-To: <55525D43.6060402@redhat.com> (Eric Blake's message of "Tue, 12
	May 2015 14:06:27 -0600")
Message-ID: <87y4kht2oc.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH] qapi: add dirty bitmap status
To: Eric Blake <eblake@redhat.com>
Cc: John Snow <jsnow@redhat.com>, qemu-devel@nongnu.org

Eric Blake <eblake@redhat.com> writes:

> On 05/12/2015 01:53 PM, John Snow wrote:
>> Bitmaps can be in a handful of different states with potentially
>> more to come as we tool around with migration and persistence patches.
>> 
>> Instead of having a bunch of boolean fields, it was suggested that we
>> just have an enum status field that will help expose the reason to
>> management APIs why certain bitmaps may be unavailable for various
>> commands
>> 
>> (e.g. busy in another operation, busy being migrated, etc.)
>
> Might be worth mentioning that this is an API change, but safe because
> the old API is unreleased (and therefore, this patch MUST go in the 2.4
> time frame, if at all).
>
>> 
>> Suggested-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>  block.c               | 13 ++++++++++++-
>>  include/block/block.h |  1 +
>>  qapi/block-core.json  | 23 +++++++++++++++++++++--
>>  3 files changed, 34 insertions(+), 3 deletions(-)
>> 
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Patch does two things:

1. Convert status from bool frozen to enum.
2. Add new status 'disabled'.

I would've done this separately, but it's no big deal.  But I think we
should spell it out in the commit message.

What about:

    qapi: add dirty bitmap status

    Bitmaps can be in a handful of different states with potentially
    more to come as we tool around with migration and persistence patches.

    Management applications may need to know why certain bitmaps are
    unavailable for various commands, e.g. busy in another operation,
    busy being migrated, etc.

    Right now, all we offer is BlockDirtyInfo's boolean member 'frozen'.
    Instead of adding more booleans, replace it by an enumeration member
    'status' with values 'active' and 'frozen'.  Then add new value
    'disabled'.

    Incompatible change.  Fine because the changed part hasn't been
    released so far.

    Suggested-by: Eric Blake <eblake@redhat.com>
    Signed-off-by: John Snow <jsnow@redhat.com>
    Reviewed-by: Eric Blake <eblake@redhat.com>
    [Commit message tweaked]
    Signed-off-by: Markus Armbruster <armbru@redhat.com>