Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
03.02.2018 19:09, Markus Armbruster wrote: Vladimir Sementsov-Ogievskiy writes: Signed-off-by: Vladimir Sementsov-Ogievskiy --- qapi/block-core.json | 42 ++ blockdev.c | 42 ++ 2 files changed, 84 insertions(+) diff --git a/qapi/block-core.json b/qapi/block-core.json index 827254db22..b3851ee760 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1672,6 +1672,48 @@ 'data': 'BlockDirtyBitmap' } ## +# @block-dirty-bitmap-enable: +# +# Enable dirty bitmap, so that it will continue tracking disk changes. +# +# Returns: nothing on success +# If @node is not a valid block device, DeviceNotFound +# If @name is not found, GenericError with an explanation What do you mean by "with an explanation"? If it's the error objects @desc member: that's trivial, any error carries it. The less error classes other than GenericError are used, the happier I am... Do users really need to distinguish between these two errors? I've just used same semantics as it is used for block-dirty-bitmap-clear.. block-dirty-bitmap-remove and block-dirty-bitmap-add have the same semantics too. +# +# Since: 2.12 +# +# Example: +# +# -> { "execute": "block-dirty-bitmap-enable", +# "arguments": { "node": "drive0", "name": "bitmap0" } } +# <- { "return": {} } +# +## + { 'command': 'block-dirty-bitmap-enable', +'data': 'BlockDirtyBitmap' } + +## +# @block-dirty-bitmap-disable: +# +# Disable dirty bitmap, so that it will stop tracking disk changes. +# +# Returns: nothing on success +# If @node is not a valid block device, DeviceNotFound +# If @name is not found, GenericError with an explanation Likewise. +# +# Since: 2.12 +# +# Example: +# +# -> { "execute": "block-dirty-bitmap-disable", +# "arguments": { "node": "drive0", "name": "bitmap0" } } +# <- { "return": {} } +# +## +{ 'command': 'block-dirty-bitmap-disable', + 'data': 'BlockDirtyBitmap' } + +## # @BlockDirtyBitmapSha256: # # SHA256 hash of dirty bitmap data [...] -- Best regards, Vladimir
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
Vladimir Sementsov-Ogievskiy writes: > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > qapi/block-core.json | 42 ++ > blockdev.c | 42 ++ > 2 files changed, 84 insertions(+) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 827254db22..b3851ee760 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1672,6 +1672,48 @@ >'data': 'BlockDirtyBitmap' } > > ## > +# @block-dirty-bitmap-enable: > +# > +# Enable dirty bitmap, so that it will continue tracking disk changes. > +# > +# Returns: nothing on success > +# If @node is not a valid block device, DeviceNotFound > +# If @name is not found, GenericError with an explanation What do you mean by "with an explanation"? If it's the error objects @desc member: that's trivial, any error carries it. The less error classes other than GenericError are used, the happier I am... Do users really need to distinguish between these two errors? > +# > +# Since: 2.12 > +# > +# Example: > +# > +# -> { "execute": "block-dirty-bitmap-enable", > +# "arguments": { "node": "drive0", "name": "bitmap0" } } > +# <- { "return": {} } > +# > +## > + { 'command': 'block-dirty-bitmap-enable', > +'data': 'BlockDirtyBitmap' } > + > +## > +# @block-dirty-bitmap-disable: > +# > +# Disable dirty bitmap, so that it will stop tracking disk changes. > +# > +# Returns: nothing on success > +# If @node is not a valid block device, DeviceNotFound > +# If @name is not found, GenericError with an explanation Likewise. > +# > +# Since: 2.12 > +# > +# Example: > +# > +# -> { "execute": "block-dirty-bitmap-disable", > +# "arguments": { "node": "drive0", "name": "bitmap0" } } > +# <- { "return": {} } > +# > +## > +{ 'command': 'block-dirty-bitmap-disable', > + 'data': 'BlockDirtyBitmap' } > + > +## > # @BlockDirtyBitmapSha256: > # > # SHA256 hash of dirty bitmap data [...]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
22.01.2018 22:56, John Snow wrote: On 01/22/2018 02:51 PM, Eric Blake wrote: On 01/22/2018 03:09 AM, Vladimir Sementsov-Ogievskiy wrote: I have to admit exposing this interface still makes me nervous, but :) Mechanically correct, and with suggesting phrasing changes: Reviewed-by: John Snow Should we go with an x- prefix for now, and let things settle for a release before promoting it to fully supported, just in case we find something that justifies your nervousness in accepting the interface? I proposed as much in a reply to the cover letter; I'm willing to take patch one now for the sake of migration; the rest of this series I want a test suite that helps document the anticipated use case, or otherwise an x- prefix for the command names. Give me a try with test, and if it will still be unclear, then let's add x- prefix for first steps. (of course, my aim is to push the api without x- prefixes =) -- Best regards, Vladimir
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
On 01/22/2018 02:51 PM, Eric Blake wrote: > On 01/22/2018 03:09 AM, Vladimir Sementsov-Ogievskiy wrote: > >>> I have to admit exposing this interface still makes me nervous, but :) >>> >>> Mechanically correct, and with suggesting phrasing changes: >>> >>> Reviewed-by: John Snow > > Should we go with an x- prefix for now, and let things settle for a > release before promoting it to fully supported, just in case we find > something that justifies your nervousness in accepting the interface? > I proposed as much in a reply to the cover letter; I'm willing to take patch one now for the sake of migration; the rest of this series I want a test suite that helps document the anticipated use case, or otherwise an x- prefix for the command names.
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
On 01/22/2018 03:09 AM, Vladimir Sementsov-Ogievskiy wrote: >>> >> I have to admit exposing this interface still makes me nervous, but :) >> >> Mechanically correct, and with suggesting phrasing changes: >> >> Reviewed-by: John Snow Should we go with an x- prefix for now, and let things settle for a release before promoting it to fully supported, just in case we find something that justifies your nervousness in accepting the interface? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
20.01.2018 02:50, John Snow wrote: On 01/16/2018 07:54 AM, Vladimir Sementsov-Ogievskiy wrote: Signed-off-by: Vladimir Sementsov-Ogievskiy --- qapi/block-core.json | 42 ++ blockdev.c | 42 ++ 2 files changed, 84 insertions(+) diff --git a/qapi/block-core.json b/qapi/block-core.json index 827254db22..b3851ee760 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1672,6 +1672,48 @@ 'data': 'BlockDirtyBitmap' } ## +# @block-dirty-bitmap-enable: +# +# Enable dirty bitmap, so that it will continue tracking disk changes. +# suggest: "Enables a dirty bitmap so that it will begin tracking disk changes." Key item here is "begin" instead of "continue". agree. +# Returns: nothing on success +# If @node is not a valid block device, DeviceNotFound +# If @name is not found, GenericError with an explanation +# +# Since: 2.12 +# +# Example: +# +# -> { "execute": "block-dirty-bitmap-enable", +# "arguments": { "node": "drive0", "name": "bitmap0" } } +# <- { "return": {} } +# +## + { 'command': 'block-dirty-bitmap-enable', +'data': 'BlockDirtyBitmap' } + +## +# @block-dirty-bitmap-disable: +# +# Disable dirty bitmap, so that it will stop tracking disk changes. +# suggest: "Disables a dirty bitmap so that it will stop tracking disk changes." +# Returns: nothing on success +# If @node is not a valid block device, DeviceNotFound +# If @name is not found, GenericError with an explanation +# +# Since: 2.12 +# +# Example: +# +# -> { "execute": "block-dirty-bitmap-disable", +# "arguments": { "node": "drive0", "name": "bitmap0" } } +# <- { "return": {} } +# +## +{ 'command': 'block-dirty-bitmap-disable', + 'data': 'BlockDirtyBitmap' } + +## # @BlockDirtyBitmapSha256: # # SHA256 hash of dirty bitmap data diff --git a/blockdev.c b/blockdev.c index 8068cbd606..997a6f514c 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2821,6 +2821,48 @@ void qmp_block_dirty_bitmap_clear(const char *node, const char *name, bdrv_clear_dirty_bitmap(bitmap, NULL); } +void qmp_block_dirty_bitmap_enable(const char *node, const char *name, + Error **errp) +{ +BlockDriverState *bs; +BdrvDirtyBitmap *bitmap; + +bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); +if (!bitmap || !bs) { +return; +} + +if (bdrv_dirty_bitmap_frozen(bitmap)) { +error_setg(errp, + "Bitmap '%s' is currently frozen and cannot be enabled", + name); +return; +} + +bdrv_enable_dirty_bitmap(bitmap); +} + +void qmp_block_dirty_bitmap_disable(const char *node, const char *name, +Error **errp) +{ +BlockDriverState *bs; +BdrvDirtyBitmap *bitmap; + +bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); +if (!bitmap || !bs) { +return; +} + +if (bdrv_dirty_bitmap_frozen(bitmap)) { +error_setg(errp, + "Bitmap '%s' is currently frozen and cannot be disabled", + name); +return; +} + +bdrv_disable_dirty_bitmap(bitmap); +} + BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char *node, const char *name, Error **errp) I have to admit exposing this interface still makes me nervous, but :) Mechanically correct, and with suggesting phrasing changes: Reviewed-by: John Snow -- Best regards, Vladimir
Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/6] qapi: add block-dirty-bitmap-enable/disable
On 01/16/2018 07:54 AM, Vladimir Sementsov-Ogievskiy wrote: > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > qapi/block-core.json | 42 ++ > blockdev.c | 42 ++ > 2 files changed, 84 insertions(+) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 827254db22..b3851ee760 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -1672,6 +1672,48 @@ >'data': 'BlockDirtyBitmap' } > > ## > +# @block-dirty-bitmap-enable: > +# > +# Enable dirty bitmap, so that it will continue tracking disk changes. > +# suggest: "Enables a dirty bitmap so that it will begin tracking disk changes." Key item here is "begin" instead of "continue". > +# Returns: nothing on success > +# If @node is not a valid block device, DeviceNotFound > +# If @name is not found, GenericError with an explanation > +# > +# Since: 2.12 > +# > +# Example: > +# > +# -> { "execute": "block-dirty-bitmap-enable", > +# "arguments": { "node": "drive0", "name": "bitmap0" } } > +# <- { "return": {} } > +# > +## > + { 'command': 'block-dirty-bitmap-enable', > +'data': 'BlockDirtyBitmap' } > + > +## > +# @block-dirty-bitmap-disable: > +# > +# Disable dirty bitmap, so that it will stop tracking disk changes. > +# suggest: "Disables a dirty bitmap so that it will stop tracking disk changes." > +# Returns: nothing on success > +# If @node is not a valid block device, DeviceNotFound > +# If @name is not found, GenericError with an explanation > +# > +# Since: 2.12 > +# > +# Example: > +# > +# -> { "execute": "block-dirty-bitmap-disable", > +# "arguments": { "node": "drive0", "name": "bitmap0" } } > +# <- { "return": {} } > +# > +## > +{ 'command': 'block-dirty-bitmap-disable', > + 'data': 'BlockDirtyBitmap' } > + > +## > # @BlockDirtyBitmapSha256: > # > # SHA256 hash of dirty bitmap data > diff --git a/blockdev.c b/blockdev.c > index 8068cbd606..997a6f514c 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2821,6 +2821,48 @@ void qmp_block_dirty_bitmap_clear(const char *node, > const char *name, > bdrv_clear_dirty_bitmap(bitmap, NULL); > } > > +void qmp_block_dirty_bitmap_enable(const char *node, const char *name, > + Error **errp) > +{ > +BlockDriverState *bs; > +BdrvDirtyBitmap *bitmap; > + > +bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); > +if (!bitmap || !bs) { > +return; > +} > + > +if (bdrv_dirty_bitmap_frozen(bitmap)) { > +error_setg(errp, > + "Bitmap '%s' is currently frozen and cannot be enabled", > + name); > +return; > +} > + > +bdrv_enable_dirty_bitmap(bitmap); > +} > + > +void qmp_block_dirty_bitmap_disable(const char *node, const char *name, > +Error **errp) > +{ > +BlockDriverState *bs; > +BdrvDirtyBitmap *bitmap; > + > +bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); > +if (!bitmap || !bs) { > +return; > +} > + > +if (bdrv_dirty_bitmap_frozen(bitmap)) { > +error_setg(errp, > + "Bitmap '%s' is currently frozen and cannot be disabled", > + name); > +return; > +} > + > +bdrv_disable_dirty_bitmap(bitmap); > +} > + > BlockDirtyBitmapSha256 *qmp_x_debug_block_dirty_bitmap_sha256(const char > *node, >const char > *name, >Error **errp) > I have to admit exposing this interface still makes me nervous, but :) Mechanically correct, and with suggesting phrasing changes: Reviewed-by: John Snow