Re: [PATCH v3 4/5] vfio-user: simplify vfio_user_recv_one()
On 01/12/2025 09:56, John Levon wrote:
This function was unnecessarily difficult to understand due to the
separate handling of request and reply messages. Use common code for
both where we can.
Signed-off-by: John Levon
---
hw/vfio-user/proxy.c | 68 +++-
1 file changed, 30 insertions(+), 38 deletions(-)
diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 87e50501af..d1d63816b3 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -281,15 +281,14 @@ static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error
**errp,
*/
static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp)
{
-VFIOUserMsg *msg = NULL;
g_autofree int *fdp = NULL;
-VFIOUserFDs *reqfds;
-VFIOUserHdr hdr;
+VFIOUserMsg *msg = NULL;
bool isreply = false;
-int i, ret;
-size_t msgleft, numfds = 0;
+size_t msgleft = 0;
+size_t numfds = 0;
char *data = NULL;
-char *buf = NULL;
+VFIOUserHdr hdr;
+int i, ret;
/*
* Complete any partial reads
@@ -317,8 +316,8 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
}
/*
- * For replies, find the matching pending request.
- * For requests, reap incoming FDs.
+ * Find the matching request if this is a reply, or initialize a new
+ * server->client request.
*/
if (isreply) {
QTAILQ_FOREACH(msg, &proxy->pending, next) {
@@ -332,51 +331,44 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
}
QTAILQ_REMOVE(&proxy->pending, msg, next);
-/*
- * Process any received FDs
- */
-if (numfds != 0) {
-if (msg->fds == NULL || msg->fds->recv_fds < numfds) {
-error_setg(errp, "unexpected FDs");
-goto err;
-}
-msg->fds->recv_fds = numfds;
-memcpy(msg->fds->fds, fdp, numfds * sizeof(int));
-}
-} else {
-if (numfds != 0) {
-reqfds = vfio_user_getfds(numfds);
-memcpy(reqfds->fds, fdp, numfds * sizeof(int));
-} else {
-reqfds = NULL;
-}
-}
-
-/*
- * Put the whole message into a single buffer.
- */
-if (isreply) {
if (hdr.size > msg->rsize) {
error_setg(errp, "reply larger than recv buffer");
goto err;
}
-*msg->hdr = hdr;
-data = (char *)msg->hdr + sizeof(hdr);
} else {
+void *buf;
+
if (hdr.size > proxy->max_xfer_size + sizeof(VFIOUserDMARW)) {
error_setg(errp, "vfio_user_recv request larger than max");
goto err;
}
+
buf = g_malloc0(hdr.size);
-memcpy(buf, &hdr, sizeof(hdr));
-data = buf + sizeof(hdr);
-msg = vfio_user_getmsg(proxy, (VFIOUserHdr *)buf, reqfds);
+msg = vfio_user_getmsg(proxy, buf, NULL);
msg->type = VFIO_MSG_REQ;
}
+*msg->hdr = hdr;
+data = (char *)msg->hdr + sizeof(hdr);
+
+if (numfds != 0) {
+if (msg->type == VFIO_MSG_REQ) {
+msg->fds = vfio_user_getfds(numfds);
+} else {
+if (msg->fds == NULL || msg->fds->recv_fds < numfds) {
+error_setg(errp, "unexpected FDs in reply");
+goto err;
+}
+msg->fds->recv_fds = numfds;
+}
+
+memcpy(msg->fds->fds, fdp, numfds * sizeof(int));
+}
+
/*
- * Read rest of message.
+ * Read rest of message into the data buffer.
*/
+
msgleft = hdr.size - sizeof(hdr);
while (msgleft > 0) {
ret = qio_channel_read(proxy->ioc, data, msgleft, errp);
Reviewed-by: Mark Cave-Ayland
ATB,
Mark.
[PATCH v3 4/5] vfio-user: simplify vfio_user_recv_one()
This function was unnecessarily difficult to understand due to the
separate handling of request and reply messages. Use common code for
both where we can.
Signed-off-by: John Levon
---
hw/vfio-user/proxy.c | 68 +++-
1 file changed, 30 insertions(+), 38 deletions(-)
diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 87e50501af..d1d63816b3 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -281,15 +281,14 @@ static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error
**errp,
*/
static int vfio_user_recv_one(VFIOUserProxy *proxy, Error **errp)
{
-VFIOUserMsg *msg = NULL;
g_autofree int *fdp = NULL;
-VFIOUserFDs *reqfds;
-VFIOUserHdr hdr;
+VFIOUserMsg *msg = NULL;
bool isreply = false;
-int i, ret;
-size_t msgleft, numfds = 0;
+size_t msgleft = 0;
+size_t numfds = 0;
char *data = NULL;
-char *buf = NULL;
+VFIOUserHdr hdr;
+int i, ret;
/*
* Complete any partial reads
@@ -317,8 +316,8 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
}
/*
- * For replies, find the matching pending request.
- * For requests, reap incoming FDs.
+ * Find the matching request if this is a reply, or initialize a new
+ * server->client request.
*/
if (isreply) {
QTAILQ_FOREACH(msg, &proxy->pending, next) {
@@ -332,51 +331,44 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
}
QTAILQ_REMOVE(&proxy->pending, msg, next);
-/*
- * Process any received FDs
- */
-if (numfds != 0) {
-if (msg->fds == NULL || msg->fds->recv_fds < numfds) {
-error_setg(errp, "unexpected FDs");
-goto err;
-}
-msg->fds->recv_fds = numfds;
-memcpy(msg->fds->fds, fdp, numfds * sizeof(int));
-}
-} else {
-if (numfds != 0) {
-reqfds = vfio_user_getfds(numfds);
-memcpy(reqfds->fds, fdp, numfds * sizeof(int));
-} else {
-reqfds = NULL;
-}
-}
-
-/*
- * Put the whole message into a single buffer.
- */
-if (isreply) {
if (hdr.size > msg->rsize) {
error_setg(errp, "reply larger than recv buffer");
goto err;
}
-*msg->hdr = hdr;
-data = (char *)msg->hdr + sizeof(hdr);
} else {
+void *buf;
+
if (hdr.size > proxy->max_xfer_size + sizeof(VFIOUserDMARW)) {
error_setg(errp, "vfio_user_recv request larger than max");
goto err;
}
+
buf = g_malloc0(hdr.size);
-memcpy(buf, &hdr, sizeof(hdr));
-data = buf + sizeof(hdr);
-msg = vfio_user_getmsg(proxy, (VFIOUserHdr *)buf, reqfds);
+msg = vfio_user_getmsg(proxy, buf, NULL);
msg->type = VFIO_MSG_REQ;
}
+*msg->hdr = hdr;
+data = (char *)msg->hdr + sizeof(hdr);
+
+if (numfds != 0) {
+if (msg->type == VFIO_MSG_REQ) {
+msg->fds = vfio_user_getfds(numfds);
+} else {
+if (msg->fds == NULL || msg->fds->recv_fds < numfds) {
+error_setg(errp, "unexpected FDs in reply");
+goto err;
+}
+msg->fds->recv_fds = numfds;
+}
+
+memcpy(msg->fds->fds, fdp, numfds * sizeof(int));
+}
+
/*
- * Read rest of message.
+ * Read rest of message into the data buffer.
*/
+
msgleft = hdr.size - sizeof(hdr);
while (msgleft > 0) {
ret = qio_channel_read(proxy->ioc, data, msgleft, errp);
--
2.43.0
