The 'x' prefix was added because I was uncertain of the direction we'd
take for the libvirt API. With the general approach solidified, I feel
comfortable committing to this API for 4.0.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
blockdev.c | 22 +++++++++++-----------
qapi/block-core.json | 34 +++++++++++++++++-----------------
qapi/transaction.json | 12 ++++++------
tests/qemu-iotests/223 | 4 ++--
4 files changed, 36 insertions(+), 36 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 6031c94121..8e37dd659e 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1963,7 +1963,7 @@ static void block_dirty_bitmap_add_prepare(BlkActionState *common,
action->has_granularity, action->granularity,
action->has_persistent, action->persistent,
action->has_autoload, action->autoload,
- action->has_x_disabled, action->x_disabled,
+ action->has_disabled, action->disabled,
&local_err);
if (!local_err) {
@@ -2048,7 +2048,7 @@ static void block_dirty_bitmap_enable_prepare(BlkActionState *common,
return;
}
- action = common->action->u.x_block_dirty_bitmap_enable.data;
+ action = common->action->u.block_dirty_bitmap_enable.data;
state->bitmap = block_dirty_bitmap_lookup(action->node,
action->name,
NULL,
@@ -2089,7 +2089,7 @@ static void block_dirty_bitmap_disable_prepare(BlkActionState *common,
return;
}
- action = common->action->u.x_block_dirty_bitmap_disable.data;
+ action = common->action->u.block_dirty_bitmap_disable.data;
state->bitmap = block_dirty_bitmap_lookup(action->node,
action->name,
NULL,
@@ -2136,7 +2136,7 @@ static void block_dirty_bitmap_merge_prepare(BlkActionState *common,
return;
}
- action = common->action->u.x_block_dirty_bitmap_merge.data;
+ action = common->action->u.block_dirty_bitmap_merge.data;
state->bitmap = do_block_dirty_bitmap_merge(action->node, action->target,
action->bitmaps, &state->backup,
@@ -2204,17 +2204,17 @@ static const BlkActionOps actions[] = {
.commit = block_dirty_bitmap_free_backup,
.abort = block_dirty_bitmap_restore,
},
- [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_ENABLE] = {
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_ENABLE] = {
.instance_size = sizeof(BlockDirtyBitmapState),
.prepare = block_dirty_bitmap_enable_prepare,
.abort = block_dirty_bitmap_enable_abort,
},
- [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_DISABLE] = {
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_DISABLE] = {
.instance_size = sizeof(BlockDirtyBitmapState),
.prepare = block_dirty_bitmap_disable_prepare,
.abort = block_dirty_bitmap_disable_abort,
},
- [TRANSACTION_ACTION_KIND_X_BLOCK_DIRTY_BITMAP_MERGE] = {
+ [TRANSACTION_ACTION_KIND_BLOCK_DIRTY_BITMAP_MERGE] = {
.instance_size = sizeof(BlockDirtyBitmapState),
.prepare = block_dirty_bitmap_merge_prepare,
.commit = block_dirty_bitmap_free_backup,
@@ -2930,7 +2930,7 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name,
bdrv_clear_dirty_bitmap(bitmap, NULL);
}
-void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_enable(const char *node, const char *name,
Error **errp)
{
BlockDriverState *bs;
@@ -2951,7 +2951,7 @@ void qmp_x_block_dirty_bitmap_enable(const char *node, const char *name,
bdrv_enable_dirty_bitmap(bitmap);
}
-void qmp_x_block_dirty_bitmap_disable(const char *node, const char *name,
+void qmp_block_dirty_bitmap_disable(const char *node, const char *name,
Error **errp)
{
BlockDriverState *bs;
@@ -3018,8 +3018,8 @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_merge(const char *node,
return dst;
}
-void qmp_x_block_dirty_bitmap_merge(const char *node, const char *target,
- strList *bitmaps, Error **errp)
+void qmp_block_dirty_bitmap_merge(const char *node, const char *target,
+ strList *bitmaps, Error **errp)
{
do_block_dirty_bitmap_merge(node, target, bitmaps, NULL, errp);
}
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a153ea4420..91685be6c2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1806,15 +1806,15 @@
# Currently, all dirty tracking bitmaps are loaded from Qcow2 on
# open.
#
-# @x-disabled: the bitmap is created in the disabled state, which means that
-# it will not track drive changes. The bitmap may be enabled with
-# x-block-dirty-bitmap-enable. Default is false. (Since: 3.0)
+# @disabled: the bitmap is created in the disabled state, which means that
+# it will not track drive changes. The bitmap may be enabled with
+# block-dirty-bitmap-enable. Default is false. (Since: 4.0)
#
# Since: 2.4
##
{ 'struct': 'BlockDirtyBitmapAdd',
'data': { 'node': 'str', 'name': 'str', '*granularity': 'uint32',
- '*persistent': 'bool', '*autoload': 'bool', '*x-disabled': 'bool' } }
+ '*persistent': 'bool', '*autoload': 'bool', '*disabled': 'bool' } }
##
# @BlockDirtyBitmapMerge:
@@ -1825,7 +1825,7 @@
#
# @bitmaps: name(s) of the source dirty bitmap(s)
#
-# Since: 3.0
+# Since: 4.0
##
{ 'struct': 'BlockDirtyBitmapMerge',
'data': { 'node': 'str', 'target': 'str', 'bitmaps': ['str'] } }
@@ -1899,7 +1899,7 @@
'data': 'BlockDirtyBitmap' }
##
-# @x-block-dirty-bitmap-enable:
+# @block-dirty-bitmap-enable:
#
# Enables a dirty bitmap so that it will begin tracking disk changes.
#
@@ -1907,20 +1907,20 @@
# If @node is not a valid block device, DeviceNotFound
# If @name is not found, GenericError with an explanation
#
-# Since: 3.0
+# Since: 4.0
#
# Example:
#
-# -> { "execute": "x-block-dirty-bitmap-enable",
+# -> { "execute": "block-dirty-bitmap-enable",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
# <- { "return": {} }
#
##
- { 'command': 'x-block-dirty-bitmap-enable',
+ { 'command': 'block-dirty-bitmap-enable',
'data': 'BlockDirtyBitmap' }
##
-# @x-block-dirty-bitmap-disable:
+# @block-dirty-bitmap-disable:
#
# Disables a dirty bitmap so that it will stop tracking disk changes.
#
@@ -1928,20 +1928,20 @@
# If @node is not a valid block device, DeviceNotFound
# If @name is not found, GenericError with an explanation
#
-# Since: 3.0
+# Since: 4.0
#
# Example:
#
-# -> { "execute": "x-block-dirty-bitmap-disable",
+# -> { "execute": "block-dirty-bitmap-disable",
# "arguments": { "node": "drive0", "name": "bitmap0" } }
# <- { "return": {} }
#
##
- { 'command': 'x-block-dirty-bitmap-disable',
+ { 'command': 'block-dirty-bitmap-disable',
'data': 'BlockDirtyBitmap' }
##
-# @x-block-dirty-bitmap-merge:
+# @block-dirty-bitmap-merge:
#
# Merge dirty bitmaps listed in @bitmaps to the @target dirty bitmap.
# The @bitmaps dirty bitmaps are unchanged.
@@ -1953,17 +1953,17 @@
# If any of the bitmaps have different sizes or granularities,
# GenericError
#
-# Since: 3.0
+# Since: 4.0
#
# Example:
#
-# -> { "execute": "x-block-dirty-bitmap-merge",
+# -> { "execute": "block-dirty-bitmap-merge",
# "arguments": { "node": "drive0", "target": "bitmap0",
# "bitmaps": ["bitmap1"] } }
# <- { "return": {} }
#
##
- { 'command': 'x-block-dirty-bitmap-merge',
+ { 'command': 'block-dirty-bitmap-merge',
'data': 'BlockDirtyBitmapMerge' }
##
diff --git a/qapi/transaction.json b/qapi/transaction.json
index 5875cdb16c..95edb78227 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -46,9 +46,9 @@
# - @abort: since 1.6
# - @block-dirty-bitmap-add: since 2.5
# - @block-dirty-bitmap-clear: since 2.5
-# - @x-block-dirty-bitmap-enable: since 3.0
-# - @x-block-dirty-bitmap-disable: since 3.0
-# - @x-block-dirty-bitmap-merge: since 3.1
+# - @block-dirty-bitmap-enable: since 4.0
+# - @block-dirty-bitmap-disable: since 4.0
+# - @block-dirty-bitmap-merge: since 4.0
# - @blockdev-backup: since 2.3
# - @blockdev-snapshot: since 2.5
# - @blockdev-snapshot-internal-sync: since 1.7
@@ -62,9 +62,9 @@
'abort': 'Abort',
'block-dirty-bitmap-add': 'BlockDirtyBitmapAdd',
'block-dirty-bitmap-clear': 'BlockDirtyBitmap',
- 'x-block-dirty-bitmap-enable': 'BlockDirtyBitmap',
- 'x-block-dirty-bitmap-disable': 'BlockDirtyBitmap',
- 'x-block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
+ 'block-dirty-bitmap-enable': 'BlockDirtyBitmap',
+ 'block-dirty-bitmap-disable': 'BlockDirtyBitmap',
+ 'block-dirty-bitmap-merge': 'BlockDirtyBitmapMerge',
'blockdev-backup': 'BlockdevBackup',
'blockdev-snapshot': 'BlockdevSnapshot',
'blockdev-snapshot-internal-sync': 'BlockdevSnapshotInternal',
diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223
index 397b865d34..5513dc6215 100755
--- a/tests/qemu-iotests/223
+++ b/tests/qemu-iotests/223
@@ -112,9 +112,9 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"qmp_capabilities"}' "return"
_send_qemu_cmd $QEMU_HANDLE '{"execute":"blockdev-add",
"arguments":{"driver":"qcow2", "node-name":"n",
"file":{"driver":"file", "filename":"'"$TEST_IMG"'"}}}' "return"
-_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"block-dirty-bitmap-disable",
"arguments":{"node":"n", "name":"b"}}' "return"
-_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-block-dirty-bitmap-disable",
+_send_qemu_cmd $QEMU_HANDLE '{"execute":"block-dirty-bitmap-disable",
"arguments":{"node":"n", "name":"b2"}}' "return"
_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start",
"arguments":{"addr":{"type":"unix",
--
2.17.2
On 12/21/18 3:35 AM, John Snow wrote: > The 'x' prefix was added because I was uncertain of the direction we'd > take for the libvirt API. With the general approach solidified, I feel > comfortable committing to this API for 4.0. > > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > blockdev.c | 22 +++++++++++----------- > qapi/block-core.json | 34 +++++++++++++++++----------------- > qapi/transaction.json | 12 ++++++------ > tests/qemu-iotests/223 | 4 ++-- > 4 files changed, 36 insertions(+), 36 deletions(-) Misses the conversion of x-nbd-server-add-bitmap in qapi/block.json (could be a separate patch), but that's also one of the commands that libvirt will need to use alongside all the bitmap changes. Speaking of x-nbd-server-add-bitmap, does it actually have to be a separate command, or should it just be an additional parameter to the existing nbd-server-add? There are two directions this could go: As a single command, it enforces that there is no window of time where the export is available without the dirty bitmap. However, it also means we can only export at most one bitmap, unless we make the parameter take an array of bitmaps to export. Fewer commands also means less roll-back scenarios in libvirt (no need to worry about the export being added but the bitmap failing). As a separate command, we could in the future allow the server to export multiple bitmaps at once by calling the add-bitmap command more than once; but right now, the code base does not allow multiple bitmap exports (a second bitmap export attempt fails). I'm leaning towards the former (drop x-nbd-server-add-bitmap altogether, and instead add a bitmap parameter to nbd-server-add). Also, I'm leaning towards locking in that we only ever export one qemu:dirty-bitmap:FOO bitmap per export, and thus not worry about an array. Besides, if we make the command take an array now but enforce that the array has at most one element, then later relax it to take more than one element, there is no way to introspect that change; but if we make the command take a single string now, and later have a strong reason why exporting more than one bitmap at once makes sense, we could still maintain backcompat by using a QAPI alternate type between a string and an array, which would be introspectible. I guess I should propose a patch for that, then... -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
04.01.2019 2:21, Eric Blake wrote: > On 12/21/18 3:35 AM, John Snow wrote: >> The 'x' prefix was added because I was uncertain of the direction we'd >> take for the libvirt API. With the general approach solidified, I feel >> comfortable committing to this API for 4.0. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> blockdev.c | 22 +++++++++++----------- >> qapi/block-core.json | 34 +++++++++++++++++----------------- >> qapi/transaction.json | 12 ++++++------ >> tests/qemu-iotests/223 | 4 ++-- >> 4 files changed, 36 insertions(+), 36 deletions(-) > > Misses the conversion of x-nbd-server-add-bitmap in qapi/block.json > (could be a separate patch), but that's also one of the commands that > libvirt will need to use alongside all the bitmap changes. > > Speaking of x-nbd-server-add-bitmap, does it actually have to be a > separate command, or should it just be an additional parameter to the > existing nbd-server-add? There are two directions this could go: > > As a single command, it enforces that there is no window of time where > the export is available without the dirty bitmap. However, it also means > we can only export at most one bitmap, unless we make the parameter take > an array of bitmaps to export. Fewer commands also means less roll-back > scenarios in libvirt (no need to worry about the export being added but > the bitmap failing). > > As a separate command, we could in the future allow the server to export > multiple bitmaps at once by calling the add-bitmap command more than > once; but right now, the code base does not allow multiple bitmap > exports (a second bitmap export attempt fails). > > I'm leaning towards the former (drop x-nbd-server-add-bitmap altogether, > and instead add a bitmap parameter to nbd-server-add). Also, I'm > leaning towards locking in that we only ever export one > qemu:dirty-bitmap:FOO bitmap per export, and thus not worry about an > array. Besides, if we make the command take an array now but enforce > that the array has at most one element, then later relax it to take more > than one element, there is no way to introspect that change; but if we > make the command take a single string now, and later have a strong > reason why exporting more than one bitmap at once makes sense, we could > still maintain backcompat by using a QAPI alternate type between a > string and an array, which would be introspectible. > > I guess I should propose a patch for that, then... > Reasonable, I've no objections. -- Best regards, Vladimir
© 2016 - 2026 Red Hat, Inc.