Re: [PATCH 6/6] nbd/server: introduce NBDClient->lock to protect fields

[Paolo Bonzini]

On 12/21/23 02:49, Stefan Hajnoczi wrote:
      nbd_client_receive_next_request(client);
+
+    qemu_mutex_unlock(&client->lock);
+
      if (ret == -EIO) {
          goto disconnect;
      }

I think I slightly prefer if disconnect is reached with lock taken, for 
consistency with the "done" label.  It does not complicate the code,
because you can just move qio_channel_set_cork() and replace:

@@ -3024,8 +3072,10 @@ static coroutine_fn void nbd_trip(void *opaque)
      }
  
      qio_channel_set_cork(client->ioc, false);
+    qemu_mutex_lock(&client->lock);

with:

+    qio_channel_set_cork(client->ioc, false);
+    qemu_mutex_lock(&client->lock);

     if (ret < 0) {
         error_prepend(&local_err, "Failed to send reply: ");
         goto disconnect;
     }

     /*
      * We must disconnect after NBD_CMD_WRITE or BLOCK_STATUS with
      * payload if we did not read the payload.
      */
     if (!req->complete) {
         error_setg(&local_err, "Request handling failed in 
intermediate state");
         goto disconnect;
     }
-    qio_channel_set_cork(client->ioc, false);
 done:

Thanks,

Paolo

  done:
      nbd_request_put(req);
+    qemu_mutex_unlock(&client->lock);