Re: [Qemu-devel] [PATCH 02/11] block: make write-threshold thread-safe

[Stefan Hajnoczi]

On Thu, Jul 06, 2017 at 06:38:19PM +0200, Paolo Bonzini wrote:
> For simplicity, use bdrv_drained_begin/end to avoid concurrent
> writes to the write threshold, or reading it while it is being set.
> qmp_block_set_write_threshold is protected by the BQL.

I find the commit message misleading: the write threshold code is not
being made thread-safe.

The commit description helps to clear this up, but we need to avoid
thinking "write threshold is thread-safe" because that would lead to
races between bdrv_write_threshold_set() callers.

Perhaps:

  block: protect write threshold qmp cmd from concurrent requests

Anyway:

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 02/11] block: make write-threshold thread-safe

[Eric Blake]

On 07/06/2017 11:38 AM, Paolo Bonzini wrote:
> For simplicity, use bdrv_drained_begin/end to avoid concurrent
> writes to the write threshold, or reading it while it is being set.
> qmp_block_set_write_threshold is protected by the BQL.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/write-threshold.c | 9 +++--
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH 02/11] block: make write-threshold thread-safe

[Paolo Bonzini]

For simplicity, use bdrv_drained_begin/end to avoid concurrent
writes to the write threshold, or reading it while it is being set.
qmp_block_set_write_threshold is protected by the BQL.

Signed-off-by: Paolo Bonzini 
---
 block/write-threshold.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/block/write-threshold.c b/block/write-threshold.c
index c8ebc32b4d..64ddd3d653 100644
--- a/block/write-threshold.c
+++ b/block/write-threshold.c
@@ -112,7 +112,6 @@ void qmp_block_set_write_threshold(const char *node_name,
Error **errp)
 {
 BlockDriverState *bs;
-AioContext *aio_context;
 
 bs = bdrv_find_node(node_name);
 if (!bs) {
@@ -120,10 +119,8 @@ void qmp_block_set_write_threshold(const char *node_name,
 return;
 }
 
-aio_context = bdrv_get_aio_context(bs);
-aio_context_acquire(aio_context);
-
+/* Avoid a concurrent write_threshold_disable.  */
+bdrv_drained_begin(bs);
 bdrv_write_threshold_set(bs, threshold_bytes);
-
-aio_context_release(aio_context);
+bdrv_drained_end(bs);
 }
-- 
2.13.0