block/dirty-bitmap.c | 20 ++++++++++++++------ qapi/block-core.json | 32 ++++++++++++++++++++++++-------- 2 files changed, 38 insertions(+), 14 deletions(-)
The meaning of the states has changed subtly over time,
this should bring the understanding more in-line with the
current, actual usages.
Reported-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
V2: Amended some wordings, though this is a little chatty now.
Hopefully it's clearer, though.
---
block/dirty-bitmap.c | 20 ++++++++++++++------
qapi/block-core.json | 32 ++++++++++++++++++++++++--------
2 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 00ea36f554..b1b9b03939 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -29,12 +29,20 @@
#include "block/blockjob.h"
/**
- * A BdrvDirtyBitmap can be in three possible states:
- * (1) successor is NULL and disabled is false: full r/w mode
- * (2) successor is NULL and disabled is true: read only mode ("disabled")
- * (3) successor is set: frozen mode.
- * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
- * or enabled. A frozen bitmap can only abdicate() or reclaim().
+ * A BdrvDirtyBitmap can be in four possible user-visible states:
+ * (1) Active: successor is NULL, and disabled is false: full r/w mode
+ * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
+ * guest writes are dropped, but monitor writes are possible,
+ * through commands like merge and clear.
+ * (3) Frozen: successor is not NULL.
+ * A frozen bitmap cannot be renamed, deleted, cleared, set,
+ * enabled, merged to, etc. A frozen bitmap can only abdicate()
+ * or reclaim().
+ * In this state, the anonymous successor bitmap may be either
+ * Active and recording writes from the guest (e.g. backup jobs),
+ * but it can be Disabled and not recording writes.
+ * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
+ * in any way from the monitor.
*/
struct BdrvDirtyBitmap {
QemuMutex *mutex;
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5f17d67d71..a775af04cf 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -417,17 +417,27 @@
#
# An enumeration of possible states that a dirty bitmap can report to the user.
#
-# @frozen: The bitmap is currently in-use by a backup operation or block job,
-# and is immutable.
+# @frozen: The bitmap is currently in-use by some operation and is immutable.
+# If the bitmap was @active prior to the operation, new writes by the
+# guest are being recorded in a temporary buffer, and will not be lost.
+# Generally, bitmaps are cleared on successful use in an operation and
+# the temporary buffer is committed into the bitmap. On failure, the
+# temporary buffer is merged back into the bitmap without first
+# clearing it.
+# Please refer to the documentation for each bitmap-using operation,
+# See also @blockdev-backup, @drive-backup.
#
-# @disabled: The bitmap is currently in-use by an internal operation and is
-# read-only. It can still be deleted.
+# @disabled: The bitmap is not currently recording new writes by the guest.
+# This is requested explicitly via @block-dirty-bitmap-disable.
+# It can still be cleared, deleted, or used for backup operations.
#
# @active: The bitmap is actively monitoring for new writes, and can be cleared,
# deleted, or used for backup operations.
#
-# @locked: The bitmap is currently in-use by some operation and can not be
-# cleared, deleted, or used for backup operations. (Since 2.12)
+# @locked: The bitmap is currently in-use by some operation and is immutable.
+# If the bitmap was @active prior to the operation, it is still
+# recording new writes. If the bitmap was @disabled, it is not
+# recording new writes. (Since 2.12)
#
# Since: 2.4
##
@@ -2052,9 +2062,15 @@
# @block-dirty-bitmap-merge:
#
# Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
-# The @bitmaps dirty bitmaps are unchanged.
+# Dirty bitmaps in @bitmaps will be unchanged, except if it also appears
+# as the @target bitmap. Any bits already set in @target will still be
+# set after the merge, i.e., this operation does not clear the target.
# On error, @target is unchanged.
#
+# The resulting bitmap will count as dirty any clusters that were dirty in any
+# of the source bitmaps. This can be used to achieve backup checkpoints, or in
+# simpler usages, to copy bitmaps.
+#
# Returns: nothing on success
# If @node is not a valid block device, DeviceNotFound
# If any bitmap in @bitmaps or @target is not found, GenericError
@@ -2089,7 +2105,7 @@
##
# @x-debug-block-dirty-bitmap-sha256:
#
-# Get bitmap SHA256
+# Get bitmap SHA256.
#
# Returns: BlockDirtyBitmapSha256 on success
# If @node is not a valid block device, DeviceNotFound
--
2.17.2
Patchew URL: https://patchew.org/QEMU/20190202011048.12343-1-jsnow@redhat.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash time make docker-test-mingw@fedora SHOW_ENV=1 J=14 === TEST SCRIPT END === Configure options: --enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/tmp/qemu-test/install --python=/usr/bin/python3 --cross-prefix=x86_64-w64-mingw32- --enable-trace-backends=simple --enable-gnutls --enable-nettle --enable-curl --enable-vnc --enable-bzip2 --enable-guest-agent --with-sdlabi=2.0 ERROR: unknown option --with-sdlabi=2.0 Try '/tmp/qemu-test/src/configure --help' for more information # QEMU configure log Sun Feb 3 17:14:51 UTC 2019 # Configured with: '/tmp/qemu-test/src/configure' '--enable-werror' '--target-list=x86_64-softmmu,aarch64-softmmu' '--prefix=/tmp/qemu-test/install' '--python=/usr/bin/python3' '--cross-prefix=x86_64-w64-mingw32-' '--enable-trace-backends=simple' '--enable-gnutls' '--enable-nettle' '--enable-curl' '--enable-vnc' '--enable-bzip2' '--enable-guest-agent' '--with-sdlabi=2.0' --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 617 634 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c config-temp/qemu-conf.c:2:2: error: #error __linux__ not defined #error __linux__ not defined ^~~~~ --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 617 686 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c config-temp/qemu-conf.c:2:2: error: #error __i386__ not defined #error __i386__ not defined ^~~~~ --- funcs: do_compiler do_cc compile_object check_define main lines: 92 122 617 689 0 x86_64-w64-mingw32-gcc -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -c -o config-temp/qemu-conf.o config-temp/qemu-conf.c config-temp/qemu-conf.c:2:2: error: #error __ILP32__ not defined #error __ILP32__ not defined ^~~~~ --- lines: 92 128 920 0 x86_64-w64-mingw32-gcc -mthreads -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99 -o config-temp/qemu-conf.exe config-temp/qemu-conf.c -g -liberty /usr/lib/gcc/x86_64-w64-mingw32/8.2.0/../../../../x86_64-w64-mingw32/bin/ld: cannot find -liberty collect2: error: ld returned 1 exit status Failed to run 'configure' Traceback (most recent call last): File "./tests/docker/docker.py", line 563, in <module> The full log is available at http://patchew.org/logs/20190202011048.12343-1-jsnow@redhat.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 2/1/19 7:10 PM, John Snow wrote:
> The meaning of the states has changed subtly over time,
> this should bring the understanding more in-line with the
> current, actual usages.
>
> Reported-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
>
> ---
> V2: Amended some wordings, though this is a little chatty now.
> Hopefully it's clearer, though.
> ---
> /**
> - * A BdrvDirtyBitmap can be in three possible states:
> - * (1) successor is NULL and disabled is false: full r/w mode
> - * (2) successor is NULL and disabled is true: read only mode ("disabled")
> - * (3) successor is set: frozen mode.
> - * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
> - * or enabled. A frozen bitmap can only abdicate() or reclaim().
> + * A BdrvDirtyBitmap can be in four possible user-visible states:
> + * (1) Active: successor is NULL, and disabled is false: full r/w mode
> + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
> + * guest writes are dropped, but monitor writes are possible,
> + * through commands like merge and clear.
> + * (3) Frozen: successor is not NULL.
> + * A frozen bitmap cannot be renamed, deleted, cleared, set,
> + * enabled, merged to, etc. A frozen bitmap can only abdicate()
> + * or reclaim().
> + * In this state, the anonymous successor bitmap may be either
> + * Active and recording writes from the guest (e.g. backup jobs),
> + * but it can be Disabled and not recording writes.
s/but/or/
> + * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
> + * in any way from the monitor.
Worth a parenthetical comment mentioning NBD export as a possible reason
for Locked to be visible?
With the one word fix,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On 2/4/19 10:00 AM, Eric Blake wrote:
> On 2/1/19 7:10 PM, John Snow wrote:
>> The meaning of the states has changed subtly over time,
>> this should bring the understanding more in-line with the
>> current, actual usages.
>>
>> Reported-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>>
>> ---
>> V2: Amended some wordings, though this is a little chatty now.
>> Hopefully it's clearer, though.
>> ---
>
>> /**
>> - * A BdrvDirtyBitmap can be in three possible states:
>> - * (1) successor is NULL and disabled is false: full r/w mode
>> - * (2) successor is NULL and disabled is true: read only mode ("disabled")
>> - * (3) successor is set: frozen mode.
>> - * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
>> - * or enabled. A frozen bitmap can only abdicate() or reclaim().
>> + * A BdrvDirtyBitmap can be in four possible user-visible states:
>> + * (1) Active: successor is NULL, and disabled is false: full r/w mode
>> + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
>> + * guest writes are dropped, but monitor writes are possible,
>> + * through commands like merge and clear.
>> + * (3) Frozen: successor is not NULL.
>> + * A frozen bitmap cannot be renamed, deleted, cleared, set,
>> + * enabled, merged to, etc. A frozen bitmap can only abdicate()
>> + * or reclaim().
>> + * In this state, the anonymous successor bitmap may be either
>> + * Active and recording writes from the guest (e.g. backup jobs),
>> + * but it can be Disabled and not recording writes.
>
> s/but/or/
>
I'll touch that up.
>> + * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
>> + * in any way from the monitor.
>
> Worth a parenthetical comment mentioning NBD export as a possible reason
> for Locked to be visible?
>
Since this is effectively developer documentation, yes. How about:
"Whether Active or Disabled, the user cannot modify this bitmap in any
way from the monitor. This mode is used to achieve a successor-less
locking of a bitmap for operations like point-in-time NBD export or for
incoming migration data."
> With the one word fix,
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
If Vladimir agrees with these phrasings I'll stage it.
--js
04.02.2019 22:26, John Snow wrote:
>
>
> On 2/4/19 10:00 AM, Eric Blake wrote:
>> On 2/1/19 7:10 PM, John Snow wrote:
>>> The meaning of the states has changed subtly over time,
>>> this should bring the understanding more in-line with the
>>> current, actual usages.
>>>
>>> Reported-by: Eric Blake <eblake@redhat.com>
>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>
>>> ---
>>> V2: Amended some wordings, though this is a little chatty now.
>>> Hopefully it's clearer, though.
>>> ---
>>
>>> /**
>>> - * A BdrvDirtyBitmap can be in three possible states:
>>> - * (1) successor is NULL and disabled is false: full r/w mode
>>> - * (2) successor is NULL and disabled is true: read only mode ("disabled")
>>> - * (3) successor is set: frozen mode.
>>> - * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
>>> - * or enabled. A frozen bitmap can only abdicate() or reclaim().
>>> + * A BdrvDirtyBitmap can be in four possible user-visible states:
>>> + * (1) Active: successor is NULL, and disabled is false: full r/w mode
>>> + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
>>> + * guest writes are dropped, but monitor writes are possible,
>>> + * through commands like merge and clear.
>>> + * (3) Frozen: successor is not NULL.
>>> + * A frozen bitmap cannot be renamed, deleted, cleared, set,
>>> + * enabled, merged to, etc. A frozen bitmap can only abdicate()
>>> + * or reclaim().
>>> + * In this state, the anonymous successor bitmap may be either
>>> + * Active and recording writes from the guest (e.g. backup jobs),
>>> + * but it can be Disabled and not recording writes.
>>
>> s/but/or/
>>
>
> I'll touch that up.
>
>>> + * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
>>> + * in any way from the monitor.
>>
>> Worth a parenthetical comment mentioning NBD export as a possible reason
>> for Locked to be visible?
>>
>
> Since this is effectively developer documentation, yes. How about:
>
> "Whether Active or Disabled, the user cannot modify this bitmap in any
> way from the monitor. This mode is used to achieve a successor-less
> locking of a bitmap for operations like point-in-time NBD export or for
> incoming migration data."
>
>> With the one word fix,
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>
> If Vladimir agrees with these phrasings I'll stage it.
>
Ooops, sorry for the long delay.
Honestly, I still don't like it very much, as is worded in manner that bitmap
may be in two states (Active and Locked), and it is not very good for something
called "state".. But anyway it is understandable and a lot better than what it was, so:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
On 2/11/19 1:25 PM, Vladimir Sementsov-Ogievskiy wrote:
> 04.02.2019 22:26, John Snow wrote:
>>
>>
>> On 2/4/19 10:00 AM, Eric Blake wrote:
>>> On 2/1/19 7:10 PM, John Snow wrote:
>>>> The meaning of the states has changed subtly over time,
>>>> this should bring the understanding more in-line with the
>>>> current, actual usages.
>>>>
>>>> Reported-by: Eric Blake <eblake@redhat.com>
>>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>>>
>>>> ---
>>>> V2: Amended some wordings, though this is a little chatty now.
>>>> Hopefully it's clearer, though.
>>>> ---
>>>
>>>> /**
>>>> - * A BdrvDirtyBitmap can be in three possible states:
>>>> - * (1) successor is NULL and disabled is false: full r/w mode
>>>> - * (2) successor is NULL and disabled is true: read only mode ("disabled")
>>>> - * (3) successor is set: frozen mode.
>>>> - * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
>>>> - * or enabled. A frozen bitmap can only abdicate() or reclaim().
>>>> + * A BdrvDirtyBitmap can be in four possible user-visible states:
>>>> + * (1) Active: successor is NULL, and disabled is false: full r/w mode
>>>> + * (2) Disabled: successor is NULL, and disabled is true: qualified r/w mode,
>>>> + * guest writes are dropped, but monitor writes are possible,
>>>> + * through commands like merge and clear.
>>>> + * (3) Frozen: successor is not NULL.
>>>> + * A frozen bitmap cannot be renamed, deleted, cleared, set,
>>>> + * enabled, merged to, etc. A frozen bitmap can only abdicate()
>>>> + * or reclaim().
>>>> + * In this state, the anonymous successor bitmap may be either
>>>> + * Active and recording writes from the guest (e.g. backup jobs),
>>>> + * but it can be Disabled and not recording writes.
>>>
>>> s/but/or/
>>>
>>
>> I'll touch that up.
>>
>>>> + * (4) Locked: Whether Active or Disabled, the user cannot modify this bitmap
>>>> + * in any way from the monitor.
>>>
>>> Worth a parenthetical comment mentioning NBD export as a possible reason
>>> for Locked to be visible?
>>>
>>
>> Since this is effectively developer documentation, yes. How about:
>>
>> "Whether Active or Disabled, the user cannot modify this bitmap in any
>> way from the monitor. This mode is used to achieve a successor-less
>> locking of a bitmap for operations like point-in-time NBD export or for
>> incoming migration data."
>>
>>> With the one word fix,
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>
>>
>> If Vladimir agrees with these phrasings I'll stage it.
>>
>
> Ooops, sorry for the long delay.
>
> Honestly, I still don't like it very much, as is worded in manner that bitmap
> may be in two states (Active and Locked), and it is not very good for something
> called "state".. But anyway it is understandable and a lot better than what it was, so:
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
>
Yeah, understood -- I think the phrasings here make it clear that we
want to change how we track this. It's not ideal right now but I think
this explains what we already have as best as I can summarize it.
Thanks for your feedback.
--js
© 2016 - 2024 Red Hat, Inc.