[Libguestfs] [libnbd PATCH v3 1/7] lib: Refactor command_common() to do more common work
Eric Blake
eblake at redhat.com
Wed May 22 21:29:01 UTC 2019
Our construction of struct command_in_flight was ad hoc; most
parameters were set in command_common(), but extent callbacks were
done after the fact, and NBD_CMD_DISC was open-coding things.
Furthermore, every caller was triggering nbd_internal_run() for the
cmd_issue event; doing that in a central place makes it easier for the
next patch to improve that logic without duplicating the fix at each
caller. Fix these things by renaming the function to
nbd_internal_command_common, which is necessary for exporting it, and
by adding parameters and an updated return type.
---
lib/disconnect.c | 20 +++------
lib/internal.h | 7 ++++
lib/rw.c | 103 +++++++++++++----------------------------------
3 files changed, 39 insertions(+), 91 deletions(-)
diff --git a/lib/disconnect.c b/lib/disconnect.c
index bc43b4c..9706835 100644
--- a/lib/disconnect.c
+++ b/lib/disconnect.c
@@ -60,27 +60,17 @@ nbd_unlocked_shutdown (struct nbd_handle *h)
int
nbd_unlocked_aio_disconnect (struct nbd_connection *conn)
{
- struct command_in_flight *cmd;
+ int64_t id;
- cmd = malloc (sizeof *cmd);
- if (cmd == NULL) {
- set_error (errno, "malloc");
+ id = nbd_internal_command_common (conn, 0, NBD_CMD_DISC, 0, 0, NULL,
+ 0, NULL);
+ if (id == -1)
return -1;
- }
- cmd->flags = 0;
- cmd->type = NBD_CMD_DISC;
- cmd->handle = conn->h->unique++;
- cmd->offset = 0;
- cmd->count = 0;
- cmd->data = NULL;
-
- cmd->next = conn->cmds_to_issue;
- conn->cmds_to_issue = cmd;
/* This will leave the command on the in-flight list. Is this a
* problem? Probably it isn't. If it is, we could add a flag to
* the command struct to tell SEND_REQUEST not to add it to the
* in-flight list.
*/
- return nbd_internal_run (conn->h, conn, cmd_issue);
+ return 0;
}
diff --git a/lib/internal.h b/lib/internal.h
index 1f742da..de9b8bc 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -265,6 +265,13 @@ extern void nbd_internal_set_last_error (int errnum, char *error);
extern int nbd_internal_errno_of_nbd_error (uint32_t error);
extern const char *nbd_internal_name_of_nbd_cmd (uint16_t type);
+/* rw.c */
+extern int64_t nbd_internal_command_common (struct nbd_connection *conn,
+ uint16_t flags, uint16_t type,
+ uint64_t offset, uint64_t count,
+ void *data, int64_t id,
+ extent_fn extent);
+
/* socket.c */
struct socket *nbd_internal_socket_create (int fd);
diff --git a/lib/rw.c b/lib/rw.c
index 861ab67..8f6227d 100644
--- a/lib/rw.c
+++ b/lib/rw.c
@@ -241,10 +241,11 @@ nbd_unlocked_block_status (struct nbd_handle *h,
return r == -1 ? -1 : 0;
}
-static struct command_in_flight *
-command_common (struct nbd_connection *conn,
- uint16_t flags, uint16_t type,
- uint64_t offset, uint64_t count, void *data)
+int64_t
+nbd_internal_command_common (struct nbd_connection *conn,
+ uint16_t flags, uint16_t type,
+ uint64_t offset, uint64_t count, void *data,
+ int64_t id, extent_fn extent)
{
struct command_in_flight *cmd;
@@ -255,7 +256,7 @@ command_common (struct nbd_connection *conn,
if (count > MAX_REQUEST_SIZE) {
set_error (ERANGE, "request too large: maximum request size is %d",
MAX_REQUEST_SIZE);
- return NULL;
+ return -1;
}
break;
@@ -268,7 +269,7 @@ command_common (struct nbd_connection *conn,
if (count > UINT32_MAX) {
set_error (ERANGE, "request too large: maximum request size is %" PRIu32,
UINT32_MAX);
- return NULL;
+ return -1;
}
break;
}
@@ -276,7 +277,7 @@ command_common (struct nbd_connection *conn,
cmd = calloc (1, sizeof *cmd);
if (cmd == NULL) {
set_error (errno, "calloc");
- return NULL;
+ return -1;
}
cmd->flags = flags;
cmd->type = type;
@@ -284,6 +285,8 @@ command_common (struct nbd_connection *conn,
cmd->offset = offset;
cmd->count = count;
cmd->data = data;
+ cmd->extent_id = id;
+ cmd->extent_fn = extent;
/* If structured replies were negotiated then we trust the server to
* send back sufficient data to cover the whole buffer. It's tricky
@@ -298,23 +301,18 @@ command_common (struct nbd_connection *conn,
cmd->next = conn->cmds_to_issue;
conn->cmds_to_issue = cmd;
+ if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
+ return -1;
- return cmd;
+ return cmd->handle;
}
int64_t
nbd_unlocked_aio_pread (struct nbd_connection *conn, void *buf,
size_t count, uint64_t offset)
{
- struct command_in_flight *cmd;
-
- cmd = command_common (conn, 0, NBD_CMD_READ, offset, count, buf);
- if (!cmd)
- return -1;
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, 0, NBD_CMD_READ, offset, count,
+ buf, 0, NULL);
}
int64_t
@@ -322,8 +320,6 @@ nbd_unlocked_aio_pwrite (struct nbd_connection *conn, const void *buf,
size_t count, uint64_t offset,
uint32_t flags)
{
- struct command_in_flight *cmd;
-
if (nbd_unlocked_read_only (conn->h) == 1) {
set_error (EINVAL, "server does not support write operations");
return -1;
@@ -340,33 +336,20 @@ nbd_unlocked_aio_pwrite (struct nbd_connection *conn, const void *buf,
return -1;
}
- cmd = command_common (conn, flags, NBD_CMD_WRITE, offset, count,
- (void *) buf);
- if (!cmd)
- return -1;
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, flags, NBD_CMD_WRITE, offset, count,
+ (void *) buf, 0, NULL);
}
int64_t
nbd_unlocked_aio_flush (struct nbd_connection *conn)
{
- struct command_in_flight *cmd;
-
if (nbd_unlocked_can_flush (conn->h) != 1) {
set_error (EINVAL, "server does not support flush operations");
return -1;
}
- cmd = command_common (conn, 0, NBD_CMD_FLUSH, 0, 0, NULL);
- if (!cmd)
- return -1;
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, 0, NBD_CMD_FLUSH, 0, 0,
+ NULL, 0, NULL);
}
int64_t
@@ -374,8 +357,6 @@ nbd_unlocked_aio_trim (struct nbd_connection *conn,
uint64_t count, uint64_t offset,
uint32_t flags)
{
- struct command_in_flight *cmd;
-
if (nbd_unlocked_read_only (conn->h) == 1) {
set_error (EINVAL, "server does not support write operations");
return -1;
@@ -397,21 +378,14 @@ nbd_unlocked_aio_trim (struct nbd_connection *conn,
return -1;
}
- cmd = command_common (conn, flags, NBD_CMD_TRIM, offset, count, NULL);
- if (!cmd)
- return -1;
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, flags, NBD_CMD_TRIM, offset, count,
+ NULL, 0, NULL);
}
int64_t
nbd_unlocked_aio_cache (struct nbd_connection *conn,
uint64_t count, uint64_t offset)
{
- struct command_in_flight *cmd;
-
/* Actually according to the NBD protocol document, servers do exist
* that support NBD_CMD_CACHE but don't advertize the
* NBD_FLAG_SEND_CACHE bit, but we ignore those.
@@ -421,13 +395,8 @@ nbd_unlocked_aio_cache (struct nbd_connection *conn,
return -1;
}
- cmd = command_common (conn, 0, NBD_CMD_CACHE, offset, count, NULL);
- if (!cmd)
- return -1;
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, 0, NBD_CMD_CACHE, offset, count,
+ NULL, 0, NULL);
}
int64_t
@@ -435,8 +404,6 @@ nbd_unlocked_aio_zero (struct nbd_connection *conn,
uint64_t count, uint64_t offset,
uint32_t flags)
{
- struct command_in_flight *cmd;
-
if (nbd_unlocked_read_only (conn->h) == 1) {
set_error (EINVAL, "server does not support write operations");
return -1;
@@ -458,13 +425,8 @@ nbd_unlocked_aio_zero (struct nbd_connection *conn,
return -1;
}
- cmd = command_common (conn, flags, NBD_CMD_WRITE_ZEROES, offset, count, NULL);
- if (!cmd)
- return -1;
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, flags, NBD_CMD_WRITE_ZEROES, offset,
+ count, NULL, 0, NULL);
}
int64_t
@@ -474,8 +436,6 @@ nbd_unlocked_aio_block_status (struct nbd_connection *conn,
int64_t id,
extent_fn extent)
{
- struct command_in_flight *cmd;
-
if (!conn->structured_replies) {
set_error (ENOTSUP, "server does not support structured replies");
return -1;
@@ -498,15 +458,6 @@ nbd_unlocked_aio_block_status (struct nbd_connection *conn,
return -1;
}
- cmd = command_common (conn, flags, NBD_CMD_BLOCK_STATUS, offset, count, NULL);
- if (!cmd)
- return -1;
-
- cmd->extent_fn = extent;
- cmd->extent_id = id;
-
- if (nbd_internal_run (conn->h, conn, cmd_issue) == -1)
- return -1;
-
- return cmd->handle;
+ return nbd_internal_command_common (conn, flags, NBD_CMD_BLOCK_STATUS, offset,
+ count, NULL, id, extent);
}
--
2.20.1
More information about the Libguestfs
mailing list