[Qemu-devel] [PATCH 0/5] dirty-bitmaps: deprecate @status field

John Snow posted 5 patches 5 years, 2 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
block/dirty-bitmap.c           | 74 +++++++++++++++++++---------------
blockdev.c                     | 18 ++++-----
include/block/dirty-bitmap.h   |  7 ++--
migration/block-dirty-bitmap.c |  8 ++--
nbd/server.c                   |  6 +--
qapi/block-core.json           |  9 ++++-
tests/qemu-iotests/236.out     | 28 +++++++++++++
7 files changed, 96 insertions(+), 54 deletions(-)
[Qemu-devel] [PATCH 0/5] dirty-bitmaps: deprecate @status field
Posted by John Snow 5 years, 2 months ago
The current internal meanings of "locked", "user_locked",
"qmp_locked", "frozen", "enabled", and "disabled" are all
a little muddled.

Deprecate the @status field in favor of two new booleans
that carry very specific meanings. Then, rename and rework
some of the internal semantics to help make the API a bit
more clear and easier to read.

Well, in my opinion.

Based on my current bitmaps branch (includes Eric's patch
and my documentation update patch.)

John Snow (5):
  block/dirty-bitmap: add recording and busy properties
  block/dirty-bitmaps: rename frozen predicate helper
  block/dirty-bitmap: change semantics of enabled predicate
  block/dirty-bitmap: explicitly lock bitmaps with successors
  block/dirty-bitmaps: unify qmp_locked and user_locked calls

 block/dirty-bitmap.c           | 74 +++++++++++++++++++---------------
 blockdev.c                     | 18 ++++-----
 include/block/dirty-bitmap.h   |  7 ++--
 migration/block-dirty-bitmap.c |  8 ++--
 nbd/server.c                   |  6 +--
 qapi/block-core.json           |  9 ++++-
 tests/qemu-iotests/236.out     | 28 +++++++++++++
 7 files changed, 96 insertions(+), 54 deletions(-)

-- 
2.17.2


Re: [Qemu-devel] [PATCH 0/5] dirty-bitmaps: deprecate @status field
Posted by Eric Blake 5 years, 2 months ago
On 2/11/19 7:02 PM, John Snow wrote:
> The current internal meanings of "locked", "user_locked",
> "qmp_locked", "frozen", "enabled", and "disabled" are all
> a little muddled.
> 
> Deprecate the @status field in favor of two new booleans
> that carry very specific meanings. Then, rename and rework
> some of the internal semantics to help make the API a bit
> more clear and easier to read.

So far, my libvirt patches are not relying on the contents of the status
field, so deprecating things should not hurt libvirt. Using the full
2-cycle process is still probably wise for other users, though, even if
it means for some awkward back-compat computation of status for a while
longer.

> 
> Well, in my opinion.
> 
> Based on my current bitmaps branch (includes Eric's patch
> and my documentation update patch.)
> 
> John Snow (5):
>   block/dirty-bitmap: add recording and busy properties
>   block/dirty-bitmaps: rename frozen predicate helper
>   block/dirty-bitmap: change semantics of enabled predicate
>   block/dirty-bitmap: explicitly lock bitmaps with successors
>   block/dirty-bitmaps: unify qmp_locked and user_locked calls
> 
>  block/dirty-bitmap.c           | 74 +++++++++++++++++++---------------
>  blockdev.c                     | 18 ++++-----
>  include/block/dirty-bitmap.h   |  7 ++--
>  migration/block-dirty-bitmap.c |  8 ++--
>  nbd/server.c                   |  6 +--
>  qapi/block-core.json           |  9 ++++-
>  tests/qemu-iotests/236.out     | 28 +++++++++++++
>  7 files changed, 96 insertions(+), 54 deletions(-)

No change to qemu-deprecated.texi to start the clock ticking, so we can
remove the old field?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 0/5] dirty-bitmaps: deprecate @status field
Posted by John Snow 5 years, 2 months ago

On 2/12/19 1:12 PM, Eric Blake wrote:
> On 2/11/19 7:02 PM, John Snow wrote:
>> The current internal meanings of "locked", "user_locked",
>> "qmp_locked", "frozen", "enabled", and "disabled" are all
>> a little muddled.
>>
>> Deprecate the @status field in favor of two new booleans
>> that carry very specific meanings. Then, rename and rework
>> some of the internal semantics to help make the API a bit
>> more clear and easier to read.
> 
> So far, my libvirt patches are not relying on the contents of the status
> field, so deprecating things should not hurt libvirt. Using the full
> 2-cycle process is still probably wise for other users, though, even if
> it means for some awkward back-compat computation of status for a while
> longer.
> 

I didn't fully remove it, either. I will leave it there for the normal
deprecation period -- it doesn't hurt to keep it around.

>>
>> Well, in my opinion.
>>
>> Based on my current bitmaps branch (includes Eric's patch
>> and my documentation update patch.)
>>
>> John Snow (5):
>>   block/dirty-bitmap: add recording and busy properties
>>   block/dirty-bitmaps: rename frozen predicate helper
>>   block/dirty-bitmap: change semantics of enabled predicate
>>   block/dirty-bitmap: explicitly lock bitmaps with successors
>>   block/dirty-bitmaps: unify qmp_locked and user_locked calls
>>
>>  block/dirty-bitmap.c           | 74 +++++++++++++++++++---------------
>>  blockdev.c                     | 18 ++++-----
>>  include/block/dirty-bitmap.h   |  7 ++--
>>  migration/block-dirty-bitmap.c |  8 ++--
>>  nbd/server.c                   |  6 +--
>>  qapi/block-core.json           |  9 ++++-
>>  tests/qemu-iotests/236.out     | 28 +++++++++++++
>>  7 files changed, 96 insertions(+), 54 deletions(-)
> 
> No change to qemu-deprecated.texi to start the clock ticking, so we can
> remove the old field?
> 

I was counting on having to send a v2, I just didn't add the RFC tag
because I wanted to trick people into reviewing it.

If this passes the sniff test for you, I will document it in the .texi.

--js

Re: [Qemu-devel] [PATCH 0/5] dirty-bitmaps: deprecate @status field
Posted by Vladimir Sementsov-Ogievskiy 5 years, 2 months ago
I am ill, I think, will review it next week, if you not push it earlier.

Best regards,
Vladimir