* [Qemu-devel] [PULL 00/17] Bitmaps patches
@ 2019-03-08 20:28 John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 01/17] block/dirty-bitmap: add recording and busy properties John Snow
` (20 more replies)
0 siblings, 21 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
The following changes since commit 234afe78281b10a798fb97c584e1b677844aaab8:
Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-03-08' into staging (2019-03-08 16:31:34 +0000)
are available in the Git repository at:
https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
for you to fetch changes up to fc91aa3d937e56d512c702d9b966fcea0e499f0e:
block/dirty-bitmaps: implement inconsistent bit (2019-03-08 14:07:29 -0500)
----------------------------------------------------------------
Pull request
----------------------------------------------------------------
John Snow (17):
block/dirty-bitmap: add recording and busy properties
block/dirty-bitmaps: rename frozen predicate helper
block/dirty-bitmap: remove set/reset assertions against enabled bit
block/dirty-bitmap: change semantics of enabled predicate
nbd: change error checking order for bitmaps
block/dirty-bitmap: explicitly lock bitmaps with successors
block/dirty-bitmaps: unify qmp_locked and user_locked calls
block/dirty-bitmaps: move comment block
blockdev: remove unused paio parameter documentation
iotests: add busy/recording bit test to 124
block/dirty-bitmaps: add inconsistent bit
block/dirty-bitmap: add inconsistent status
block/dirty-bitmaps: add block_dirty_bitmap_check function
block/dirty-bitmaps: prohibit readonly bitmaps for backups
block/dirty-bitmaps: prohibit removing readonly bitmaps
block/dirty-bitmaps: disallow busy bitmaps as merge source
block/dirty-bitmaps: implement inconsistent bit
qemu-deprecated.texi | 6 ++
qapi/block-core.json | 28 +++++-
include/block/dirty-bitmap.h | 20 +++-
block/dirty-bitmap.c | 169 ++++++++++++++++++++++-----------
block/qcow2-bitmap.c | 103 ++++++++++----------
blockdev.c | 51 ++--------
migration/block-dirty-bitmap.c | 18 ++--
nbd/server.c | 13 ++-
tests/qemu-iotests/124 | 113 ++++++++++++++++++++++
tests/qemu-iotests/124.out | 4 +-
tests/qemu-iotests/236.out | 28 ++++++
11 files changed, 377 insertions(+), 176 deletions(-)
--
2.17.2
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 01/17] block/dirty-bitmap: add recording and busy properties
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 02/17] block/dirty-bitmaps: rename frozen predicate helper John Snow
` (19 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
The current API allows us to report a single status, which we've defined as:
Frozen: has a successor, treated as qmp_locked, may or may not be enabled.
Locked: no successor, qmp_locked. may or may not be enabled.
Disabled: Not frozen or locked, disabled.
Active: Not frozen, locked, or disabled.
The problem is that both "Frozen" and "Locked" mean nearly the same thing,
and that both of them do not intuit whether they are recording guest writes
or not.
This patch deprecates that status field and introduces two orthogonal
properties instead to replace it.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-2-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
qemu-deprecated.texi | 6 ++++++
qapi/block-core.json | 10 +++++++++-
block/dirty-bitmap.c | 9 +++++++++
tests/qemu-iotests/236.out | 28 ++++++++++++++++++++++++++++
4 files changed, 52 insertions(+), 1 deletion(-)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 45c57952da..7cfc9c3fce 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -67,6 +67,12 @@ topologies described with -smp include all possible cpus, i.e.
"autoload" parameter is now ignored. All bitmaps are automatically loaded
from qcow2 images.
+@subsection query-block result field dirty-bitmaps[i].status (since 4.0)
+
+The ``status'' field of the ``BlockDirtyInfo'' structure, returned by
+the query-block command is deprecated. Two new boolean fields,
+``recording'' and ``busy'' effectively replace it.
+
@subsection query-cpus (since 2.12.0)
The ``query-cpus'' command is replaced by the ``query-cpus-fast'' command.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 2b8afbb924..6e543594b3 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -458,7 +458,14 @@
#
# @granularity: granularity of the dirty bitmap in bytes (since 1.4)
#
-# @status: current status of the dirty bitmap (since 2.4)
+# @status: Deprecated in favor of @recording and @locked. (since 2.4)
+#
+# @recording: true if the bitmap is recording new writes from the guest.
+# Replaces `active` and `disabled` statuses. (since 4.0)
+#
+# @busy: true if the bitmap is in-use by some operation (NBD or jobs)
+# and cannot be modified via QMP or used by another operation.
+# Replaces `locked` and `frozen` statuses. (since 4.0)
#
# @persistent: true if the bitmap will eventually be flushed to persistent
# storage (since 4.0)
@@ -467,6 +474,7 @@
##
{ 'struct': 'BlockDirtyInfo',
'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
+ 'recording': 'bool', 'busy': 'bool',
'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
##
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c6d4acebfa..101383b3af 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -226,6 +226,13 @@ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
}
}
+/* Called with BQL taken. */
+static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)
+{
+ return !bitmap->disabled || (bitmap->successor &&
+ !bitmap->successor->disabled);
+}
+
/**
* Create a successor bitmap destined to replace this bitmap after an operation.
* Requires that the bitmap is not frozen and has no successor.
@@ -448,6 +455,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->has_name = !!bm->name;
info->name = g_strdup(bm->name);
info->status = bdrv_dirty_bitmap_status(bm);
+ info->recording = bdrv_dirty_bitmap_recording(bm);
+ info->busy = bdrv_dirty_bitmap_user_locked(bm);
info->persistent = bm->persistent;
entry->value = info;
*plist = entry;
diff --git a/tests/qemu-iotests/236.out b/tests/qemu-iotests/236.out
index 5006f7bca1..815cd053f0 100644
--- a/tests/qemu-iotests/236.out
+++ b/tests/qemu-iotests/236.out
@@ -22,17 +22,21 @@ write -P0xcd 0x3ff0000 64k
"bitmaps": {
"drive0": [
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapB",
"persistent": false,
+ "recording": true,
"status": "active"
},
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapA",
"persistent": false,
+ "recording": true,
"status": "active"
}
]
@@ -84,17 +88,21 @@ write -P0xcd 0x3ff0000 64k
"bitmaps": {
"drive0": [
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapB",
"persistent": false,
+ "recording": true,
"status": "active"
},
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapA",
"persistent": false,
+ "recording": true,
"status": "active"
}
]
@@ -184,24 +192,30 @@ write -P0xea 0x3fe0000 64k
"bitmaps": {
"drive0": [
{
+ "busy": false,
"count": 393216,
"granularity": 65536,
"name": "bitmapC",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapB",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 458752,
"granularity": 65536,
"name": "bitmapA",
"persistent": false,
+ "recording": false,
"status": "disabled"
}
]
@@ -251,24 +265,30 @@ write -P0xea 0x3fe0000 64k
"bitmaps": {
"drive0": [
{
+ "busy": false,
"count": 393216,
"granularity": 65536,
"name": "bitmapC",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapB",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 458752,
"granularity": 65536,
"name": "bitmapA",
"persistent": false,
+ "recording": false,
"status": "disabled"
}
]
@@ -311,31 +331,39 @@ write -P0xea 0x3fe0000 64k
"bitmaps": {
"drive0": [
{
+ "busy": false,
"count": 458752,
"granularity": 65536,
"name": "bitmapD",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 393216,
"granularity": 65536,
"name": "bitmapC",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 262144,
"granularity": 65536,
"name": "bitmapB",
"persistent": false,
+ "recording": false,
"status": "disabled"
},
{
+ "busy": false,
"count": 458752,
"granularity": 65536,
"name": "bitmapA",
"persistent": false,
+ "recording": false,
"status": "disabled"
}
]
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 02/17] block/dirty-bitmaps: rename frozen predicate helper
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 01/17] block/dirty-bitmap: add recording and busy properties John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 03/17] block/dirty-bitmap: remove set/reset assertions against enabled bit John Snow
` (18 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
"Frozen" was a good description a long time ago, but it isn't adequate now.
Rename the frozen predicate to has_successor to make the semantics of the
predicate more clear to outside callers.
In the process, remove some calls to frozen() that no longer semantically
make sense. For bdrv_enable_dirty_bitmap_locked and
bdrv_disable_dirty_bitmap_locked, it doesn't make sense to prohibit QEMU
internals from performing this action when we only wished to prohibit QMP
users from issuing these commands. All of the QMP API commands for bitmap
manipulation already check against user_locked() to prohibit these actions.
Several other assertions really want to check that the bitmap isn't in-use
by another operation -- use the bitmap_user_locked function for this instead,
which presently also checks for has_successor. This leaves some redundant
checks of has_successor through different helpers that are addressed in
forthcoming patches.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/block/dirty-bitmap.h | 2 +-
block/dirty-bitmap.c | 34 +++++++++++++++++++---------------
migration/block-dirty-bitmap.c | 2 +-
3 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 04a117fc81..cdbb4dfefd 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -36,7 +36,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs);
uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs);
uint32_t bdrv_dirty_bitmap_granularity(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
-bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap);
const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap);
int64_t bdrv_dirty_bitmap_size(const BdrvDirtyBitmap *bitmap);
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 101383b3af..c8fa21b526 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -50,7 +50,7 @@ struct BdrvDirtyBitmap {
HBitmap *meta; /* Meta dirty bitmap */
bool qmp_locked; /* Bitmap is locked, it can't be modified
through QMP */
- BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
+ BdrvDirtyBitmap *successor; /* Anonymous child; implies user_locked state */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap, in bytes */
bool disabled; /* Bitmap is disabled. It ignores all writes to
@@ -183,14 +183,14 @@ const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap)
}
/* Called with BQL taken. */
-bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
+bool bdrv_dirty_bitmap_has_successor(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) ||
+ return bdrv_dirty_bitmap_has_successor(bitmap) ||
bdrv_dirty_bitmap_qmp_locked(bitmap);
}
@@ -215,7 +215,7 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
/* Called with BQL taken. */
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
{
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
+ if (bdrv_dirty_bitmap_has_successor(bitmap)) {
return DIRTY_BITMAP_STATUS_FROZEN;
} else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
return DIRTY_BITMAP_STATUS_LOCKED;
@@ -235,7 +235,7 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)
/**
* Create a successor bitmap destined to replace this bitmap after an operation.
- * Requires that the bitmap is not frozen and has no successor.
+ * Requires that the bitmap is not user_locked and has no successor.
* Called with BQL taken.
*/
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
@@ -244,12 +244,16 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
uint64_t granularity;
BdrvDirtyBitmap *child;
- if (bdrv_dirty_bitmap_frozen(bitmap)) {
- error_setg(errp, "Cannot create a successor for a bitmap that is "
- "currently frozen");
+ if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
+ "by an operation");
+ return -1;
+ }
+ if (bdrv_dirty_bitmap_has_successor(bitmap)) {
+ error_setg(errp, "Cannot create a successor for a bitmap that already "
+ "has one");
return -1;
}
- assert(!bitmap->successor);
/* Create an anonymous successor */
granularity = bdrv_dirty_bitmap_granularity(bitmap);
@@ -268,7 +272,6 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
{
- assert(!bdrv_dirty_bitmap_frozen(bitmap));
bitmap->disabled = false;
}
@@ -285,7 +288,8 @@ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
{
assert(!bitmap->active_iterators);
- assert(!bdrv_dirty_bitmap_frozen(bitmap));
+ assert(!bdrv_dirty_bitmap_user_locked(bitmap));
+ assert(!bdrv_dirty_bitmap_has_successor(bitmap));
assert(!bitmap->meta);
QLIST_REMOVE(bitmap, list);
hbitmap_free(bitmap->bitmap);
@@ -325,7 +329,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
/**
* In cases of failure where we can no longer safely delete the parent,
* we may wish to re-join the parent and child/successor.
- * The merged parent will be un-frozen, but not explicitly re-enabled.
+ * The merged parent will not be user_locked, nor explicitly re-enabled.
* Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken.
*/
BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
@@ -373,7 +377,8 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
bdrv_dirty_bitmaps_lock(bs);
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
- assert(!bdrv_dirty_bitmap_frozen(bitmap));
+ assert(!bdrv_dirty_bitmap_user_locked(bitmap));
+ assert(!bdrv_dirty_bitmap_has_successor(bitmap));
assert(!bitmap->active_iterators);
hbitmap_truncate(bitmap->bitmap, bytes);
bitmap->size = bytes;
@@ -391,7 +396,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
/**
* Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()).
- * There must not be any frozen bitmaps attached.
+ * There must not be any user_locked bitmaps attached.
* This function does not remove persistent bitmaps from the storage.
* Called with BQL taken.
*/
@@ -428,7 +433,6 @@ void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
{
bdrv_dirty_bitmap_lock(bitmap);
- assert(!bdrv_dirty_bitmap_frozen(bitmap));
bitmap->disabled = true;
bdrv_dirty_bitmap_unlock(bitmap);
}
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 6426151e4f..ac6954142f 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -542,7 +542,7 @@ static void dirty_bitmap_load_complete(QEMUFile *f, DirtyBitmapLoadState *s)
}
}
- if (bdrv_dirty_bitmap_frozen(s->bitmap)) {
+ if (bdrv_dirty_bitmap_has_successor(s->bitmap)) {
bdrv_dirty_bitmap_lock(s->bitmap);
if (enabled_bitmaps == NULL) {
/* in postcopy */
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 03/17] block/dirty-bitmap: remove set/reset assertions against enabled bit
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 01/17] block/dirty-bitmap: add recording and busy properties John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 02/17] block/dirty-bitmaps: rename frozen predicate helper John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 04/17] block/dirty-bitmap: change semantics of enabled predicate John Snow
` (17 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
bdrv_set_dirty_bitmap and bdrv_reset_dirty_bitmap are only used as an
internal API by the mirror and migration areas of our code. These
calls modify the bitmap, but do so at the behest of QEMU and not the
guest.
Presently, these bitmaps are always "enabled" anyway, but there's no
reason they have to be.
Modify these internal APIs to drop this assertion.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-4-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c8fa21b526..b2b32e0323 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -544,7 +544,6 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter)
void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
int64_t offset, int64_t bytes)
{
- assert(bdrv_dirty_bitmap_enabled(bitmap));
assert(!bdrv_dirty_bitmap_readonly(bitmap));
hbitmap_set(bitmap->bitmap, offset, bytes);
}
@@ -561,7 +560,6 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
int64_t offset, int64_t bytes)
{
- assert(bdrv_dirty_bitmap_enabled(bitmap));
assert(!bdrv_dirty_bitmap_readonly(bitmap));
hbitmap_reset(bitmap->bitmap, offset, bytes);
}
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 04/17] block/dirty-bitmap: change semantics of enabled predicate
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (2 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 03/17] block/dirty-bitmap: remove set/reset assertions against enabled bit John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 05/17] nbd: change error checking order for bitmaps John Snow
` (16 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Currently, the enabled predicate means something like:
"the QAPI status of the bitmap is ACTIVE."
After this patch, it should mean exclusively:
"This bitmap is recording guest writes, and is allowed to do so."
In many places, this is how this predicate was already used.
Internal usages of the bitmap QPI can call user_locked to find out if
the bitmap is in use by an operation.
To accommodate this, modify the create_successor routine to now
explicitly disable the parent bitmap at creation time.
Justifications:
1. bdrv_dirty_bitmap_status suffers no change from the lack of
1:1 parity with the new predicates because of the order in which
the predicates are checked. This is now only for compatibility.
2. bdrv_set_dirty() is unchanged: pre-patch, it was skipping bitmaps that were
disabled or had a successor, while post-patch it is only skipping bitmaps
that are disabled. To accommodate this, create_successor now ensures that
any bitmap with a successor is explicitly disabled.
3. qcow2_store_persistent_dirty_bitmaps: No functional change. This function
cares only about the literal enabled bit, and makes no effort to check if
the bitmap is in-use or not. After this patch there are still no ways to
produce an enabled bitmap with a successor.
4. block_dirty_bitmap_enable_prepare
block_dirty_bitmap_disable_prepare
init_dirty_bitmap_migration
nbd_export_new
These functions care about the literal enabled bit,
and already check user_locked separately.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-5-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index b2b32e0323..2e3192d86f 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -209,7 +209,7 @@ bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap)
/* Called with BQL taken. */
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
{
- return !(bitmap->disabled || bitmap->successor);
+ return !bitmap->disabled;
}
/* Called with BQL taken. */
@@ -236,6 +236,7 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)
/**
* Create a successor bitmap destined to replace this bitmap after an operation.
* Requires that the bitmap is not user_locked and has no successor.
+ * The successor will be enabled if the parent bitmap was.
* Called with BQL taken.
*/
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
@@ -264,6 +265,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
/* Successor will be on or off based on our current state. */
child->disabled = bitmap->disabled;
+ bitmap->disabled = true;
/* Install the successor and freeze the parent */
bitmap->successor = child;
@@ -329,7 +331,8 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
/**
* In cases of failure where we can no longer safely delete the parent,
* we may wish to re-join the parent and child/successor.
- * The merged parent will not be user_locked, nor explicitly re-enabled.
+ * The merged parent will not be user_locked.
+ * The marged parent will be enabled if and only if the successor was enabled.
* Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken.
*/
BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
@@ -347,6 +350,8 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
error_setg(errp, "Merging of parent and successor bitmap failed");
return NULL;
}
+
+ parent->disabled = successor->disabled;
bdrv_release_dirty_bitmap_locked(successor);
parent->successor = NULL;
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 05/17] nbd: change error checking order for bitmaps
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (3 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 04/17] block/dirty-bitmap: change semantics of enabled predicate John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 06/17] block/dirty-bitmap: explicitly lock bitmaps with successors John Snow
` (15 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Check that the bitmap is not in use prior to it checking if it is
not enabled/recording guest writes. The bitmap being busy was likely
at the behest of the user, so this error has a greater chance of being
understood by the user.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-6-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
nbd/server.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 0910d09a6d..de21c64ce3 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1510,6 +1510,11 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
goto fail;
}
+ if (bdrv_dirty_bitmap_user_locked(bm)) {
+ error_setg(errp, "Bitmap '%s' is in use", bitmap);
+ goto fail;
+ }
+
if ((nbdflags & NBD_FLAG_READ_ONLY) && bdrv_is_writable(bs) &&
bdrv_dirty_bitmap_enabled(bm)) {
error_setg(errp,
@@ -1518,11 +1523,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
goto fail;
}
- if (bdrv_dirty_bitmap_user_locked(bm)) {
- error_setg(errp, "Bitmap '%s' is in use", bitmap);
- goto fail;
- }
-
bdrv_dirty_bitmap_set_qmp_locked(bm, true);
exp->export_bitmap = bm;
exp->export_bitmap_context = g_strdup_printf("qemu:dirty-bitmap:%s",
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 06/17] block/dirty-bitmap: explicitly lock bitmaps with successors
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (4 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 05/17] nbd: change error checking order for bitmaps John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 07/17] block/dirty-bitmaps: unify qmp_locked and user_locked calls John Snow
` (14 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Instead of implying a user_locked/busy status, make it explicit.
Now, bitmaps in use by migration, NBD or backup operations
are all treated the same way with the same code paths.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-7-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 2e3192d86f..abc219814c 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -50,7 +50,7 @@ struct BdrvDirtyBitmap {
HBitmap *meta; /* Meta dirty bitmap */
bool qmp_locked; /* Bitmap is locked, it can't be modified
through QMP */
- BdrvDirtyBitmap *successor; /* Anonymous child; implies user_locked state */
+ BdrvDirtyBitmap *successor; /* Anonymous child, if any. */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap, in bytes */
bool disabled; /* Bitmap is disabled. It ignores all writes to
@@ -188,10 +188,8 @@ bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap)
return bitmap->successor;
}
-/* Both conditions disallow user-modification via QMP. */
bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) {
- return bdrv_dirty_bitmap_has_successor(bitmap) ||
- bdrv_dirty_bitmap_qmp_locked(bitmap);
+ return bdrv_dirty_bitmap_qmp_locked(bitmap);
}
void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked)
@@ -267,8 +265,9 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
child->disabled = bitmap->disabled;
bitmap->disabled = true;
- /* Install the successor and freeze the parent */
+ /* Install the successor and lock the parent */
bitmap->successor = child;
+ bitmap->qmp_locked = true;
return 0;
}
@@ -323,6 +322,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
bitmap->successor = NULL;
successor->persistent = bitmap->persistent;
bitmap->persistent = false;
+ bitmap->qmp_locked = false;
bdrv_release_dirty_bitmap(bs, bitmap);
return successor;
@@ -352,6 +352,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
}
parent->disabled = successor->disabled;
+ parent->qmp_locked = false;
bdrv_release_dirty_bitmap_locked(successor);
parent->successor = NULL;
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 07/17] block/dirty-bitmaps: unify qmp_locked and user_locked calls
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (5 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 06/17] block/dirty-bitmap: explicitly lock bitmaps with successors John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 08/17] block/dirty-bitmaps: move comment block John Snow
` (13 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
These mean the same thing now. Unify them and rename the merged call
bdrv_dirty_bitmap_busy to indicate semantically what we are describing,
as well as help disambiguate from the various _locked and _unlocked
versions of bitmap helpers that refer to mutex locks.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-8-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/block/dirty-bitmap.h | 5 ++--
block/dirty-bitmap.c | 42 +++++++++++++++-------------------
blockdev.c | 18 +++++++--------
migration/block-dirty-bitmap.c | 6 ++---
nbd/server.c | 6 ++---
5 files changed, 35 insertions(+), 42 deletions(-)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index cdbb4dfefd..ba8477b73f 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -68,7 +68,7 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
bool persistent);
-void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked);
+void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
HBitmap **backup, Error **errp);
void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration);
@@ -91,8 +91,7 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
-bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap);
-bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index abc219814c..d50e74d337 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -48,8 +48,7 @@ struct BdrvDirtyBitmap {
QemuMutex *mutex;
HBitmap *bitmap; /* Dirty bitmap implementation */
HBitmap *meta; /* Meta dirty bitmap */
- bool qmp_locked; /* Bitmap is locked, it can't be modified
- through QMP */
+ bool busy; /* Bitmap is busy, it can't be used via QMP */
BdrvDirtyBitmap *successor; /* Anonymous child, if any. */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap, in bytes */
@@ -188,22 +187,17 @@ bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap)
return bitmap->successor;
}
-bool bdrv_dirty_bitmap_user_locked(BdrvDirtyBitmap *bitmap) {
- return bdrv_dirty_bitmap_qmp_locked(bitmap);
+bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {
+ return bitmap->busy;
}
-void bdrv_dirty_bitmap_set_qmp_locked(BdrvDirtyBitmap *bitmap, bool qmp_locked)
+void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy)
{
qemu_mutex_lock(bitmap->mutex);
- bitmap->qmp_locked = qmp_locked;
+ bitmap->busy = busy;
qemu_mutex_unlock(bitmap->mutex);
}
-bool bdrv_dirty_bitmap_qmp_locked(BdrvDirtyBitmap *bitmap)
-{
- return bitmap->qmp_locked;
-}
-
/* Called with BQL taken. */
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
{
@@ -215,7 +209,7 @@ DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
{
if (bdrv_dirty_bitmap_has_successor(bitmap)) {
return DIRTY_BITMAP_STATUS_FROZEN;
- } else if (bdrv_dirty_bitmap_qmp_locked(bitmap)) {
+ } else if (bdrv_dirty_bitmap_busy(bitmap)) {
return DIRTY_BITMAP_STATUS_LOCKED;
} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
return DIRTY_BITMAP_STATUS_DISABLED;
@@ -233,7 +227,7 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)
/**
* Create a successor bitmap destined to replace this bitmap after an operation.
- * Requires that the bitmap is not user_locked and has no successor.
+ * Requires that the bitmap is not marked busy and has no successor.
* The successor will be enabled if the parent bitmap was.
* Called with BQL taken.
*/
@@ -243,7 +237,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
uint64_t granularity;
BdrvDirtyBitmap *child;
- if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ if (bdrv_dirty_bitmap_busy(bitmap)) {
error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
"by an operation");
return -1;
@@ -265,9 +259,9 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
child->disabled = bitmap->disabled;
bitmap->disabled = true;
- /* Install the successor and lock the parent */
+ /* Install the successor and mark the parent as busy */
bitmap->successor = child;
- bitmap->qmp_locked = true;
+ bitmap->busy = true;
return 0;
}
@@ -289,7 +283,7 @@ void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap)
static void bdrv_release_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap)
{
assert(!bitmap->active_iterators);
- assert(!bdrv_dirty_bitmap_user_locked(bitmap));
+ assert(!bdrv_dirty_bitmap_busy(bitmap));
assert(!bdrv_dirty_bitmap_has_successor(bitmap));
assert(!bitmap->meta);
QLIST_REMOVE(bitmap, list);
@@ -322,7 +316,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
bitmap->successor = NULL;
successor->persistent = bitmap->persistent;
bitmap->persistent = false;
- bitmap->qmp_locked = false;
+ bitmap->busy = false;
bdrv_release_dirty_bitmap(bs, bitmap);
return successor;
@@ -331,7 +325,7 @@ BdrvDirtyBitmap *bdrv_dirty_bitmap_abdicate(BlockDriverState *bs,
/**
* In cases of failure where we can no longer safely delete the parent,
* we may wish to re-join the parent and child/successor.
- * The merged parent will not be user_locked.
+ * The merged parent will be marked as not busy.
* The marged parent will be enabled if and only if the successor was enabled.
* Called within bdrv_dirty_bitmap_lock..unlock and with BQL taken.
*/
@@ -352,7 +346,7 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs,
}
parent->disabled = successor->disabled;
- parent->qmp_locked = false;
+ parent->busy = false;
bdrv_release_dirty_bitmap_locked(successor);
parent->successor = NULL;
@@ -383,7 +377,7 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
bdrv_dirty_bitmaps_lock(bs);
QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
- assert(!bdrv_dirty_bitmap_user_locked(bitmap));
+ assert(!bdrv_dirty_bitmap_busy(bitmap));
assert(!bdrv_dirty_bitmap_has_successor(bitmap));
assert(!bitmap->active_iterators);
hbitmap_truncate(bitmap->bitmap, bytes);
@@ -402,7 +396,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap)
/**
* Release all named dirty bitmaps attached to a BDS (for use in bdrv_close()).
- * There must not be any user_locked bitmaps attached.
+ * There must not be any busy bitmaps attached.
* This function does not remove persistent bitmaps from the storage.
* Called with BQL taken.
*/
@@ -466,7 +460,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->name = g_strdup(bm->name);
info->status = bdrv_dirty_bitmap_status(bm);
info->recording = bdrv_dirty_bitmap_recording(bm);
- info->busy = bdrv_dirty_bitmap_user_locked(bm);
+ info->busy = bdrv_dirty_bitmap_busy(bm);
info->persistent = bm->persistent;
entry->value = info;
*plist = entry;
@@ -774,7 +768,7 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
qemu_mutex_lock(dest->mutex);
- if (bdrv_dirty_bitmap_user_locked(dest)) {
+ if (bdrv_dirty_bitmap_busy(dest)) {
error_setg(errp, "Bitmap '%s' is currently in use by another"
" operation and cannot be modified", dest->name);
goto out;
diff --git a/blockdev.c b/blockdev.c
index 7e6bf9955c..b4d790131e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2009,7 +2009,7 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+ if (bdrv_dirty_bitmap_busy(state->bitmap)) {
error_setg(errp, "Cannot modify a bitmap in use by another operation");
return;
} else if (bdrv_dirty_bitmap_readonly(state->bitmap)) {
@@ -2058,7 +2058,7 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+ if (bdrv_dirty_bitmap_busy(state->bitmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be enabled", action->name);
@@ -2099,7 +2099,7 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_user_locked(state->bitmap)) {
+ if (bdrv_dirty_bitmap_busy(state->bitmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be disabled", action->name);
@@ -2893,7 +2893,7 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ if (bdrv_dirty_bitmap_busy(bitmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation and"
" cannot be removed", name);
@@ -2932,7 +2932,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ if (bdrv_dirty_bitmap_busy(bitmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be cleared", name);
@@ -2956,7 +2956,7 @@ void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ if (bdrv_dirty_bitmap_busy(bitmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be enabled", name);
@@ -2977,7 +2977,7 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ if (bdrv_dirty_bitmap_busy(bitmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be disabled", name);
@@ -3559,7 +3559,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
bdrv_unref(target_bs);
goto out;
}
- if (bdrv_dirty_bitmap_user_locked(bmap)) {
+ if (bdrv_dirty_bitmap_busy(bmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be used for backup", backup->bitmap);
@@ -3672,7 +3672,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
goto out;
}
- if (bdrv_dirty_bitmap_user_locked(bmap)) {
+ if (bdrv_dirty_bitmap_busy(bmap)) {
error_setg(errp,
"Bitmap '%s' is currently in use by another operation"
" and cannot be used for backup", backup->bitmap);
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index ac6954142f..7057fff242 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -261,7 +261,7 @@ static void dirty_bitmap_mig_cleanup(void)
while ((dbms = QSIMPLEQ_FIRST(&dirty_bitmap_mig_state.dbms_list)) != NULL) {
QSIMPLEQ_REMOVE_HEAD(&dirty_bitmap_mig_state.dbms_list, entry);
- bdrv_dirty_bitmap_set_qmp_locked(dbms->bitmap, false);
+ bdrv_dirty_bitmap_set_busy(dbms->bitmap, false);
bdrv_unref(dbms->bs);
g_free(dbms);
}
@@ -301,7 +301,7 @@ static int init_dirty_bitmap_migration(void)
goto fail;
}
- if (bdrv_dirty_bitmap_user_locked(bitmap)) {
+ if (bdrv_dirty_bitmap_busy(bitmap)) {
error_report("Can't migrate a bitmap that is in use by another operation: '%s'",
bdrv_dirty_bitmap_name(bitmap));
goto fail;
@@ -314,7 +314,7 @@ static int init_dirty_bitmap_migration(void)
}
bdrv_ref(bs);
- bdrv_dirty_bitmap_set_qmp_locked(bitmap, true);
+ bdrv_dirty_bitmap_set_busy(bitmap, true);
dbms = g_new0(DirtyBitmapMigBitmapState, 1);
dbms->bs = bs;
diff --git a/nbd/server.c b/nbd/server.c
index de21c64ce3..02773e2836 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1510,7 +1510,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
goto fail;
}
- if (bdrv_dirty_bitmap_user_locked(bm)) {
+ if (bdrv_dirty_bitmap_busy(bm)) {
error_setg(errp, "Bitmap '%s' is in use", bitmap);
goto fail;
}
@@ -1523,7 +1523,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
goto fail;
}
- bdrv_dirty_bitmap_set_qmp_locked(bm, true);
+ bdrv_dirty_bitmap_set_busy(bm, true);
exp->export_bitmap = bm;
exp->export_bitmap_context = g_strdup_printf("qemu:dirty-bitmap:%s",
bitmap);
@@ -1641,7 +1641,7 @@ void nbd_export_put(NBDExport *exp)
}
if (exp->export_bitmap) {
- bdrv_dirty_bitmap_set_qmp_locked(exp->export_bitmap, false);
+ bdrv_dirty_bitmap_set_busy(exp->export_bitmap, false);
g_free(exp->export_bitmap_context);
}
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 08/17] block/dirty-bitmaps: move comment block
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (6 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 07/17] block/dirty-bitmaps: unify qmp_locked and user_locked calls John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 09/17] blockdev: remove unused paio parameter documentation John Snow
` (12 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Simply move the big status enum comment block to above the status
function, and document it as being deprecated. The whole confusing
block can get deleted in three releases time.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-9-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index d50e74d337..980cae4fa3 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -28,22 +28,6 @@
#include "block/block_int.h"
#include "block/blockjob.h"
-/**
- * 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;
HBitmap *bitmap; /* Dirty bitmap implementation */
@@ -204,7 +188,25 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
return !bitmap->disabled;
}
-/* Called with BQL taken. */
+/**
+ * bdrv_dirty_bitmap_status: This API is now deprecated.
+ * Called with BQL taken.
+ *
+ * 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),
+ * or 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.
+ */
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
{
if (bdrv_dirty_bitmap_has_successor(bitmap)) {
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 09/17] blockdev: remove unused paio parameter documentation
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (7 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 08/17] block/dirty-bitmaps: move comment block John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 10/17] iotests: add busy/recording bit test to 124 John Snow
` (11 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
This field isn't present anymore.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190223000614.13894-10-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index b4d790131e..8e002cf681 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1255,7 +1255,6 @@ out_aio_context:
* @node: The name of the BDS node to search for bitmaps
* @name: The name of the bitmap to search for
* @pbs: Output pointer for BDS lookup, if desired. Can be NULL.
- * @paio: Output pointer for aio_context acquisition, if desired. Can be NULL.
* @errp: Output pointer for error information. Can be NULL.
*
* @return: A bitmap object on success, or NULL on failure.
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 10/17] iotests: add busy/recording bit test to 124
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (8 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 09/17] blockdev: remove unused paio parameter documentation John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 11/17] block/dirty-bitmaps: add inconsistent bit John Snow
` (10 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
This adds a simple test that ensures the busy bit works for push backups,
as well as doubling as bonus test for incremental backups that get interrupted
by EIO errors.
Recording bit tests are already handled sufficiently by 236.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Message-id: 20190223000614.13894-11-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
tests/qemu-iotests/124 | 113 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/124.out | 4 +-
2 files changed, 115 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index 5aa1bf1bd6..80b356f7bb 100755
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -634,6 +634,119 @@ class TestIncrementalBackupBlkdebug(TestIncrementalBackupBase):
self.vm.shutdown()
self.check_backups()
+ def test_incremental_pause(self):
+ """
+ Test an incremental backup that errors into a pause and is resumed.
+ """
+
+ drive0 = self.drives[0]
+ # NB: The blkdebug script here looks for a "flush, read, read" pattern.
+ # The flush occurs in hmp_io_writes, the first read in device_add, and
+ # the last read during the block job.
+ result = self.vm.qmp('blockdev-add',
+ node_name=drive0['id'],
+ driver=drive0['fmt'],
+ file={
+ 'driver': 'blkdebug',
+ 'image': {
+ 'driver': 'file',
+ 'filename': drive0['file']
+ },
+ 'set-state': [{
+ 'event': 'flush_to_disk',
+ 'state': 1,
+ 'new_state': 2
+ },{
+ 'event': 'read_aio',
+ 'state': 2,
+ 'new_state': 3
+ }],
+ 'inject-error': [{
+ 'event': 'read_aio',
+ 'errno': 5,
+ 'state': 3,
+ 'immediately': False,
+ 'once': True
+ }],
+ })
+ self.assert_qmp(result, 'return', {})
+ self.create_anchor_backup(drive0)
+ bitmap = self.add_bitmap('bitmap0', drive0)
+
+ # Emulate guest activity
+ self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
+ ('0xfe', '16M', '256k'),
+ ('0x64', '32736k', '64k')))
+
+ # For the purposes of query-block visibility of bitmaps, add a drive
+ # frontend after we've written data; otherwise we can't use hmp-io
+ result = self.vm.qmp("device_add",
+ id="device0",
+ drive=drive0['id'],
+ driver="virtio-blk")
+ self.assert_qmp(result, 'return', {})
+
+ # Bitmap Status Check
+ query = self.vm.qmp('query-block')
+ ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
+ if bmap.get('name') == bitmap.name][0]
+ self.assert_qmp(ret, 'count', 458752)
+ self.assert_qmp(ret, 'granularity', 65536)
+ self.assert_qmp(ret, 'status', 'active')
+ self.assert_qmp(ret, 'busy', False)
+ self.assert_qmp(ret, 'recording', True)
+
+ # Start backup
+ parent, _ = bitmap.last_target()
+ target = self.prepare_backup(bitmap, parent)
+ res = self.vm.qmp('drive-backup',
+ job_id=bitmap.drive['id'],
+ device=bitmap.drive['id'],
+ sync='incremental',
+ bitmap=bitmap.name,
+ format=bitmap.drive['fmt'],
+ target=target,
+ mode='existing',
+ on_source_error='stop')
+ self.assert_qmp(res, 'return', {})
+
+ # Wait for the error
+ event = self.vm.event_wait(name="BLOCK_JOB_ERROR",
+ match={"data":{"device":bitmap.drive['id']}})
+ self.assert_qmp(event, 'data', {'device': bitmap.drive['id'],
+ 'action': 'stop',
+ 'operation': 'read'})
+
+ # Bitmap Status Check
+ query = self.vm.qmp('query-block')
+ ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
+ if bmap.get('name') == bitmap.name][0]
+ self.assert_qmp(ret, 'count', 458752)
+ self.assert_qmp(ret, 'granularity', 65536)
+ self.assert_qmp(ret, 'status', 'frozen')
+ self.assert_qmp(ret, 'busy', True)
+ self.assert_qmp(ret, 'recording', True)
+
+ # Resume and check incremental backup for consistency
+ res = self.vm.qmp('block-job-resume', device=bitmap.drive['id'])
+ self.assert_qmp(res, 'return', {})
+ self.wait_qmp_backup(bitmap.drive['id'])
+
+ # Bitmap Status Check
+ query = self.vm.qmp('query-block')
+ ret = [bmap for bmap in query['return'][0]['dirty-bitmaps']
+ if bmap.get('name') == bitmap.name][0]
+ self.assert_qmp(ret, 'count', 0)
+ self.assert_qmp(ret, 'granularity', 65536)
+ self.assert_qmp(ret, 'status', 'active')
+ self.assert_qmp(ret, 'busy', False)
+ self.assert_qmp(ret, 'recording', True)
+
+ # Finalize / Cleanup
+ self.make_reference_backup(bitmap)
+ self.vm.shutdown()
+ self.check_backups()
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'])
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index e56cae021b..281b69efea 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-...........
+............
----------------------------------------------------------------------
-Ran 11 tests
+Ran 12 tests
OK
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 11/17] block/dirty-bitmaps: add inconsistent bit
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (9 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 10/17] iotests: add busy/recording bit test to 124 John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 12/17] block/dirty-bitmap: add inconsistent status John Snow
` (9 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Add an inconsistent bit to dirty-bitmaps that allows us to report a bitmap as
persistent but potentially inconsistent, i.e. if we find bitmaps on a qcow2
that have been marked as "in use".
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190301191545.8728-2-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
qapi/block-core.json | 13 +++++++++----
include/block/dirty-bitmap.h | 2 ++
block/dirty-bitmap.c | 20 ++++++++++++++++++++
3 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 6e543594b3..e639ef6d1c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -467,15 +467,20 @@
# and cannot be modified via QMP or used by another operation.
# Replaces `locked` and `frozen` statuses. (since 4.0)
#
-# @persistent: true if the bitmap will eventually be flushed to persistent
-# storage (since 4.0)
+# @persistent: true if the bitmap was stored on disk, is scheduled to be stored
+# on disk, or both. (since 4.0)
+#
+# @inconsistent: true if this is a persistent bitmap that was improperly
+# stored. Implies @persistent to be true; @recording and
+# @busy to be false. This bitmap cannot be used. To remove
+# it, use @block-dirty-bitmap-remove. (Since 4.0)
#
# Since: 1.3
##
{ 'struct': 'BlockDirtyInfo',
'data': {'*name': 'str', 'count': 'int', 'granularity': 'uint32',
- 'recording': 'bool', 'busy': 'bool',
- 'status': 'DirtyBitmapStatus', 'persistent': 'bool' } }
+ 'recording': 'bool', 'busy': 'bool', 'status': 'DirtyBitmapStatus',
+ 'persistent': 'bool', '*inconsistent': 'bool' } }
##
# @Qcow2BitmapInfoFlags:
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index ba8477b73f..bd1b6479df 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -68,6 +68,7 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_set_readonly(BdrvDirtyBitmap *bitmap, bool value);
void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap,
bool persistent);
+void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap);
void bdrv_dirty_bitmap_set_busy(BdrvDirtyBitmap *bitmap, bool busy);
void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
HBitmap **backup, Error **errp);
@@ -91,6 +92,7 @@ bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
+bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 980cae4fa3..c611bfb971 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -46,6 +46,9 @@ struct BdrvDirtyBitmap {
and this bitmap must remain unchanged while
this flag is set. */
bool persistent; /* bitmap must be saved to owner disk image */
+ bool inconsistent; /* bitmap is persistent, but inconsistent.
+ * It cannot be used at all in any way, except
+ * a QMP user can remove it. */
bool migration; /* Bitmap is selected for migration, it should
not be stored on the next inactivation
(persistent flag doesn't matter until next
@@ -464,6 +467,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
info->recording = bdrv_dirty_bitmap_recording(bm);
info->busy = bdrv_dirty_bitmap_busy(bm);
info->persistent = bm->persistent;
+ info->has_inconsistent = bm->inconsistent;
+ info->inconsistent = bm->inconsistent;
entry->value = info;
*plist = entry;
plist = &entry->next;
@@ -711,6 +716,16 @@ void bdrv_dirty_bitmap_set_persistance(BdrvDirtyBitmap *bitmap, bool persistent)
qemu_mutex_unlock(bitmap->mutex);
}
+/* Called with BQL taken. */
+void bdrv_dirty_bitmap_set_inconsistent(BdrvDirtyBitmap *bitmap)
+{
+ qemu_mutex_lock(bitmap->mutex);
+ assert(bitmap->persistent == true);
+ bitmap->inconsistent = true;
+ bitmap->disabled = true;
+ qemu_mutex_unlock(bitmap->mutex);
+}
+
/* Called with BQL taken. */
void bdrv_dirty_bitmap_set_migration(BdrvDirtyBitmap *bitmap, bool migration)
{
@@ -724,6 +739,11 @@ bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap)
return bitmap->persistent && !bitmap->migration;
}
+bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap)
+{
+ return bitmap->inconsistent;
+}
+
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
{
BdrvDirtyBitmap *bm;
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 12/17] block/dirty-bitmap: add inconsistent status
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (10 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 11/17] block/dirty-bitmaps: add inconsistent bit John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 13/17] block/dirty-bitmaps: add block_dirty_bitmap_check function John Snow
` (8 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Even though the status field is deprecated, we still have to support
it for a few more releases. Since this is a very new kind of bitmap
state, it makes sense for it to have its own status field.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190301191545.8728-3-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
qapi/block-core.json | 7 ++++++-
block/dirty-bitmap.c | 7 ++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index e639ef6d1c..ae55cd0704 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -442,10 +442,15 @@
# recording new writes. If the bitmap was @disabled, it is not
# recording new writes. (Since 2.12)
#
+# @inconsistent: This is a persistent dirty bitmap that was marked in-use on
+# disk, and is unusable by QEMU. It can only be deleted.
+# Please rely on the inconsistent field in @BlockDirtyInfo
+# instead, as the status field is deprecated. (Since 4.0)
+#
# Since: 2.4
##
{ 'enum': 'DirtyBitmapStatus',
- 'data': ['active', 'disabled', 'frozen', 'locked'] }
+ 'data': ['active', 'disabled', 'frozen', 'locked', 'inconsistent'] }
##
# @BlockDirtyInfo:
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index c611bfb971..987ea7ca29 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -209,10 +209,15 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
* or 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.
+ * (5) Inconsistent: This is a persistent bitmap whose "in use" bit is set, and
+ * is unusable by QEMU. It can be deleted to remove it from
+ * the qcow2.
*/
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
{
- if (bdrv_dirty_bitmap_has_successor(bitmap)) {
+ if (bdrv_dirty_bitmap_inconsistent(bitmap)) {
+ return DIRTY_BITMAP_STATUS_INCONSISTENT;
+ } else if (bdrv_dirty_bitmap_has_successor(bitmap)) {
return DIRTY_BITMAP_STATUS_FROZEN;
} else if (bdrv_dirty_bitmap_busy(bitmap)) {
return DIRTY_BITMAP_STATUS_LOCKED;
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 13/17] block/dirty-bitmaps: add block_dirty_bitmap_check function
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (11 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 12/17] block/dirty-bitmap: add inconsistent status John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 14/17] block/dirty-bitmaps: prohibit readonly bitmaps for backups John Snow
` (7 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Instead of checking against busy, inconsistent, or read only directly,
use a check function with permissions bits that let us streamline the
checks without reproducing them in many places.
Included in this patch are permissions changes that simply add the
inconsistent check to existing permissions call spots, without
addressing existing bugs.
In general, this means that busy+readonly checks become BDRV_BITMAP_DEFAULT,
which checks against all three conditions. busy-only checks become
BDRV_BITMAP_ALLOW_RO.
Notably, remove allows inconsistent bitmaps, so it doesn't follow the pattern.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190301191545.8728-4-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
include/block/dirty-bitmap.h | 13 ++++++++-
block/dirty-bitmap.c | 42 ++++++++++++++++++++---------
blockdev.c | 49 +++++++---------------------------
migration/block-dirty-bitmap.c | 12 +++------
nbd/server.c | 3 +--
5 files changed, 55 insertions(+), 64 deletions(-)
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index bd1b6479df..2a78243954 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -5,6 +5,16 @@
#include "qapi/qapi-types-block-core.h"
#include "qemu/hbitmap.h"
+typedef enum BitmapCheckFlags {
+ BDRV_BITMAP_BUSY = 1,
+ BDRV_BITMAP_RO = 2,
+ BDRV_BITMAP_INCONSISTENT = 4,
+} BitmapCheckFlags;
+
+#define BDRV_BITMAP_DEFAULT (BDRV_BITMAP_BUSY | BDRV_BITMAP_RO | \
+ BDRV_BITMAP_INCONSISTENT)
+#define BDRV_BITMAP_ALLOW_RO (BDRV_BITMAP_BUSY | BDRV_BITMAP_INCONSISTENT)
+
BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
uint32_t granularity,
const char *name,
@@ -24,6 +34,8 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
void bdrv_dirty_bitmap_enable_successor(BdrvDirtyBitmap *bitmap);
BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs,
const char *name);
+int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags,
+ Error **errp);
void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap);
void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs);
void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs,
@@ -93,7 +105,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_get_persistance(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap);
-bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap);
bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap);
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 987ea7ca29..6b6fed1363 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -174,7 +174,7 @@ bool bdrv_dirty_bitmap_has_successor(BdrvDirtyBitmap *bitmap)
return bitmap->successor;
}
-bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {
+static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) {
return bitmap->busy;
}
@@ -235,6 +235,33 @@ static bool bdrv_dirty_bitmap_recording(BdrvDirtyBitmap *bitmap)
!bitmap->successor->disabled);
}
+int bdrv_dirty_bitmap_check(const BdrvDirtyBitmap *bitmap, uint32_t flags,
+ Error **errp)
+{
+ if ((flags & BDRV_BITMAP_BUSY) && bdrv_dirty_bitmap_busy(bitmap)) {
+ error_setg(errp, "Bitmap '%s' is currently in use by another"
+ " operation and cannot be used", bitmap->name);
+ return -1;
+ }
+
+ if ((flags & BDRV_BITMAP_RO) && bdrv_dirty_bitmap_readonly(bitmap)) {
+ error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
+ bitmap->name);
+ return -1;
+ }
+
+ if ((flags & BDRV_BITMAP_INCONSISTENT) &&
+ bdrv_dirty_bitmap_inconsistent(bitmap)) {
+ error_setg(errp, "Bitmap '%s' is inconsistent and cannot be used",
+ bitmap->name);
+ error_append_hint(errp, "Try block-dirty-bitmap-remove to delete"
+ " this bitmap from disk");
+ return -1;
+ }
+
+ return 0;
+}
+
/**
* Create a successor bitmap destined to replace this bitmap after an operation.
* Requires that the bitmap is not marked busy and has no successor.
@@ -247,9 +274,7 @@ int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
uint64_t granularity;
BdrvDirtyBitmap *child;
- if (bdrv_dirty_bitmap_busy(bitmap)) {
- error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
- "by an operation");
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY, errp)) {
return -1;
}
if (bdrv_dirty_bitmap_has_successor(bitmap)) {
@@ -795,17 +820,10 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
qemu_mutex_lock(dest->mutex);
- if (bdrv_dirty_bitmap_busy(dest)) {
- error_setg(errp, "Bitmap '%s' is currently in use by another"
- " operation and cannot be modified", dest->name);
+ if (bdrv_dirty_bitmap_check(dest, BDRV_BITMAP_DEFAULT, errp)) {
goto out;
}
- if (bdrv_dirty_bitmap_readonly(dest)) {
- error_setg(errp, "Bitmap '%s' is readonly and cannot be modified",
- dest->name);
- goto out;
- }
if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
error_setg(errp, "Bitmaps are incompatible and can't be merged");
diff --git a/blockdev.c b/blockdev.c
index 8e002cf681..455ab806b2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2008,11 +2008,7 @@ static void block_dirty_bitmap_clear_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_busy(state->bitmap)) {
- error_setg(errp, "Cannot modify a bitmap in use by another operation");
- return;
- } else if (bdrv_dirty_bitmap_readonly(state->bitmap)) {
- error_setg(errp, "Cannot clear a readonly bitmap");
+ if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_DEFAULT, errp)) {
return;
}
@@ -2057,10 +2053,7 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_busy(state->bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be enabled", action->name);
+ if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
return;
}
@@ -2098,10 +2091,7 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
return;
}
- if (bdrv_dirty_bitmap_busy(state->bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be disabled", action->name);
+ if (bdrv_dirty_bitmap_check(state->bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
return;
}
@@ -2892,10 +2882,7 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_busy(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation and"
- " cannot be removed", name);
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY, errp)) {
return;
}
@@ -2931,13 +2918,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_busy(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be cleared", name);
- return;
- } else if (bdrv_dirty_bitmap_readonly(bitmap)) {
- error_setg(errp, "Bitmap '%s' is readonly and cannot be cleared", name);
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) {
return;
}
@@ -2955,10 +2936,7 @@ void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_busy(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be enabled", name);
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
return;
}
@@ -2976,10 +2954,7 @@ void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_busy(bitmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be disabled", name);
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
return;
}
@@ -3558,10 +3533,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
bdrv_unref(target_bs);
goto out;
}
- if (bdrv_dirty_bitmap_busy(bmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be used for backup", backup->bitmap);
+ if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_ALLOW_RO, errp)) {
goto out;
}
}
@@ -3671,10 +3643,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
goto out;
}
- if (bdrv_dirty_bitmap_busy(bmap)) {
- error_setg(errp,
- "Bitmap '%s' is currently in use by another operation"
- " and cannot be used for backup", backup->bitmap);
+ if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_ALLOW_RO, errp)) {
goto out;
}
}
diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 7057fff242..0fcf897f32 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -274,6 +274,7 @@ static int init_dirty_bitmap_migration(void)
BdrvDirtyBitmap *bitmap;
DirtyBitmapMigBitmapState *dbms;
BdrvNextIterator it;
+ Error *local_err = NULL;
dirty_bitmap_mig_state.bulk_completed = false;
dirty_bitmap_mig_state.prev_bs = NULL;
@@ -301,15 +302,8 @@ static int init_dirty_bitmap_migration(void)
goto fail;
}
- if (bdrv_dirty_bitmap_busy(bitmap)) {
- error_report("Can't migrate a bitmap that is in use by another operation: '%s'",
- bdrv_dirty_bitmap_name(bitmap));
- goto fail;
- }
-
- if (bdrv_dirty_bitmap_readonly(bitmap)) {
- error_report("Can't migrate read-only dirty bitmap: '%s",
- bdrv_dirty_bitmap_name(bitmap));
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
+ error_report_err(local_err);
goto fail;
}
diff --git a/nbd/server.c b/nbd/server.c
index 02773e2836..9b87c7f243 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1510,8 +1510,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
goto fail;
}
- if (bdrv_dirty_bitmap_busy(bm)) {
- error_setg(errp, "Bitmap '%s' is in use", bitmap);
+ if (bdrv_dirty_bitmap_check(bm, BDRV_BITMAP_ALLOW_RO, errp)) {
goto fail;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 14/17] block/dirty-bitmaps: prohibit readonly bitmaps for backups
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (12 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 13/17] block/dirty-bitmaps: add block_dirty_bitmap_check function John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 15/17] block/dirty-bitmaps: prohibit removing readonly bitmaps John Snow
` (6 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
drive and blockdev backup cannot use readonly bitmaps, because the
sync=incremental mechanism actually edits the bitmaps on success.
If you really want to do this operation, use a copied bitmap.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190301191545.8728-5-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 455ab806b2..c6c7a64bfc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3533,7 +3533,7 @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
bdrv_unref(target_bs);
goto out;
}
- if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_ALLOW_RO, errp)) {
+ if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_DEFAULT, errp)) {
goto out;
}
}
@@ -3643,7 +3643,7 @@ BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);
goto out;
}
- if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_ALLOW_RO, errp)) {
+ if (bdrv_dirty_bitmap_check(bmap, BDRV_BITMAP_DEFAULT, errp)) {
goto out;
}
}
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 15/17] block/dirty-bitmaps: prohibit removing readonly bitmaps
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (13 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 14/17] block/dirty-bitmaps: prohibit readonly bitmaps for backups John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 16/17] block/dirty-bitmaps: disallow busy bitmaps as merge source John Snow
` (5 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Remove is an inherently RW operation, so this will fail anyway, but
we can fail it very quickly instead of trying and failing, so do so.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190301191545.8728-6-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
blockdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index c6c7a64bfc..51fcfb7faf 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2882,7 +2882,8 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name,
return;
}
- if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY, errp)) {
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY | BDRV_BITMAP_RO,
+ errp)) {
return;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 16/17] block/dirty-bitmaps: disallow busy bitmaps as merge source
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (14 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 15/17] block/dirty-bitmaps: prohibit removing readonly bitmaps John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 17/17] block/dirty-bitmaps: implement inconsistent bit John Snow
` (4 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
We didn't do any state checking on source bitmaps at all,
so this adds inconsistent and busy checks. readonly is
allowed, so you can still copy a readonly bitmap to a new
destination to use it for operations like drive-backup.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20190301191545.8728-7-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/dirty-bitmap.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 6b6fed1363..59c403c3fd 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -824,6 +824,9 @@ void bdrv_merge_dirty_bitmap(BdrvDirtyBitmap *dest, const BdrvDirtyBitmap *src,
goto out;
}
+ if (bdrv_dirty_bitmap_check(src, BDRV_BITMAP_ALLOW_RO, errp)) {
+ goto out;
+ }
if (!hbitmap_can_merge(dest->bitmap, src->bitmap)) {
error_setg(errp, "Bitmaps are incompatible and can't be merged");
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PULL 17/17] block/dirty-bitmaps: implement inconsistent bit
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (15 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 16/17] block/dirty-bitmaps: disallow busy bitmaps as merge source John Snow
@ 2019-03-08 20:28 ` John Snow
2019-03-08 20:46 ` [Qemu-devel] [PULL 00/17] Bitmaps patches no-reply
` (3 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 20:28 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, peter.maydell
Set the inconsistent bit on load instead of rejecting such bitmaps.
There is no way to un-set it; the only option is to delete the bitmap.
Obvervations:
- bitmap loading does not need to update the header for in_use bitmaps.
- inconsistent bitmaps don't need to have their data loaded; they're
glorified corruption sentinels.
- bitmap saving does not need to save inconsistent bitmaps back to disk.
- bitmap reopening DOES need to drop the readonly flag from inconsistent
bitmaps to allow reopening of qcow2 files with non-qemu-owned bitmaps
being eventually flushed back to disk.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20190301191545.8728-8-jsnow@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
---
block/qcow2-bitmap.c | 103 ++++++++++++++++++++++---------------------
1 file changed, 53 insertions(+), 50 deletions(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 3ee524da4b..80926966de 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -343,9 +343,15 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
uint32_t granularity;
BdrvDirtyBitmap *bitmap = NULL;
+ granularity = 1U << bm->granularity_bits;
+ bitmap = bdrv_create_dirty_bitmap(bs, granularity, bm->name, errp);
+ if (bitmap == NULL) {
+ goto fail;
+ }
+
if (bm->flags & BME_FLAG_IN_USE) {
- error_setg(errp, "Bitmap '%s' is in use", bm->name);
- goto fail;
+ /* Data is unusable, skip loading it */
+ return bitmap;
}
ret = bitmap_table_load(bs, &bm->table, &bitmap_table);
@@ -356,12 +362,6 @@ static BdrvDirtyBitmap *load_bitmap(BlockDriverState *bs,
goto fail;
}
- granularity = 1U << bm->granularity_bits;
- bitmap = bdrv_create_dirty_bitmap(bs, granularity, bm->name, errp);
- if (bitmap == NULL) {
- goto fail;
- }
-
ret = load_bitmap_data(bs, bitmap_table, bm->table.size, bitmap);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not read bitmap '%s' from image",
@@ -949,6 +949,7 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp)
Qcow2Bitmap *bm;
GSList *created_dirty_bitmaps = NULL;
bool header_updated = false;
+ bool needs_update = false;
if (s->nb_bitmaps == 0) {
/* No bitmaps - nothing to do */
@@ -962,35 +963,39 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp)
}
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
- if (!(bm->flags & BME_FLAG_IN_USE)) {
- BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp);
- if (bitmap == NULL) {
- goto fail;
- }
-
- if (!(bm->flags & BME_FLAG_AUTO)) {
- bdrv_disable_dirty_bitmap(bitmap);
- }
- bdrv_dirty_bitmap_set_persistance(bitmap, true);
- bm->flags |= BME_FLAG_IN_USE;
- created_dirty_bitmaps =
- g_slist_append(created_dirty_bitmaps, bitmap);
+ BdrvDirtyBitmap *bitmap = load_bitmap(bs, bm, errp);
+ if (bitmap == NULL) {
+ goto fail;
}
- }
- if (created_dirty_bitmaps != NULL) {
- if (can_write(bs)) {
- /* in_use flags must be updated */
- int ret = update_ext_header_and_dir_in_place(bs, bm_list);
- if (ret < 0) {
- error_setg_errno(errp, -ret, "Can't update bitmap directory");
- goto fail;
- }
- header_updated = true;
+ bdrv_dirty_bitmap_set_persistance(bitmap, true);
+ if (bm->flags & BME_FLAG_IN_USE) {
+ bdrv_dirty_bitmap_set_inconsistent(bitmap);
} else {
- g_slist_foreach(created_dirty_bitmaps, set_readonly_helper,
- (gpointer)true);
+ /* NB: updated flags only get written if can_write(bs) is true. */
+ bm->flags |= BME_FLAG_IN_USE;
+ needs_update = true;
}
+ if (!(bm->flags & BME_FLAG_AUTO)) {
+ bdrv_disable_dirty_bitmap(bitmap);
+ }
+ created_dirty_bitmaps =
+ g_slist_append(created_dirty_bitmaps, bitmap);
+ }
+
+ if (needs_update && can_write(bs)) {
+ /* in_use flags must be updated */
+ int ret = update_ext_header_and_dir_in_place(bs, bm_list);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Can't update bitmap directory");
+ goto fail;
+ }
+ header_updated = true;
+ }
+
+ if (!can_write(bs)) {
+ g_slist_foreach(created_dirty_bitmaps, set_readonly_helper,
+ (gpointer)true);
}
g_slist_free(created_dirty_bitmaps);
@@ -1112,23 +1117,21 @@ int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated,
}
QSIMPLEQ_FOREACH(bm, bm_list, entry) {
- if (!(bm->flags & BME_FLAG_IN_USE)) {
- BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(bs, bm->name);
- if (bitmap == NULL) {
- continue;
- }
-
- if (!bdrv_dirty_bitmap_readonly(bitmap)) {
- error_setg(errp, "Bitmap %s is not readonly but not marked"
- "'IN_USE' in the image. Something went wrong,"
- "all the bitmaps may be corrupted", bm->name);
- ret = -EINVAL;
- goto out;
- }
+ BdrvDirtyBitmap *bitmap = bdrv_find_dirty_bitmap(bs, bm->name);
+ if (bitmap == NULL) {
+ continue;
+ }
- bm->flags |= BME_FLAG_IN_USE;
- ro_dirty_bitmaps = g_slist_append(ro_dirty_bitmaps, bitmap);
+ if (!bdrv_dirty_bitmap_readonly(bitmap)) {
+ error_setg(errp, "Bitmap %s was loaded prior to rw-reopen, but was "
+ "not marked as readonly. This is a bug, something went "
+ "wrong. All of the bitmaps may be corrupted", bm->name);
+ ret = -EINVAL;
+ goto out;
}
+
+ bm->flags |= BME_FLAG_IN_USE;
+ ro_dirty_bitmaps = g_slist_append(ro_dirty_bitmaps, bitmap);
}
if (ro_dirty_bitmaps != NULL) {
@@ -1424,8 +1427,8 @@ void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
Qcow2Bitmap *bm;
if (!bdrv_dirty_bitmap_get_persistance(bitmap) ||
- bdrv_dirty_bitmap_readonly(bitmap))
- {
+ bdrv_dirty_bitmap_readonly(bitmap) ||
+ bdrv_dirty_bitmap_inconsistent(bitmap)) {
continue;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PULL 00/17] Bitmaps patches
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (16 preceding siblings ...)
2019-03-08 20:28 ` [Qemu-devel] [PULL 17/17] block/dirty-bitmaps: implement inconsistent bit John Snow
@ 2019-03-08 20:46 ` no-reply
2019-03-08 20:50 ` no-reply
` (2 subsequent siblings)
20 siblings, 0 replies; 23+ messages in thread
From: no-reply @ 2019-03-08 20:46 UTC (permalink / raw)
To: jsnow; +Cc: fam, qemu-devel, peter.maydell
Patchew URL: https://patchew.org/QEMU/20190308202858.26636-1-jsnow@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20190308202858.26636-1-jsnow@redhat.com
Subject: [Qemu-devel] [PULL 00/17] Bitmaps patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20190308202858.26636-1-jsnow@redhat.com -> patchew/20190308202858.26636-1-jsnow@redhat.com
Switched to a new branch 'test'
6a083e7a44 block/dirty-bitmaps: implement inconsistent bit
f00e672dd7 block/dirty-bitmaps: disallow busy bitmaps as merge source
ca8fdb5e18 block/dirty-bitmaps: prohibit removing readonly bitmaps
4085064d76 block/dirty-bitmaps: prohibit readonly bitmaps for backups
46e4307e40 block/dirty-bitmaps: add block_dirty_bitmap_check function
e0e53b95d2 block/dirty-bitmap: add inconsistent status
061152794d block/dirty-bitmaps: add inconsistent bit
20eb6ea4b5 iotests: add busy/recording bit test to 124
f0dea169fe blockdev: remove unused paio parameter documentation
5b090a904b block/dirty-bitmaps: move comment block
235c618265 block/dirty-bitmaps: unify qmp_locked and user_locked calls
9cd31200c1 block/dirty-bitmap: explicitly lock bitmaps with successors
0260a9c1c5 nbd: change error checking order for bitmaps
80fbc5114e block/dirty-bitmap: change semantics of enabled predicate
62a0dda136 block/dirty-bitmap: remove set/reset assertions against enabled bit
4d10356687 block/dirty-bitmaps: rename frozen predicate helper
8e21a1f266 block/dirty-bitmap: add recording and busy properties
=== OUTPUT BEGIN ===
1/17 Checking commit 8e21a1f266ae (block/dirty-bitmap: add recording and busy properties)
2/17 Checking commit 4d10356687c2 (block/dirty-bitmaps: rename frozen predicate helper)
WARNING: line over 80 characters
#87: FILE: block/dirty-bitmap.c:248:
+ error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
total: 0 errors, 1 warnings, 124 lines checked
Patch 2/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/17 Checking commit 62a0dda1369b (block/dirty-bitmap: remove set/reset assertions against enabled bit)
4/17 Checking commit 80fbc5114eb1 (block/dirty-bitmap: change semantics of enabled predicate)
5/17 Checking commit 0260a9c1c566 (nbd: change error checking order for bitmaps)
6/17 Checking commit 9cd31200c142 (block/dirty-bitmap: explicitly lock bitmaps with successors)
7/17 Checking commit 235c61826568 (block/dirty-bitmaps: unify qmp_locked and user_locked calls)
ERROR: open brace '{' following function declarations go on the next line
#39: FILE: block/dirty-bitmap.c:190:
+bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {
total: 1 errors, 0 warnings, 271 lines checked
Patch 7/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/17 Checking commit 5b090a904b91 (block/dirty-bitmaps: move comment block)
9/17 Checking commit f0dea169fe5e (blockdev: remove unused paio parameter documentation)
10/17 Checking commit 20eb6ea4b57a (iotests: add busy/recording bit test to 124)
11/17 Checking commit 061152794d63 (block/dirty-bitmaps: add inconsistent bit)
WARNING: Block comments use a leading /* on a separate line
#26: FILE: block/dirty-bitmap.c:49:
+ bool inconsistent; /* bitmap is persistent, but inconsistent.
WARNING: Block comments use a trailing */ on a separate line
#28: FILE: block/dirty-bitmap.c:51:
+ * a QMP user can remove it. */
total: 0 errors, 2 warnings, 82 lines checked
Patch 11/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/17 Checking commit e0e53b95d21c (block/dirty-bitmap: add inconsistent status)
13/17 Checking commit 46e4307e401e (block/dirty-bitmaps: add block_dirty_bitmap_check function)
ERROR: open brace '{' following function declarations go on the next line
#37: FILE: block/dirty-bitmap.c:177:
+static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) {
WARNING: line over 80 characters
#284: FILE: migration/block-dirty-bitmap.c:305:
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
total: 1 errors, 1 warnings, 236 lines checked
Patch 13/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/17 Checking commit 4085064d763d (block/dirty-bitmaps: prohibit readonly bitmaps for backups)
15/17 Checking commit ca8fdb5e18fe (block/dirty-bitmaps: prohibit removing readonly bitmaps)
16/17 Checking commit f00e672dd7a8 (block/dirty-bitmaps: disallow busy bitmaps as merge source)
17/17 Checking commit 6a083e7a449c (block/dirty-bitmaps: implement inconsistent bit)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190308202858.26636-1-jsnow@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PULL 00/17] Bitmaps patches
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (17 preceding siblings ...)
2019-03-08 20:46 ` [Qemu-devel] [PULL 00/17] Bitmaps patches no-reply
@ 2019-03-08 20:50 ` no-reply
2019-03-08 20:53 ` no-reply
2019-03-08 21:22 ` Eric Blake
20 siblings, 0 replies; 23+ messages in thread
From: no-reply @ 2019-03-08 20:50 UTC (permalink / raw)
To: jsnow; +Cc: fam, qemu-devel, peter.maydell
Patchew URL: https://patchew.org/QEMU/20190308202858.26636-1-jsnow@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20190308202858.26636-1-jsnow@redhat.com
Subject: [Qemu-devel] [PULL 00/17] Bitmaps patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/20190308202858.26636-1-jsnow@redhat.com -> patchew/20190308202858.26636-1-jsnow@redhat.com
Switched to a new branch 'test'
3d5e719ca4 block/dirty-bitmaps: implement inconsistent bit
465831cd59 block/dirty-bitmaps: disallow busy bitmaps as merge source
31780545dd block/dirty-bitmaps: prohibit removing readonly bitmaps
7559e374a2 block/dirty-bitmaps: prohibit readonly bitmaps for backups
aa7d65f2c7 block/dirty-bitmaps: add block_dirty_bitmap_check function
c680f34e1a block/dirty-bitmap: add inconsistent status
144ff09b4a block/dirty-bitmaps: add inconsistent bit
3404fb4d3c iotests: add busy/recording bit test to 124
c1abe84ba0 blockdev: remove unused paio parameter documentation
a9cea08dae block/dirty-bitmaps: move comment block
0c7cb0afda block/dirty-bitmaps: unify qmp_locked and user_locked calls
0978398f68 block/dirty-bitmap: explicitly lock bitmaps with successors
359bac67ac nbd: change error checking order for bitmaps
03ca272dcf block/dirty-bitmap: change semantics of enabled predicate
39788c7010 block/dirty-bitmap: remove set/reset assertions against enabled bit
953dca788e block/dirty-bitmaps: rename frozen predicate helper
ad226cd02f block/dirty-bitmap: add recording and busy properties
=== OUTPUT BEGIN ===
1/17 Checking commit ad226cd02f0d (block/dirty-bitmap: add recording and busy properties)
2/17 Checking commit 953dca788e01 (block/dirty-bitmaps: rename frozen predicate helper)
WARNING: line over 80 characters
#87: FILE: block/dirty-bitmap.c:248:
+ error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
total: 0 errors, 1 warnings, 124 lines checked
Patch 2/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/17 Checking commit 39788c7010d2 (block/dirty-bitmap: remove set/reset assertions against enabled bit)
4/17 Checking commit 03ca272dcfe2 (block/dirty-bitmap: change semantics of enabled predicate)
5/17 Checking commit 359bac67ac40 (nbd: change error checking order for bitmaps)
6/17 Checking commit 0978398f6879 (block/dirty-bitmap: explicitly lock bitmaps with successors)
7/17 Checking commit 0c7cb0afdaff (block/dirty-bitmaps: unify qmp_locked and user_locked calls)
ERROR: open brace '{' following function declarations go on the next line
#39: FILE: block/dirty-bitmap.c:190:
+bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {
total: 1 errors, 0 warnings, 271 lines checked
Patch 7/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/17 Checking commit a9cea08daeed (block/dirty-bitmaps: move comment block)
9/17 Checking commit c1abe84ba004 (blockdev: remove unused paio parameter documentation)
10/17 Checking commit 3404fb4d3ca7 (iotests: add busy/recording bit test to 124)
11/17 Checking commit 144ff09b4a7c (block/dirty-bitmaps: add inconsistent bit)
WARNING: Block comments use a leading /* on a separate line
#26: FILE: block/dirty-bitmap.c:49:
+ bool inconsistent; /* bitmap is persistent, but inconsistent.
WARNING: Block comments use a trailing */ on a separate line
#28: FILE: block/dirty-bitmap.c:51:
+ * a QMP user can remove it. */
total: 0 errors, 2 warnings, 82 lines checked
Patch 11/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/17 Checking commit c680f34e1af5 (block/dirty-bitmap: add inconsistent status)
13/17 Checking commit aa7d65f2c71f (block/dirty-bitmaps: add block_dirty_bitmap_check function)
ERROR: open brace '{' following function declarations go on the next line
#37: FILE: block/dirty-bitmap.c:177:
+static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) {
WARNING: line over 80 characters
#284: FILE: migration/block-dirty-bitmap.c:305:
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
total: 1 errors, 1 warnings, 236 lines checked
Patch 13/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/17 Checking commit 7559e374a22f (block/dirty-bitmaps: prohibit readonly bitmaps for backups)
15/17 Checking commit 31780545dd08 (block/dirty-bitmaps: prohibit removing readonly bitmaps)
16/17 Checking commit 465831cd595f (block/dirty-bitmaps: disallow busy bitmaps as merge source)
17/17 Checking commit 3d5e719ca44a (block/dirty-bitmaps: implement inconsistent bit)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190308202858.26636-1-jsnow@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PULL 00/17] Bitmaps patches
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (18 preceding siblings ...)
2019-03-08 20:50 ` no-reply
@ 2019-03-08 20:53 ` no-reply
2019-03-08 21:05 ` John Snow
2019-03-08 21:22 ` Eric Blake
20 siblings, 1 reply; 23+ messages in thread
From: no-reply @ 2019-03-08 20:53 UTC (permalink / raw)
To: jsnow; +Cc: fam, qemu-devel, peter.maydell
Patchew URL: https://patchew.org/QEMU/20190308202858.26636-1-jsnow@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20190308202858.26636-1-jsnow@redhat.com
Subject: [Qemu-devel] [PULL 00/17] Bitmaps patches
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
t [tag update] patchew/20190308202858.26636-1-jsnow@redhat.com -> patchew/20190308202858.26636-1-jsnow@redhat.com
Switched to a new branch 'test'
d67d39e43b block/dirty-bitmaps: implement inconsistent bit
cc50ffdaaa block/dirty-bitmaps: disallow busy bitmaps as merge source
fe93ecd8b1 block/dirty-bitmaps: prohibit removing readonly bitmaps
1d82e65c16 block/dirty-bitmaps: prohibit readonly bitmaps for backups
61c55a9de7 block/dirty-bitmaps: add block_dirty_bitmap_check function
4f1ceecd95 block/dirty-bitmap: add inconsistent status
9d5d76cbd6 block/dirty-bitmaps: add inconsistent bit
bdc998d9d5 iotests: add busy/recording bit test to 124
22c03a2fd5 blockdev: remove unused paio parameter documentation
d74e8a7202 block/dirty-bitmaps: move comment block
8ea8b7a875 block/dirty-bitmaps: unify qmp_locked and user_locked calls
ae989ad6ff block/dirty-bitmap: explicitly lock bitmaps with successors
6c71258b44 nbd: change error checking order for bitmaps
cf861d0168 block/dirty-bitmap: change semantics of enabled predicate
5ba70dfbcf block/dirty-bitmap: remove set/reset assertions against enabled bit
83886ed7db block/dirty-bitmaps: rename frozen predicate helper
0e38e3ff4f block/dirty-bitmap: add recording and busy properties
=== OUTPUT BEGIN ===
1/17 Checking commit 0e38e3ff4ff4 (block/dirty-bitmap: add recording and busy properties)
2/17 Checking commit 83886ed7db44 (block/dirty-bitmaps: rename frozen predicate helper)
WARNING: line over 80 characters
#87: FILE: block/dirty-bitmap.c:248:
+ error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
total: 0 errors, 1 warnings, 124 lines checked
Patch 2/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/17 Checking commit 5ba70dfbcf19 (block/dirty-bitmap: remove set/reset assertions against enabled bit)
4/17 Checking commit cf861d01687f (block/dirty-bitmap: change semantics of enabled predicate)
5/17 Checking commit 6c71258b445a (nbd: change error checking order for bitmaps)
6/17 Checking commit ae989ad6ffc7 (block/dirty-bitmap: explicitly lock bitmaps with successors)
7/17 Checking commit 8ea8b7a8757e (block/dirty-bitmaps: unify qmp_locked and user_locked calls)
ERROR: open brace '{' following function declarations go on the next line
#39: FILE: block/dirty-bitmap.c:190:
+bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {
total: 1 errors, 0 warnings, 271 lines checked
Patch 7/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
8/17 Checking commit d74e8a720254 (block/dirty-bitmaps: move comment block)
9/17 Checking commit 22c03a2fd589 (blockdev: remove unused paio parameter documentation)
10/17 Checking commit bdc998d9d5ed (iotests: add busy/recording bit test to 124)
11/17 Checking commit 9d5d76cbd6ca (block/dirty-bitmaps: add inconsistent bit)
WARNING: Block comments use a leading /* on a separate line
#26: FILE: block/dirty-bitmap.c:49:
+ bool inconsistent; /* bitmap is persistent, but inconsistent.
WARNING: Block comments use a trailing */ on a separate line
#28: FILE: block/dirty-bitmap.c:51:
+ * a QMP user can remove it. */
total: 0 errors, 2 warnings, 82 lines checked
Patch 11/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
12/17 Checking commit 4f1ceecd9528 (block/dirty-bitmap: add inconsistent status)
13/17 Checking commit 61c55a9de7e4 (block/dirty-bitmaps: add block_dirty_bitmap_check function)
ERROR: open brace '{' following function declarations go on the next line
#37: FILE: block/dirty-bitmap.c:177:
+static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) {
WARNING: line over 80 characters
#284: FILE: migration/block-dirty-bitmap.c:305:
+ if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
total: 1 errors, 1 warnings, 236 lines checked
Patch 13/17 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
14/17 Checking commit 1d82e65c16d6 (block/dirty-bitmaps: prohibit readonly bitmaps for backups)
15/17 Checking commit fe93ecd8b1cd (block/dirty-bitmaps: prohibit removing readonly bitmaps)
16/17 Checking commit cc50ffdaaa38 (block/dirty-bitmaps: disallow busy bitmaps as merge source)
17/17 Checking commit d67d39e43b84 (block/dirty-bitmaps: implement inconsistent bit)
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190308202858.26636-1-jsnow@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PULL 00/17] Bitmaps patches
2019-03-08 20:53 ` no-reply
@ 2019-03-08 21:05 ` John Snow
0 siblings, 0 replies; 23+ messages in thread
From: John Snow @ 2019-03-08 21:05 UTC (permalink / raw)
To: qemu-devel; +Cc: fam, peter.maydell
Guess I'll fix these first, sorry for the noise.
On 3/8/19 3:53 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20190308202858.26636-1-jsnow@redhat.com/
>
>
>
> Hi,
>
> This series seems to have some coding style problems. See output below for
> more information:
>
> Type: series
> Message-id: 20190308202858.26636-1-jsnow@redhat.com
> Subject: [Qemu-devel] [PULL 00/17] Bitmaps patches
>
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
>
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
> t [tag update] patchew/20190308202858.26636-1-jsnow@redhat.com -> patchew/20190308202858.26636-1-jsnow@redhat.com
> Switched to a new branch 'test'
> d67d39e43b block/dirty-bitmaps: implement inconsistent bit
> cc50ffdaaa block/dirty-bitmaps: disallow busy bitmaps as merge source
> fe93ecd8b1 block/dirty-bitmaps: prohibit removing readonly bitmaps
> 1d82e65c16 block/dirty-bitmaps: prohibit readonly bitmaps for backups
> 61c55a9de7 block/dirty-bitmaps: add block_dirty_bitmap_check function
> 4f1ceecd95 block/dirty-bitmap: add inconsistent status
> 9d5d76cbd6 block/dirty-bitmaps: add inconsistent bit
> bdc998d9d5 iotests: add busy/recording bit test to 124
> 22c03a2fd5 blockdev: remove unused paio parameter documentation
> d74e8a7202 block/dirty-bitmaps: move comment block
> 8ea8b7a875 block/dirty-bitmaps: unify qmp_locked and user_locked calls
> ae989ad6ff block/dirty-bitmap: explicitly lock bitmaps with successors
> 6c71258b44 nbd: change error checking order for bitmaps
> cf861d0168 block/dirty-bitmap: change semantics of enabled predicate
> 5ba70dfbcf block/dirty-bitmap: remove set/reset assertions against enabled bit
> 83886ed7db block/dirty-bitmaps: rename frozen predicate helper
> 0e38e3ff4f block/dirty-bitmap: add recording and busy properties
>
> === OUTPUT BEGIN ===
> 1/17 Checking commit 0e38e3ff4ff4 (block/dirty-bitmap: add recording and busy properties)
> 2/17 Checking commit 83886ed7db44 (block/dirty-bitmaps: rename frozen predicate helper)
> WARNING: line over 80 characters
> #87: FILE: block/dirty-bitmap.c:248:
> + error_setg(errp, "Cannot create a successor for a bitmap that is in-use "
>
> total: 0 errors, 1 warnings, 124 lines checked
>
> Patch 2/17 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 3/17 Checking commit 5ba70dfbcf19 (block/dirty-bitmap: remove set/reset assertions against enabled bit)
> 4/17 Checking commit cf861d01687f (block/dirty-bitmap: change semantics of enabled predicate)
> 5/17 Checking commit 6c71258b445a (nbd: change error checking order for bitmaps)
> 6/17 Checking commit ae989ad6ffc7 (block/dirty-bitmap: explicitly lock bitmaps with successors)
> 7/17 Checking commit 8ea8b7a8757e (block/dirty-bitmaps: unify qmp_locked and user_locked calls)
> ERROR: open brace '{' following function declarations go on the next line
> #39: FILE: block/dirty-bitmap.c:190:
> +bool bdrv_dirty_bitmap_busy(BdrvDirtyBitmap *bitmap) {
>
> total: 1 errors, 0 warnings, 271 lines checked
>
> Patch 7/17 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 8/17 Checking commit d74e8a720254 (block/dirty-bitmaps: move comment block)
> 9/17 Checking commit 22c03a2fd589 (blockdev: remove unused paio parameter documentation)
> 10/17 Checking commit bdc998d9d5ed (iotests: add busy/recording bit test to 124)
> 11/17 Checking commit 9d5d76cbd6ca (block/dirty-bitmaps: add inconsistent bit)
> WARNING: Block comments use a leading /* on a separate line
> #26: FILE: block/dirty-bitmap.c:49:
> + bool inconsistent; /* bitmap is persistent, but inconsistent.
>
> WARNING: Block comments use a trailing */ on a separate line
> #28: FILE: block/dirty-bitmap.c:51:
> + * a QMP user can remove it. */
>
> total: 0 errors, 2 warnings, 82 lines checked
>
> Patch 11/17 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 12/17 Checking commit 4f1ceecd9528 (block/dirty-bitmap: add inconsistent status)
> 13/17 Checking commit 61c55a9de7e4 (block/dirty-bitmaps: add block_dirty_bitmap_check function)
> ERROR: open brace '{' following function declarations go on the next line
> #37: FILE: block/dirty-bitmap.c:177:
> +static bool bdrv_dirty_bitmap_busy(const BdrvDirtyBitmap *bitmap) {
>
> WARNING: line over 80 characters
> #284: FILE: migration/block-dirty-bitmap.c:305:
> + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, &local_err)) {
>
> total: 1 errors, 1 warnings, 236 lines checked
>
> Patch 13/17 has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> 14/17 Checking commit 1d82e65c16d6 (block/dirty-bitmaps: prohibit readonly bitmaps for backups)
> 15/17 Checking commit fe93ecd8b1cd (block/dirty-bitmaps: prohibit removing readonly bitmaps)
> 16/17 Checking commit cc50ffdaaa38 (block/dirty-bitmaps: disallow busy bitmaps as merge source)
> 17/17 Checking commit d67d39e43b84 (block/dirty-bitmaps: implement inconsistent bit)
> === OUTPUT END ===
>
> Test command exited with code: 1
>
>
> The full log is available at
> http://patchew.org/logs/20190308202858.26636-1-jsnow@redhat.com/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [http://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PULL 00/17] Bitmaps patches
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
` (19 preceding siblings ...)
2019-03-08 20:53 ` no-reply
@ 2019-03-08 21:22 ` Eric Blake
20 siblings, 0 replies; 23+ messages in thread
From: Eric Blake @ 2019-03-08 21:22 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: peter.maydell
[-- Attachment #1: Type: text/plain, Size: 872 bytes --]
On 3/8/19 2:28 PM, John Snow wrote:
> The following changes since commit 234afe78281b10a798fb97c584e1b677844aaab8:
>
> Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-03-08' into staging (2019-03-08 16:31:34 +0000)
>
> are available in the Git repository at:
>
> https://github.com/jnsnow/qemu.git tags/bitmaps-pull-request
>
> for you to fetch changes up to fc91aa3d937e56d512c702d9b966fcea0e499f0e:
>
> block/dirty-bitmaps: implement inconsistent bit (2019-03-08 14:07:29 -0500)
>
> ----------------------------------------------------------------
> Pull request
Replying to the cover letter in case Peter doesn't see the nested reply:
NACK, v2 coming up for minor checkpatch fixes
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2019-03-08 21:22 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 20:28 [Qemu-devel] [PULL 00/17] Bitmaps patches John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 01/17] block/dirty-bitmap: add recording and busy properties John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 02/17] block/dirty-bitmaps: rename frozen predicate helper John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 03/17] block/dirty-bitmap: remove set/reset assertions against enabled bit John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 04/17] block/dirty-bitmap: change semantics of enabled predicate John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 05/17] nbd: change error checking order for bitmaps John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 06/17] block/dirty-bitmap: explicitly lock bitmaps with successors John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 07/17] block/dirty-bitmaps: unify qmp_locked and user_locked calls John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 08/17] block/dirty-bitmaps: move comment block John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 09/17] blockdev: remove unused paio parameter documentation John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 10/17] iotests: add busy/recording bit test to 124 John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 11/17] block/dirty-bitmaps: add inconsistent bit John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 12/17] block/dirty-bitmap: add inconsistent status John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 13/17] block/dirty-bitmaps: add block_dirty_bitmap_check function John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 14/17] block/dirty-bitmaps: prohibit readonly bitmaps for backups John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 15/17] block/dirty-bitmaps: prohibit removing readonly bitmaps John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 16/17] block/dirty-bitmaps: disallow busy bitmaps as merge source John Snow
2019-03-08 20:28 ` [Qemu-devel] [PULL 17/17] block/dirty-bitmaps: implement inconsistent bit John Snow
2019-03-08 20:46 ` [Qemu-devel] [PULL 00/17] Bitmaps patches no-reply
2019-03-08 20:50 ` no-reply
2019-03-08 20:53 ` no-reply
2019-03-08 21:05 ` John Snow
2019-03-08 21:22 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.