Re: [Qemu-devel] [PATCH] qapi: add dirty bitmap status

[John Snow <jsnow@redhat.com>]

On 05/22/2015 04:52 AM, Markus Armbruster wrote:
> 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>
> 

This is OK by me, Markus. I'll assume that you are OK with or have
already made these changes locally, so I won't resend.

Thank you,
--John