On 4/16/22 16:00, Vladimir Sementsov-Ogievskiy wrote:
14.04.2022 20:57, Paolo Bonzini wrote:
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
block/nbd.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/block/nbd.c b/block/nbd.c
index 31c684772e..d0d94b40bd 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -81,12 +81,18 @@ typedef struct BDRVNBDState {
NBDClientRequest requests[MAX_NBD_REQUESTS];
QEMUTimer *reconnect_delay_timer;
+ /* Protects sending data on the socket. */
CoMutex send_mutex;
+
+ /*
+ * Protects receiving reply headers from the socket, as well as the
+ * fields reply and requests[].receiving
I think, worth noting, that s->reply is used without mutex after
nbd_receive_replies() success and till setting s->reply.handle to 0 in
nbd_co_receive_one_chunk()..
Should "s->reply.handle = 0" be done under mutex as well? And may be, in
same critical section as nbd_recv_coroutines_wake() ?
Could be an idea. It could also be a store-release but no reason to be
fancy:
diff --git a/block/nbd.c b/block/nbd.c
index 0bd9b674a9..cd760bfd50 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -149,11 +149,11 @@ static bool coroutine_fn
nbd_recv_coroutine_wake_one(NBDClientRequest *req)
return false;
}
+/* Called with s->receive_mutex taken. */
static void coroutine_fn nbd_recv_coroutines_wake(BDRVNBDState *s)
{
int i;
- QEMU_LOCK_GUARD(&s->receive_mutex);
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
if (nbd_recv_coroutine_wake_one(&s->requests[i])) {
return;
@@ -924,9 +924,11 @@ static coroutine_fn int nbd_co_receive_one_chunk(
/* For assert at loop start in nbd_connection_entry */
*reply = s->reply;
}
- s->reply.handle = 0;
- nbd_recv_coroutines_wake(s);
+ WITH_QEMU_LOCK_GUARD(&s->receive_mutex) {
+ s->reply.handle = 0;
+ nbd_recv_coroutines_wake(s);
+ }
return ret;
}
Paolo
+ */
CoMutex receive_mutex;
+ NBDReply reply;
QEMUTimer *open_timer;
- NBDReply reply;
BlockDriverState *bs;
/* Connection parameters */