diff mbox

vhost-user: pass message as a pointer to process_message_reply()

Message ID 20170524083446.32261-1-maxime.coquelin@redhat.com
State New
Headers show

Commit Message

Maxime Coquelin May 24, 2017, 8:34 a.m. UTC
process_message_reply() was recently updated to get full message
content instead of only its request field.

There is no need to copy all the struct content into the stack,
so just pass its pointer.

Cc: Zhiyong Yang <zhiyong.yang@intel.com>
Fixes: 60cd11024f41 ("hw/virtio: fix vhost user fails to startup when MQ")
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 hw/virtio/vhost-user.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Marc-André Lureau May 24, 2017, 8:40 a.m. UTC | #1
On Wed, May 24, 2017 at 11:35 AM Maxime Coquelin <maxime.coquelin@redhat.com>
wrote:

> process_message_reply() was recently updated to get full message
> content instead of only its request field.
>
> There is no need to copy all the struct content into the stack,
> so just pass its pointer.
>
> Cc: Zhiyong Yang <zhiyong.yang@intel.com>
> Fixes: 60cd11024f41 ("hw/virtio: fix vhost user fails to startup when MQ")
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  hw/virtio/vhost-user.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index b87a176..baf2487 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -162,11 +162,11 @@ fail:
>  }
>
>  static int process_message_reply(struct vhost_dev *dev,
> -                                 VhostUserMsg msg)
> +                                 VhostUserMsg *msg)
>

Can you make it const?


>  {
>      VhostUserMsg msg_reply;
>
> -    if ((msg.flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
> +    if ((msg->flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
>          return 0;
>      }
>
> @@ -174,10 +174,10 @@ static int process_message_reply(struct vhost_dev
> *dev,
>          return -1;
>      }
>
> -    if (msg_reply.request != msg.request) {
> +    if (msg_reply.request != msg->request) {
>          error_report("Received unexpected msg type."
>                       "Expected %d received %d",
> -                     msg.request, msg_reply.request);
> +                     msg->request, msg_reply.request);
>          return -1;
>      }
>
> @@ -324,7 +324,7 @@ static int vhost_user_set_mem_table(struct vhost_dev
> *dev,
>      }
>
>      if (reply_supported) {
> -        return process_message_reply(dev, msg);
> +        return process_message_reply(dev, &msg);
>      }
>
>      return 0;
> @@ -716,7 +716,7 @@ static int vhost_user_net_set_mtu(struct vhost_dev
> *dev, uint16_t mtu)
>
>      /* If reply_ack supported, slave has to ack specified MTU is valid */
>      if (reply_supported) {
> -        return process_message_reply(dev, msg);
> +        return process_message_reply(dev, &msg);
>      }
>
>      return 0;
> --
> 2.9.4
>
>
> --
Marc-André Lureau
Maxime Coquelin May 24, 2017, 8:42 a.m. UTC | #2
On 05/24/2017 10:40 AM, Marc-André Lureau wrote:
> 
> 
> On Wed, May 24, 2017 at 11:35 AM Maxime Coquelin 
> <maxime.coquelin@redhat.com <mailto:maxime.coquelin@redhat.com>> wrote:
> 
>     process_message_reply() was recently updated to get full message
>     content instead of only its request field.
> 
>     There is no need to copy all the struct content into the stack,
>     so just pass its pointer.
> 
>     Cc: Zhiyong Yang <zhiyong.yang@intel.com
>     <mailto:zhiyong.yang@intel.com>>
>     Fixes: 60cd11024f41 ("hw/virtio: fix vhost user fails to startup
>     when MQ")
>     Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com
>     <mailto:maxime.coquelin@redhat.com>>
>     ---
>       hw/virtio/vhost-user.c | 12 ++++++------
>       1 file changed, 6 insertions(+), 6 deletions(-)
> 
>     diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>     index b87a176..baf2487 100644
>     --- a/hw/virtio/vhost-user.c
>     +++ b/hw/virtio/vhost-user.c
>     @@ -162,11 +162,11 @@ fail:
>       }
> 
>       static int process_message_reply(struct vhost_dev *dev,
>     -                                 VhostUserMsg msg)
>     +                                 VhostUserMsg *msg)
> 
> 
> Can you make it const?
Sure!

Thanks,
Maxime
Yang, Zhiyong May 24, 2017, 8:50 a.m. UTC | #3
Hi, Maxime:

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Wednesday, May 24, 2017 4:35 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; mst@redhat.com; qemu-
> devel@nongnu.org; jfreiman@redhat.com; marcandre.lureau@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH] vhost-user: pass message as a pointer to
> process_message_reply()
> 
> process_message_reply() was recently updated to get full message content
> instead of only its request field.
> 
> There is no need to copy all the struct content into the stack, so just pass its
> pointer.
> 
> Cc: Zhiyong Yang <zhiyong.yang@intel.com>
> Fixes: 60cd11024f41 ("hw/virtio: fix vhost user fails to startup when MQ")
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Good modification. Thanks.

Reviewed-by:  Zhiyong Yang <zhiyong.yang@intel.com>

> ---
>  hw/virtio/vhost-user.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index
> b87a176..baf2487 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -162,11 +162,11 @@ fail:
>  }
> 
>  static int process_message_reply(struct vhost_dev *dev,
> -                                 VhostUserMsg msg)
> +                                 VhostUserMsg *msg)
>  {
>      VhostUserMsg msg_reply;
> 
> -    if ((msg.flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
> +    if ((msg->flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
>          return 0;
>      }
> 
> @@ -174,10 +174,10 @@ static int process_message_reply(struct vhost_dev
> *dev,
>          return -1;
>      }
> 
> -    if (msg_reply.request != msg.request) {
> +    if (msg_reply.request != msg->request) {
>          error_report("Received unexpected msg type."
>                       "Expected %d received %d",
> -                     msg.request, msg_reply.request);
> +                     msg->request, msg_reply.request);
>          return -1;
>      }
> 
> @@ -324,7 +324,7 @@ static int vhost_user_set_mem_table(struct vhost_dev
> *dev,
>      }
> 
>      if (reply_supported) {
> -        return process_message_reply(dev, msg);
> +        return process_message_reply(dev, &msg);
>      }
> 
>      return 0;
> @@ -716,7 +716,7 @@ static int vhost_user_net_set_mtu(struct vhost_dev
> *dev, uint16_t mtu)
> 
>      /* If reply_ack supported, slave has to ack specified MTU is valid */
>      if (reply_supported) {
> -        return process_message_reply(dev, msg);
> +        return process_message_reply(dev, &msg);
>      }
> 
>      return 0;
> --
> 2.9.4
Jens Freimann May 24, 2017, 8:50 a.m. UTC | #4
On Wed, May 24, 2017 at 10:42:30AM +0200, Maxime Coquelin wrote:
> 
> 
> On 05/24/2017 10:40 AM, Marc-André Lureau wrote:
> > 
> > 
> > On Wed, May 24, 2017 at 11:35 AM Maxime Coquelin
> > <maxime.coquelin@redhat.com <mailto:maxime.coquelin@redhat.com>> wrote:
> > 
> >     process_message_reply() was recently updated to get full message
> >     content instead of only its request field.
> > 
> >     There is no need to copy all the struct content into the stack,
> >     so just pass its pointer.
> > 
> >     Cc: Zhiyong Yang <zhiyong.yang@intel.com
> >     <mailto:zhiyong.yang@intel.com>>
> >     Fixes: 60cd11024f41 ("hw/virtio: fix vhost user fails to startup
> >     when MQ")
> >     Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com
> >     <mailto:maxime.coquelin@redhat.com>>
> >     ---
> >       hw/virtio/vhost-user.c | 12 ++++++------
> >       1 file changed, 6 insertions(+), 6 deletions(-)
> > 
> >     diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >     index b87a176..baf2487 100644
> >     --- a/hw/virtio/vhost-user.c
> >     +++ b/hw/virtio/vhost-user.c
> >     @@ -162,11 +162,11 @@ fail:
> >       }
> > 
> >       static int process_message_reply(struct vhost_dev *dev,
> >     -                                 VhostUserMsg msg)
> >     +                                 VhostUserMsg *msg)
> > 
> > 
> > Can you make it const?
> Sure!

with that:

Reviewed-by: Jens Freimann <jfreimann@redhat.com>
diff mbox

Patch

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index b87a176..baf2487 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -162,11 +162,11 @@  fail:
 }
 
 static int process_message_reply(struct vhost_dev *dev,
-                                 VhostUserMsg msg)
+                                 VhostUserMsg *msg)
 {
     VhostUserMsg msg_reply;
 
-    if ((msg.flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
+    if ((msg->flags & VHOST_USER_NEED_REPLY_MASK) == 0) {
         return 0;
     }
 
@@ -174,10 +174,10 @@  static int process_message_reply(struct vhost_dev *dev,
         return -1;
     }
 
-    if (msg_reply.request != msg.request) {
+    if (msg_reply.request != msg->request) {
         error_report("Received unexpected msg type."
                      "Expected %d received %d",
-                     msg.request, msg_reply.request);
+                     msg->request, msg_reply.request);
         return -1;
     }
 
@@ -324,7 +324,7 @@  static int vhost_user_set_mem_table(struct vhost_dev *dev,
     }
 
     if (reply_supported) {
-        return process_message_reply(dev, msg);
+        return process_message_reply(dev, &msg);
     }
 
     return 0;
@@ -716,7 +716,7 @@  static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
 
     /* If reply_ack supported, slave has to ack specified MTU is valid */
     if (reply_supported) {
-        return process_message_reply(dev, msg);
+        return process_message_reply(dev, &msg);
     }
 
     return 0;