From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38440) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPrYU-00055a-TB for qemu-devel@nongnu.org; Wed, 20 Jul 2016 09:28:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPrYQ-0003ty-MF for qemu-devel@nongnu.org; Wed, 20 Jul 2016 09:28:53 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44442) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPrYQ-0003ts-Dm for qemu-devel@nongnu.org; Wed, 20 Jul 2016 09:28:50 -0400 Date: Wed, 20 Jul 2016 16:28:44 +0300 From: "Michael S. Tsirkin" Message-ID: <20160720162630-mutt-send-email-mst@kernel.org> References: <20160706184721.2007-1-marcandre.lureau@redhat.com> <20160706184721.2007-14-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20160706184721.2007-14-marcandre.lureau@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 13/28] vhost-user: check vhost_user_{read, write}() return value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com Cc: qemu-devel@nongnu.org, mukawa@igel.co.jp, yuanhan.liu@linux.intel.com, victork@redhat.com, jonshin@cisco.com On Wed, Jul 06, 2016 at 08:47:06PM +0200, marcandre.lureau@redhat.com wro= te: > From: Marc-Andr=E9 Lureau >=20 > More error checking to break code flow and report appropriate errors. >=20 > Signed-off-by: Marc-Andr=E9 Lureau So this will cause asserts here: /* inform backend of log switching, this must be done before releasing the current log, to ensure no logging is lost */ r =3D dev->vhost_ops->vhost_set_log_base(dev, log_base, log); assert(r >=3D 0); which is the opposite of what we want. For vhost_net ioctl failure is a fatal error bit for vhost user this means backend exited and will not change memory so it's benign. the right thing to do is to return 0 status to caller on most errors. If that is necessary for debugging, add a printout for now. > --- > hw/virtio/vhost-user.c | 50 ++++++++++++++++++++++++++++++++++--------= -------- > 1 file changed, 34 insertions(+), 16 deletions(-) >=20 > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > index 5dae496..819481d 100644 > --- a/hw/virtio/vhost-user.c > +++ b/hw/virtio/vhost-user.c > @@ -214,12 +214,14 @@ static int vhost_user_set_log_base(struct vhost_d= ev *dev, uint64_t base, > fds[fd_num++] =3D log->fd; > } > =20 > - vhost_user_write(dev, &msg, fds, fd_num); > + if (vhost_user_write(dev, &msg, fds, fd_num) < 0) { > + return -1; > + } > =20 > if (shmfd) { > msg.size =3D 0; > if (vhost_user_read(dev, &msg) < 0) { > - return 0; > + return -1; > } > =20 > if (msg.request !=3D VHOST_USER_SET_LOG_BASE) { > @@ -275,7 +277,9 @@ static int vhost_user_set_mem_table(struct vhost_de= v *dev, > msg.size +=3D sizeof(msg.payload.memory.padding); > msg.size +=3D fd_num * sizeof(VhostUserMemoryRegion); > =20 > - vhost_user_write(dev, &msg, fds, fd_num); > + if (vhost_user_write(dev, &msg, fds, fd_num) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -290,7 +294,9 @@ static int vhost_user_set_vring_addr(struct vhost_d= ev *dev, > .size =3D sizeof(msg.payload.addr), > }; > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -313,7 +319,9 @@ static int vhost_set_vring(struct vhost_dev *dev, > .size =3D sizeof(msg.payload.state), > }; > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -360,10 +368,12 @@ static int vhost_user_get_vring_base(struct vhost= _dev *dev, > .size =3D sizeof(msg.payload.state), > }; > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > if (vhost_user_read(dev, &msg) < 0) { > - return 0; > + return -1; > } > =20 > if (msg.request !=3D VHOST_USER_GET_VRING_BASE) { > @@ -401,7 +411,9 @@ static int vhost_set_vring_file(struct vhost_dev *d= ev, > msg.payload.u64 |=3D VHOST_USER_VRING_NOFD_MASK; > } > =20 > - vhost_user_write(dev, &msg, fds, fd_num); > + if (vhost_user_write(dev, &msg, fds, fd_num) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -427,7 +439,9 @@ static int vhost_user_set_u64(struct vhost_dev *dev= , int request, uint64_t u64) > .size =3D sizeof(msg.payload.u64), > }; > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -455,10 +469,12 @@ static int vhost_user_get_u64(struct vhost_dev *d= ev, int request, uint64_t *u64) > return 0; > } > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > if (vhost_user_read(dev, &msg) < 0) { > - return 0; > + return -1; > } > =20 > if (msg.request !=3D request) { > @@ -489,7 +505,9 @@ static int vhost_user_set_owner(struct vhost_dev *d= ev) > .flags =3D VHOST_USER_VERSION, > }; > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -501,7 +519,9 @@ static int vhost_user_reset_device(struct vhost_dev= *dev) > .flags =3D VHOST_USER_VERSION, > }; > =20 > - vhost_user_write(dev, &msg, NULL, 0); > + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { > + return -1; > + } > =20 > return 0; > } > @@ -588,7 +608,6 @@ static bool vhost_user_requires_shm_log(struct vhos= t_dev *dev) > static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_= addr) > { > VhostUserMsg msg =3D { 0 }; > - int err; > =20 > assert(dev->vhost_ops->backend_type =3D=3D VHOST_BACKEND_TYPE_USER= ); > =20 > @@ -605,8 +624,7 @@ static int vhost_user_migration_done(struct vhost_d= ev *dev, char* mac_addr) > memcpy((char *)&msg.payload.u64, mac_addr, 6); > msg.size =3D sizeof(msg.payload.u64); > =20 > - err =3D vhost_user_write(dev, &msg, NULL, 0); > - return err; > + return vhost_user_write(dev, &msg, NULL, 0); > } > return -1; > } > --=20 > 2.9.0