[PATCH 2/3] libvhost-user: Fix extra vu_add/rem_mem_reg reply

Kevin Wolf posted 3 patches 3 years, 10 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
[PATCH 2/3] libvhost-user: Fix extra vu_add/rem_mem_reg reply
Posted by Kevin Wolf 3 years, 10 months ago
Outside of postcopy mode, neither VHOST_USER_ADD_MEM_REG nor
VHOST_USER_REM_MEM_REG are supposed to send a reply unless explicitly
requested with the need_reply flag. Their current implementation always
sends a reply, even if it isn't requested. This confuses the master
because it will interpret the reply as a reply for the next message for
which it actually expects a reply.

need_reply is already handled correctly by vu_dispatch(), so just don't
send a reply in the non-postcopy part of the message handler for these
two commands.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 47d2efc60f..eccaff5168 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -800,8 +800,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 
         DPRINT("Successfully added new region\n");
         dev->nregions++;
-        vmsg_set_reply_u64(vmsg, 0);
-        return true;
+        return false;
     }
 }
 
@@ -874,15 +873,13 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
         }
     }
 
-    if (found) {
-        vmsg_set_reply_u64(vmsg, 0);
-    } else {
+    if (!found) {
         vu_panic(dev, "Specified region not found\n");
     }
 
     close(vmsg->fds[0]);
 
-    return true;
+    return false;
 }
 
 static bool
-- 
2.35.1
Re: [PATCH 2/3] libvhost-user: Fix extra vu_add/rem_mem_reg reply
Posted by Raphael Norwitz 3 years, 10 months ago
On Thu, Apr 07, 2022 at 03:36:56PM +0200, Kevin Wolf wrote:
> Outside of postcopy mode, neither VHOST_USER_ADD_MEM_REG nor
> VHOST_USER_REM_MEM_REG are supposed to send a reply unless explicitly
> requested with the need_reply flag. Their current implementation always
> sends a reply, even if it isn't requested. This confuses the master
> because it will interpret the reply as a reply for the next message for
> which it actually expects a reply.
> 
> need_reply is already handled correctly by vu_dispatch(), so just don't
> send a reply in the non-postcopy part of the message handler for these
> two commands.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> ---
>  subprojects/libvhost-user/libvhost-user.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
> index 47d2efc60f..eccaff5168 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -800,8 +800,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
>  
>          DPRINT("Successfully added new region\n");
>          dev->nregions++;
> -        vmsg_set_reply_u64(vmsg, 0);
> -        return true;
> +        return false;
>      }
>  }
>  
> @@ -874,15 +873,13 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
>          }
>      }
>  
> -    if (found) {
> -        vmsg_set_reply_u64(vmsg, 0);
> -    } else {
> +    if (!found) {
>          vu_panic(dev, "Specified region not found\n");
>      }
>  
>      close(vmsg->fds[0]);
>  
> -    return true;
> +    return false;
>  }
>  
>  static bool
> -- 
> 2.35.1
>