Re: [Qemu-devel] libvhost-user: Fix the VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD check

[Boeuf, Sebastien]

On Wed, 2019-08-07 at 17:09 +0100, Dr. David Alan Gilbert wrote:
> * Boeuf, Sebastien (address@hidden) wrote:
> > From 0a53a81db6dd069f9b7bcdcd386845bceb3a2ac6 Mon Sep 17 00:00:00
> > 2001
> > From: Sebastien Boeuf <address@hidden>
> > Date: Wed, 7 Aug 2019 07:15:32 -0700
> > Subject: [PATCH] libvhost-user: Fix the
> > VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD
> >  check
> > 
> > Vhost user protocol features are set as a bitmask. And the
> > following
> > constant VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD value is 10 because
> > the
> > bit
> > 10 indicates if the features is set or not.
> > 
> > The proper way to check for the presence or absence of this feature
> > is
> > to shift 1 by the value of this constant and then mask it with the
> > actual bitmask representing the supported protocol features.
> > 
> > This patch aims to fix the current code as it was not doing the
> > shifting, but instead it was masking directly with the value of the
> > constant itself.
> > 
> > Signed-off-by: Sebastien Boeuf <address@hidden>
> 
> Nicely spotted.
> 
> Two things;
>   a) I think your mail client has wrapped the lines at some point.
>   b) I think this is why the has_feature() functione exists, so does
>      that become
> 
>       if (!has_feature(dev->protocol_features,
> VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD))

Ah yes but that's because I forgot to check the patch format first :(

I'm going to update the patch.

Thanks,
Sebastien
> 
> Dave
> 
> > ---
> >  contrib/libvhost-user/libvhost-user.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/contrib/libvhost-user/libvhost-user.c
> > b/contrib/libvhost-
> > user/libvhost-user.c
> > index fb61142bcc..11909fb7c1 100644
> > --- a/contrib/libvhost-user/libvhost-user.c
> > +++ b/contrib/libvhost-user/libvhost-user.c
> > @@ -1112,7 +1112,7 @@ bool vu_set_queue_host_notifier(VuDev *dev,
> > VuVirtq *vq, int fd,
> >  
> >      vmsg.fd_num = fd_num;
> >  
> > -    if ((dev->protocol_features &
> > VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD)
> > == 0) {
> > +    if ((dev->protocol_features & (1ULL <<
> > VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD)) == 0) {
> >          return false;
> >      }
> >  
> > @@ -2537,7 +2537,7 @@ int64_t vu_fs_cache_request(VuDev *dev,
> > VhostUserSlaveRequest req, int fd,
> >  
> >      vmsg.fd_num = fd_num;
> >  
> > -    if ((dev->protocol_features &
> > VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD)
> > == 0) {
> > +    if ((dev->protocol_features & (1ULL <<
> > VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD)) == 0) {
> >          return -EINVAL;
> >      }
> >  
> > -- 
> > 2.17.1
> 
> --
> Dr. David Alan Gilbert / address@hidden / Manchester, UK