* Boeuf, Sebastien (sebastien.bo...@intel.com) wrote:
> From 734625fe0c031d26e612800cd9331235f58ae2e0 Mon Sep 17 00:00:00 2001
> From: Sebastien Boeuf <sebastien.bo...@intel.com>
> 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 <sebastien.bo...@intel.com>
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Michael: Can you pick this up?
> ---
> contrib/libvhost-user/libvhost-user.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index fb61142bcc..d75a9c86ed 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -1112,7 +1112,8 @@ 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 +2538,8 @@ 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
>
> ________________________________________
> From: Boeuf, Sebastien
> Sent: Wednesday, August 07, 2019 9:35 AM
> To: dgilb...@redhat.com
> Cc: marcandre.lur...@redhat.com; qemu-devel@nongnu.org
> Subject: RE: libvhost-user: Fix the VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD check
>
> From 950c62dd450c8f6c3fc04269bbefa3a368bb39b6 Mon Sep 17 00:00:00 2001
> From: Sebastien Boeuf <sebastien.bo...@intel.com>
> 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 <sebastien.bo...@intel.com>
> ---
> contrib/libvhost-user/libvhost-user.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index fb61142bcc..8ff387deff 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -71,7 +71,7 @@
>
> /* The version of the protocol we support */
> #define VHOST_USER_VERSION 1
> -#define LIBVHOST_USER_DEBUG 0
> +#define LIBVHOST_USER_DEBUG 1
>
> #define DPRINT(...) \
> do { \
> @@ -1112,7 +1112,8 @@ 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 +2538,8 @@ 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
>
> ________________________________________
> From: Boeuf, Sebastien
> Sent: Wednesday, August 07, 2019 9:24 AM
> To: dgilb...@redhat.com
> Cc: marcandre.lur...@redhat.com; qemu-devel@nongnu.org
> Subject: Re: libvhost-user: Fix the VHOST_USER_PROTOCOL_F_SLAVE_SEND_FD check
>
> On Wed, 2019-08-07 at 17:09 +0100, Dr. David Alan Gilbert wrote:
> > * Boeuf, Sebastien (sebastien.bo...@intel.com) wrote:
> > > From 0a53a81db6dd069f9b7bcdcd386845bceb3a2ac6 Mon Sep 17 00:00:00
> > > 2001
> > > From: Sebastien Boeuf <sebastien.bo...@intel.com>
> > > 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 <sebastien.bo...@intel.com>
> >
> > 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 / dgilb...@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
