enable/disable copy-on-read via qmp
cover enable/disable copy-on-read via qmp
In some cases there is a need to perform a special access pattern to a disk backend. This may be done by providing disk reading access to external readers. In this case there should be the ability to enable/disable the copy-on-read mode for the disk during VM runtime.
By the moment, there is no such ability. The patch set adds qmp command to enable/disable the copy-on-read mode.
Denis Plotnikov (2):
block: check for read-only on copy-on-read setting
qmp: add block-set-copy-on-read command
block.c | 4 +--- block/io.c | 10 ++++++++-- block/stream.c | 5 +++-- blockdev.c | 38 ++++++++++++++++++++++++++++++++++++++ include/block/block.h | 2 +- qapi/block-core.json | 20 ++++++++++++++++++++ 6 files changed, 71 insertions(+), 8 deletions(-)
Signature · 1 line
2.17.0
PATCH 1/2 block: check for read-only on copy-on-read setting
We should always check whether block device is in the read-only state before enabling copy-on-read. The patch embeds the check in the copy-on-read setter.
--- block.c | 4 +--- block/io.c | 10 ++++++++-- block/stream.c | 5 +++-- include/block/block.h | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-)
block.c +1 -3
@@ -1370,9 +1370,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file,assert (atomic_read (&bs->copy_on_read) ==0 );if (bs->open_flags &BDRV_O_COPY_ON_READ ) { -if (!bs->read_only) { -bdrv_enable_copy_on_read (bs); - }else { +if (!bdrv_enable_copy_on_read (bs)) {error_setg (errp,"Can't use copy-on-read on read-only device" ); ret = -EINVAL ; goto fail_opts;
block/io.c +8 -2
@@ -131,9 +131,15 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error **errp)* use the feature without worrying about clobbering its previous state. * Copy-on-read stays enabled until all users have called to disable it. */ -void bdrv_enable_copy_on_read (BlockDriverState *bs) +bool bdrv_enable_copy_on_read (BlockDriverState *bs) { -atomic_inc (&bs->copy_on_read); +if (bs->read_only) { +return false; + }else { +atomic_inc (&bs->copy_on_read); + } + +return true; }void bdrv_disable_copy_on_read (BlockDriverState *bs)
block/stream.c +3 -2
@@ -130,8 +130,9 @@ static void coroutine_fn stream_run(void *opaque)* backing chain since the copy-on-read operation does not take base into * account. */ -if (!base) { -bdrv_enable_copy_on_read (bs); +if (!base && !bdrv_enable_copy_on_read (bs)) { + ret = -EPERM ; + goto out; }for ( ; offset < len; offset += n) {
include/block/block.h +1 -1
@@ -490,7 +490,7 @@ void *qemu_try_blockalign(BlockDriverState *bs, size_t size);void *qemu_try_blockalign0 (BlockDriverState *bs,size_t size);bool bdrv_qiov_is_aligned (BlockDriverState *bs,QEMUIOVector *qiov); -void bdrv_enable_copy_on_read (BlockDriverState *bs); +bool bdrv_enable_copy_on_read (BlockDriverState *bs);void bdrv_disable_copy_on_read (BlockDriverState *bs);void bdrv_ref (BlockDriverState *bs);
Signature · 1 line
2.17.0
PATCH 2/2 qmp: add block-set-copy-on-read command
In reply to Denis PlotnikovThe command enables/disables copy-on-read mode for VM's disk while VM is running.
This is needed when using external disk readers to shape access pattern to the disk backend.
--- blockdev.c | 38 ++++++++++++++++++++++++++++++++++++++ qapi/block-core.json | 20 ++++++++++++++++++++ 2 files changed, 58 insertions(+)
blockdev.c +38 -0
@@ -4276,6 +4276,44 @@ out:aio_context_release (aio_context); } +void qmp_block_set_copy_on_read (const char *device,bool enable,Error **errp) +{ +Error *local_err =NULL ; +AioContext *aio_context; +BlockDriverState *bs =bdrv_lookup_bs (device, device, &local_err); +if (local_err) { +error_propagate (errp, local_err); +return ; + } + + aio_context =bdrv_get_aio_context (bs); +aio_context_acquire (aio_context); + +if (enable) { +if (bs->open_flags &BDRV_O_COPY_ON_READ ) { +error_setg (errp,"Can't enable copy-on-read for the device. " +"Copy-on-read is already enabled." ); + }else { +if (bdrv_enable_copy_on_read (bs)) { + bs->open_flags |=BDRV_O_COPY_ON_READ ; + }else { +error_setg (errp,"Can't enable copy-on-read. " +"The device is read-only." ); + } + } + }else { +if (bs->open_flags &BDRV_O_COPY_ON_READ ) { + bs->open_flags &= ~BDRV_O_COPY_ON_READ ; +bdrv_disable_copy_on_read (bs); + }else { +error_setg (errp,"Can't disable copy-on-read for the device. " +"Copy-on-read is already disabled." ); + } + } + +aio_context_release (aio_context); +} +static BdrvChild *bdrv_find_child (BlockDriverState *parent_bs,const char *child_name) {
qapi/block-core.json +20 -0
@@ -4701,6 +4701,26 @@ { 'command': 'block-set-write-threshold', 'data': { 'node-name': 'str', 'write-threshold': 'uint64' } } +## +# @block-set-copy-on-read: +# +# Enables and disables the copy-on-read property of a block device. +# +# @device: device or graph node name on which copy-on-read must be set. +# +# Since: 2.12 +# +# Example: +# +# -> { "execute": "block-set-copy-on-read", +# "arguments": { "device": "scsi0-0-0-0", +# "enable": true } } +# <- { "return": {} } +# +## +{ 'command': 'block-set-copy-on-read', + 'data': { 'device': 'str', 'enable': 'bool' } } + ## # @x-blockdev-change: #
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v0 2/2] qmp: add block-set-copy-on-read command
Quoted 8 lines · Denis Plotnikov
The command enables/disables copy-on-read mode for VM's disk while VM is running.
This is needed when using external disk readers to shape access pattern to the disk backend.
Signed-off-by Denis Plotnikov <[email protected]>---
Deferring thoughts on the actual design for later; this is just a cursory implementation review for now.
qapi/block-core.json line 4711
Quoted 13 lines
+++
b/qapi/block-core.jsonqapi/block-core.json +8
-0@@ -4701,6 +4701,26 @@ { 'command': 'block-set-write-threshold', 'data': { 'node-name': 'str', 'write-threshold': 'uint64' } } +## +# @block-set-copy-on-read: +# +# Enables and disables the copy-on-read property of a block device. +# +# @device: device or graph node name on which copy-on-read must be set. +# +# Since: 2.12
We've missed the 2.12 release. This should be since 3.0.
qapi/block-core.json line 4722
Quoted 11 lines
qapi/block-core.json +11
-0+# +# Example: +# +# -> { "execute": "block-set-copy-on-read", +# "arguments": { "device": "scsi0-0-0-0", +# "enable": true } } +# <- { "return": {} } +# +## +{ 'command': 'block-set-copy-on-read', + 'data': { 'device': 'str', 'enable': 'bool' } }
Missing documentation of @enable.
And just checking: is the current copy-on-read setting visible through an existing query-* command, or have you just introduced a write-only toggle? If the state can be changed at runtime, then it needs to be easy to learn what the current state is.
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v0 2/2] qmp: add block-set-copy-on-read command
Quoted 11 lines · Eric Blake
Quoted 8 lines · Denis Plotnikov
The command enables/disables copy-on-read mode for VM's disk while VM is running.
This is needed when using external disk readers to shape access pattern to the disk backend.
Signed-off-by Denis Plotnikov <[email protected]>---
Deferring thoughts on the actual design for later;
But why? ;-)
This patch would definitely be superseded by a block reconfiguration command (i.e. presumably one that makes reopen accessible over QMP). With such a command, you could insert or remove a copy-on-read filter node at any point in time.
Since we definitely want block graph configuration, I don't think we want to add special commands now.
Max
Re: [Qemu-devel] [PATCH v0 2/2] qmp: add block-set-copy-on-read command
Am 13.06.2018 um 18:41 hat Max Reitz geschrieben:
Quoted 15 lines · Eric Blake
Quoted 11 lines · Denis Plotnikov
Quoted 8 lines
The command enables/disables copy-on-read mode for VM's disk while VM is running.
This is needed when using external disk readers to shape access pattern to the disk backend.
Signed-off-by Denis Plotnikov <[email protected]>---
Deferring thoughts on the actual design for later;
But why? ;-)
Quoted 7 lines
This patch would definitely be superseded by a block reconfiguration command (i.e. presumably one that makes reopen accessible over QMP). With such a command, you could insert or remove a copy-on-read filter node at any point in time.
Since we definitely want block graph configuration, I don't think we want to add special commands now.
I agree, and it seems that we get more and more use cases for a block reconfiguration commands. Only yesterday we talked about the "true" blockdev way for libvirt to implement I/O throttling. The result was that without reopen, we still need to use the old QMP command to set BlockBackend-based I/O throttling instead of using a filter node.
So I'm eagerly awaiting Berto's promised patches for it.
Kevin
Re: [Qemu-devel] [PATCH v0 2/2] qmp: add block-set-copy-on-read command
Quoted 24 lines · Kevin Wolf
Am 13.06.2018 um 18:41 hat Max Reitz geschrieben:
Quoted 15 lines · Eric Blake
Quoted 11 lines · Denis Plotnikov
Quoted 8 lines
The command enables/disables copy-on-read mode for VM's disk while VM is running.
This is needed when using external disk readers to shape access pattern to the disk backend.
Signed-off-by Denis Plotnikov <[email protected]>---
Deferring thoughts on the actual design for later;
But why? ;-)
Quoted 7 lines
This patch would definitely be superseded by a block reconfiguration command (i.e. presumably one that makes reopen accessible over QMP). With such a command, you could insert or remove a copy-on-read filter node at any point in time.
Since we definitely want block graph configuration, I don't think we want to add special commands now.
Quoted 7 lines
I agree, and it seems that we get more and more use cases for a block reconfiguration commands. Only yesterday we talked about the "true" blockdev way for libvirt to implement I/O throttling. The result was that without reopen, we still need to use the old QMP command to set BlockBackend-based I/O throttling instead of using a filter node.
So I'm eagerly awaiting Berto's promised patches for it.
They're coming, it's just a bit tricker than I initially though. I'll probably send the first series as an RFC if I don't manage to fix all problems, so we can discuss them a bit.
Berto
Re: [Qemu-devel] [PATCH v0 2/2] qmp: add block-set-copy-on-read command
Am 14.06.2018 um 11:03 hat Alberto Garcia geschrieben:
Quoted 31 lines · Kevin Wolf
Quoted 30 lines
Am 13.06.2018 um 18:41 hat Max Reitz geschrieben:
Quoted 15 lines · Eric Blake
Quoted 11 lines · Denis Plotnikov
Quoted 8 lines
The command enables/disables copy-on-read mode for VM's disk while VM is running.
This is needed when using external disk readers to shape access pattern to the disk backend.
Signed-off-by Denis Plotnikov <[email protected]>---
Deferring thoughts on the actual design for later;
But why? ;-)
Quoted 7 lines
This patch would definitely be superseded by a block reconfiguration command (i.e. presumably one that makes reopen accessible over QMP). With such a command, you could insert or remove a copy-on-read filter node at any point in time.
Since we definitely want block graph configuration, I don't think we want to add special commands now.
I agree, and it seems that we get more and more use cases for a block reconfiguration commands. Only yesterday we talked about the "true" blockdev way for libvirt to implement I/O throttling. The result was that without reopen, we still need to use the old QMP command to set BlockBackend-based I/O throttling instead of using a filter node.
Quoted 5 lines
Quoted 1 line
So I'm eagerly awaiting Berto's promised patches for it.
They're coming, it's just a bit tricker than I initially though. I'll probably send the first series as an RFC if I don't manage to fix all problems, so we can discuss them a bit.
Sounds good to me. It's not completely surprising that there are some tricky spots there, we've been postponing this for a reason...
Kevin