Re: [Qemu-devel] [PATCH 5/5] nbd/server: refactor nbd_trip: split out nbd_handle_request

2018-03-09 Thread Eric Blake

On 03/08/2018 12:46 PM, Vladimir Sementsov-Ogievskiy wrote:

Split out request handling logic.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  nbd/server.c | 129 +++
  1 file changed, 67 insertions(+), 62 deletions(-)




+
+switch (request->type) {
+case NBD_CMD_READ:
+return nbd_do_cmd_read(client, request, data, errp);
+
+case NBD_CMD_WRITE:
+flags = 0;
+if (request->flags & NBD_CMD_FLAG_FUA) {
+flags |= BDRV_REQ_FUA;
+}
+ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
+ data, request->len, flags);
+
+return nbd_send_generic_reply(client, request->handle, ret,
+  "writing to file failed", errp);
+case NBD_CMD_WRITE_ZEROES:


Inconsistent spacing between return and the next case label.

But switching whitespace is trivial, so

Reviewed-by: Eric Blake 

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



[Qemu-devel] [PATCH 5/5] nbd/server: refactor nbd_trip: split out nbd_handle_request

2018-03-08 Thread Vladimir Sementsov-Ogievskiy
Split out request handling logic.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 129 +++
 1 file changed, 67 insertions(+), 62 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index a2f5f73d52..7b236f404e 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1584,17 +1584,79 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient 
*client, NBDRequest *request,
 }
 }
 
+/* Handle NBD request.
+ * Return -errno if sending fails. Other errors are reported directly to the
+ * client as an error reply. */
+static coroutine_fn int nbd_handle_request(NBDClient *client,
+   NBDRequest *request,
+   uint8_t *data, Error **errp)
+{
+int ret;
+int flags;
+NBDExport *exp = client->exp;
+char *msg;
+
+switch (request->type) {
+case NBD_CMD_READ:
+return nbd_do_cmd_read(client, request, data, errp);
+
+case NBD_CMD_WRITE:
+flags = 0;
+if (request->flags & NBD_CMD_FLAG_FUA) {
+flags |= BDRV_REQ_FUA;
+}
+ret = blk_pwrite(exp->blk, request->from + exp->dev_offset,
+ data, request->len, flags);
+
+return nbd_send_generic_reply(client, request->handle, ret,
+  "writing to file failed", errp);
+case NBD_CMD_WRITE_ZEROES:
+flags = 0;
+if (request->flags & NBD_CMD_FLAG_FUA) {
+flags |= BDRV_REQ_FUA;
+}
+if (!(request->flags & NBD_CMD_FLAG_NO_HOLE)) {
+flags |= BDRV_REQ_MAY_UNMAP;
+}
+ret = blk_pwrite_zeroes(exp->blk, request->from + exp->dev_offset,
+request->len, flags);
+
+return nbd_send_generic_reply(client, request->handle, ret,
+  "writing to file failed", errp);
+case NBD_CMD_DISC:
+/* unreachable, thanks to special case in nbd_co_receive_request() */
+abort();
+
+case NBD_CMD_FLUSH:
+ret = blk_co_flush(exp->blk);
+
+return nbd_send_generic_reply(client, request->handle, ret,
+  "flush failed", errp);
+case NBD_CMD_TRIM:
+ret = blk_co_pdiscard(exp->blk, request->from + exp->dev_offset,
+  request->len);
+
+return nbd_send_generic_reply(client, request->handle, ret,
+  "discard failed", errp);
+default:
+msg = g_strdup_printf("invalid request type (%" PRIu32 ") received",
+  request->type);
+ret = nbd_send_generic_reply(client, request->handle, -EINVAL, msg,
+ errp);
+g_free(msg);
+
+return ret;
+}
+}
+
 /* Owns a reference to the NBDClient passed as opaque.  */
 static coroutine_fn void nbd_trip(void *opaque)
 {
 NBDClient *client = opaque;
-NBDExport *exp = client->exp;
 NBDRequestData *req;
 NBDRequest request = { 0 };/* GCC thinks it can be used uninitialized 
*/
 int ret;
-int flags;
 Error *local_err = NULL;
-char *msg = NULL;
 
 trace_nbd_trip();
 if (client->closing) {
@@ -1627,66 +1689,9 @@ static coroutine_fn void nbd_trip(void *opaque)
 ret = nbd_send_generic_reply(client, request.handle, -EINVAL,
  error_get_pretty(export_err), &local_err);
 error_free(export_err);
-
-goto replied;
-}
-
-switch (request.type) {
-case NBD_CMD_READ:
-ret = nbd_do_cmd_read(client, &request, req->data, &local_err);
-
-break;
-case NBD_CMD_WRITE:
-flags = 0;
-if (request.flags & NBD_CMD_FLAG_FUA) {
-flags |= BDRV_REQ_FUA;
-}
-ret = blk_pwrite(exp->blk, request.from + exp->dev_offset,
- req->data, request.len, flags);
-ret = nbd_send_generic_reply(client, request.handle, ret,
- "writing to file failed", &local_err);
-
-break;
-case NBD_CMD_WRITE_ZEROES:
-flags = 0;
-if (request.flags & NBD_CMD_FLAG_FUA) {
-flags |= BDRV_REQ_FUA;
-}
-if (!(request.flags & NBD_CMD_FLAG_NO_HOLE)) {
-flags |= BDRV_REQ_MAY_UNMAP;
-}
-ret = blk_pwrite_zeroes(exp->blk, request.from + exp->dev_offset,
-request.len, flags);
-ret = nbd_send_generic_reply(client, request.handle, ret,
- "writing to file failed", &local_err);
-
-break;
-case NBD_CMD_DISC:
-/* unreachable, thanks to special case in nbd_co_receive_request() */
-abort();
-
-case NBD_CMD_FLUSH:
-ret = blk_co_flush(exp->blk);
-ret = nbd_send_generic_reply(client, request.handle, ret,
- "flush failed", &local_er