Re: [PATCH v3 3/5] vfio-user: refactor out header handling
Hello John,
On 12/2/25 11:53, Mark Cave-Ayland wrote:
On 01/12/2025 09:56, John Levon wrote:
Simplify vfio_user_recv_one() by moving the header handling out to a
helper function.
Signed-off-by: John Levon
---
hw/vfio-user/proxy.c | 101 +--
1 file changed, 60 insertions(+), 41 deletions(-)
diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 82c76c6665..87e50501af 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -218,6 +218,61 @@ static int vfio_user_complete(VFIOUserProxy *proxy, Error
**errp)
return 1;
}
+static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error **errp,
+ VFIOUserHdr *hdr, int **fdp, size_t *numfdp,
+ bool *isreply)
+{
+ struct iovec iov = {
+ .iov_base = hdr,
+ .iov_len = sizeof(*hdr),
+ };
+ int ret;
+
+ /*
+ * Read header
+ */
+ ret = qio_channel_readv_full(proxy->ioc, &iov, 1, fdp, numfdp, 0,
+ errp);
+ if (ret == QIO_CHANNEL_ERR_BLOCK) {
+ return ret;
+ }
+
+ /* read error or other side closed connection */
+ if (ret <= 0) {
+ error_setg(errp, "failed to read header");
Can we use error_setg_errno() here to record the real underlying return code in
errp? Otherwise with this change it gets lost due to vfio_user_recv_hdr()
always returning -1 if there is an error.
Can you please send a v4 with this change requested by Mark ?
I will then prepare a last QEMU 10.2 PR with this series and
the vfio-user doc update I left behind.
Thanks,
C.
+ return -1;
+ }
+
+ if (ret < sizeof(*hdr)) {
+ error_setg(errp, "short read of header");
+ return -1;
+ }
+
+ /*
+ * Validate header
+ */
+ if (hdr->size < sizeof(*hdr)) {
+ error_setg(errp, "bad header size");
+ return -1;
+ }
+
+ switch (hdr->flags & VFIO_USER_TYPE) {
+ case VFIO_USER_REQUEST:
+ *isreply = false;
+ break;
+ case VFIO_USER_REPLY:
+ *isreply = true;
+ break;
+ default:
+ error_setg(errp, "unknown message type");
+ return -1;
+ }
+
+ trace_vfio_user_recv_hdr(proxy->sockname, hdr->id, hdr->command, hdr->size,
+ hdr->flags);
+ return 0;
+}
+
/*
* Receive and process one incoming message.
*
@@ -230,10 +285,6 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
g_autofree int *fdp = NULL;
VFIOUserFDs *reqfds;
VFIOUserHdr hdr;
- struct iovec iov = {
- .iov_base = &hdr,
- .iov_len = sizeof(hdr),
- };
bool isreply = false;
int i, ret;
size_t msgleft, numfds = 0;
@@ -257,45 +308,13 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
/* else fall into reading another msg */
}
- /*
- * Read header
- */
- ret = qio_channel_readv_full(proxy->ioc, &iov, 1, &fdp, &numfds, 0,
- errp);
- if (ret == QIO_CHANNEL_ERR_BLOCK) {
- return ret;
- }
-
- /* read error or other side closed connection */
- if (ret <= 0) {
- goto fatal;
- }
-
- if (ret < sizeof(hdr)) {
- error_setg(errp, "short read of header");
- goto fatal;
- }
-
- /*
- * Validate header
- */
- if (hdr.size < sizeof(VFIOUserHdr)) {
- error_setg(errp, "bad header size");
- goto fatal;
- }
- switch (hdr.flags & VFIO_USER_TYPE) {
- case VFIO_USER_REQUEST:
- isreply = false;
- break;
- case VFIO_USER_REPLY:
- isreply = true;
- break;
- default:
- error_setg(errp, "unknown message type");
+ ret = vfio_user_recv_hdr(proxy, errp, &hdr, &fdp, &numfds, &isreply);
+ if (ret < 0) {
+ if (ret == QIO_CHANNEL_ERR_BLOCK) {
+ return ret;
+ }
goto fatal;
}
- trace_vfio_user_recv_hdr(proxy->sockname, hdr.id, hdr.command, hdr.size,
- hdr.flags);
/*
* For replies, find the matching pending request.
ATB,
Mark.
Re: [PATCH v3 3/5] vfio-user: refactor out header handling
On 01/12/2025 09:56, John Levon wrote:
Simplify vfio_user_recv_one() by moving the header handling out to a
helper function.
Signed-off-by: John Levon
---
hw/vfio-user/proxy.c | 101 +--
1 file changed, 60 insertions(+), 41 deletions(-)
diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 82c76c6665..87e50501af 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -218,6 +218,61 @@ static int vfio_user_complete(VFIOUserProxy *proxy, Error
**errp)
return 1;
}
+static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error **errp,
+ VFIOUserHdr *hdr, int **fdp, size_t *numfdp,
+ bool *isreply)
+{
+struct iovec iov = {
+.iov_base = hdr,
+.iov_len = sizeof(*hdr),
+};
+int ret;
+
+/*
+ * Read header
+ */
+ret = qio_channel_readv_full(proxy->ioc, &iov, 1, fdp, numfdp, 0,
+ errp);
+if (ret == QIO_CHANNEL_ERR_BLOCK) {
+return ret;
+}
+
+/* read error or other side closed connection */
+if (ret <= 0) {
+error_setg(errp, "failed to read header");
Can we use error_setg_errno() here to record the real underlying return
code in errp? Otherwise with this change it gets lost due to
vfio_user_recv_hdr() always returning -1 if there is an error.
+return -1;
+}
+
+if (ret < sizeof(*hdr)) {
+error_setg(errp, "short read of header");
+return -1;
+}
+
+/*
+ * Validate header
+ */
+if (hdr->size < sizeof(*hdr)) {
+error_setg(errp, "bad header size");
+return -1;
+}
+
+switch (hdr->flags & VFIO_USER_TYPE) {
+case VFIO_USER_REQUEST:
+*isreply = false;
+break;
+case VFIO_USER_REPLY:
+*isreply = true;
+break;
+default:
+error_setg(errp, "unknown message type");
+return -1;
+}
+
+trace_vfio_user_recv_hdr(proxy->sockname, hdr->id, hdr->command, hdr->size,
+ hdr->flags);
+return 0;
+}
+
/*
* Receive and process one incoming message.
*
@@ -230,10 +285,6 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
g_autofree int *fdp = NULL;
VFIOUserFDs *reqfds;
VFIOUserHdr hdr;
-struct iovec iov = {
-.iov_base = &hdr,
-.iov_len = sizeof(hdr),
-};
bool isreply = false;
int i, ret;
size_t msgleft, numfds = 0;
@@ -257,45 +308,13 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
/* else fall into reading another msg */
}
-/*
- * Read header
- */
-ret = qio_channel_readv_full(proxy->ioc, &iov, 1, &fdp, &numfds, 0,
- errp);
-if (ret == QIO_CHANNEL_ERR_BLOCK) {
-return ret;
-}
-
-/* read error or other side closed connection */
-if (ret <= 0) {
-goto fatal;
-}
-
-if (ret < sizeof(hdr)) {
-error_setg(errp, "short read of header");
-goto fatal;
-}
-
-/*
- * Validate header
- */
-if (hdr.size < sizeof(VFIOUserHdr)) {
-error_setg(errp, "bad header size");
-goto fatal;
-}
-switch (hdr.flags & VFIO_USER_TYPE) {
-case VFIO_USER_REQUEST:
-isreply = false;
-break;
-case VFIO_USER_REPLY:
-isreply = true;
-break;
-default:
-error_setg(errp, "unknown message type");
+ret = vfio_user_recv_hdr(proxy, errp, &hdr, &fdp, &numfds, &isreply);
+if (ret < 0) {
+if (ret == QIO_CHANNEL_ERR_BLOCK) {
+return ret;
+}
goto fatal;
}
-trace_vfio_user_recv_hdr(proxy->sockname, hdr.id, hdr.command, hdr.size,
- hdr.flags);
/*
* For replies, find the matching pending request.
ATB,
Mark.
[PATCH v3 3/5] vfio-user: refactor out header handling
Simplify vfio_user_recv_one() by moving the header handling out to a
helper function.
Signed-off-by: John Levon
---
hw/vfio-user/proxy.c | 101 +--
1 file changed, 60 insertions(+), 41 deletions(-)
diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c
index 82c76c6665..87e50501af 100644
--- a/hw/vfio-user/proxy.c
+++ b/hw/vfio-user/proxy.c
@@ -218,6 +218,61 @@ static int vfio_user_complete(VFIOUserProxy *proxy, Error
**errp)
return 1;
}
+static int vfio_user_recv_hdr(VFIOUserProxy *proxy, Error **errp,
+ VFIOUserHdr *hdr, int **fdp, size_t *numfdp,
+ bool *isreply)
+{
+struct iovec iov = {
+.iov_base = hdr,
+.iov_len = sizeof(*hdr),
+};
+int ret;
+
+/*
+ * Read header
+ */
+ret = qio_channel_readv_full(proxy->ioc, &iov, 1, fdp, numfdp, 0,
+ errp);
+if (ret == QIO_CHANNEL_ERR_BLOCK) {
+return ret;
+}
+
+/* read error or other side closed connection */
+if (ret <= 0) {
+error_setg(errp, "failed to read header");
+return -1;
+}
+
+if (ret < sizeof(*hdr)) {
+error_setg(errp, "short read of header");
+return -1;
+}
+
+/*
+ * Validate header
+ */
+if (hdr->size < sizeof(*hdr)) {
+error_setg(errp, "bad header size");
+return -1;
+}
+
+switch (hdr->flags & VFIO_USER_TYPE) {
+case VFIO_USER_REQUEST:
+*isreply = false;
+break;
+case VFIO_USER_REPLY:
+*isreply = true;
+break;
+default:
+error_setg(errp, "unknown message type");
+return -1;
+}
+
+trace_vfio_user_recv_hdr(proxy->sockname, hdr->id, hdr->command, hdr->size,
+ hdr->flags);
+return 0;
+}
+
/*
* Receive and process one incoming message.
*
@@ -230,10 +285,6 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
g_autofree int *fdp = NULL;
VFIOUserFDs *reqfds;
VFIOUserHdr hdr;
-struct iovec iov = {
-.iov_base = &hdr,
-.iov_len = sizeof(hdr),
-};
bool isreply = false;
int i, ret;
size_t msgleft, numfds = 0;
@@ -257,45 +308,13 @@ static int vfio_user_recv_one(VFIOUserProxy *proxy, Error
**errp)
/* else fall into reading another msg */
}
-/*
- * Read header
- */
-ret = qio_channel_readv_full(proxy->ioc, &iov, 1, &fdp, &numfds, 0,
- errp);
-if (ret == QIO_CHANNEL_ERR_BLOCK) {
-return ret;
-}
-
-/* read error or other side closed connection */
-if (ret <= 0) {
-goto fatal;
-}
-
-if (ret < sizeof(hdr)) {
-error_setg(errp, "short read of header");
-goto fatal;
-}
-
-/*
- * Validate header
- */
-if (hdr.size < sizeof(VFIOUserHdr)) {
-error_setg(errp, "bad header size");
-goto fatal;
-}
-switch (hdr.flags & VFIO_USER_TYPE) {
-case VFIO_USER_REQUEST:
-isreply = false;
-break;
-case VFIO_USER_REPLY:
-isreply = true;
-break;
-default:
-error_setg(errp, "unknown message type");
+ret = vfio_user_recv_hdr(proxy, errp, &hdr, &fdp, &numfds, &isreply);
+if (ret < 0) {
+if (ret == QIO_CHANNEL_ERR_BLOCK) {
+return ret;
+}
goto fatal;
}
-trace_vfio_user_recv_hdr(proxy->sockname, hdr.id, hdr.command, hdr.size,
- hdr.flags);
/*
* For replies, find the matching pending request.
--
2.43.0
