From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPrUl-0008F3-81 for qemu-devel@nongnu.org; Wed, 20 Jul 2016 09:25:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPrUh-00035v-1g for qemu-devel@nongnu.org; Wed, 20 Jul 2016 09:25:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45780) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPrUg-000358-Ps for qemu-devel@nongnu.org; Wed, 20 Jul 2016 09:24:58 -0400 Date: Wed, 20 Jul 2016 16:24:53 +0300 From: "Michael S. Tsirkin" Message-ID: <20160720162315-mutt-send-email-mst@kernel.org> References: <20160706184721.2007-1-marcandre.lureau@redhat.com> <20160706184721.2007-11-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20160706184721.2007-11-marcandre.lureau@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 10/28] vhost: change some assert() for error_report() or silent fail 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:03PM +0200, marcandre.lureau@redhat.com wro= te: > From: Marc-Andr=E9 Lureau >=20 > Calling a vhost operation may fail, especially with disconnectable > backends. Treat some that look harmless as recoverable errors (print > error, or ignore on error code path). >=20 > Signed-off-by: Marc-Andr=E9 Lureau So this is handy for now until disconnect handling stabilizes, but eventually these bnign errors should be hidden from the user. What I'd like to see is something like #define VHOST_DISCONNECT_DEBUG error_report and then use that. After things stabilize we'll be able to drop these easily. > --- > hw/virtio/vhost.c | 32 +++++++++++++++++++++----------- > 1 file changed, 21 insertions(+), 11 deletions(-) >=20 > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > index 75bc51e..e03a031 100644 > --- a/hw/virtio/vhost.c > +++ b/hw/virtio/vhost.c > @@ -400,7 +400,10 @@ static inline void vhost_dev_log_resize(struct vho= st_dev *dev, uint64_t size) > /* 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); > + if (r < 0) { > + error_report("Failed to change backend log"); > + } > + > vhost_log_put(dev, true); > dev->log =3D log; > dev->log_size =3D size; > @@ -567,7 +570,9 @@ static void vhost_commit(MemoryListener *listener) > =20 > if (!dev->log_enabled) { > r =3D dev->vhost_ops->vhost_set_mem_table(dev, dev->mem); > - assert(r >=3D 0); > + if (r < 0) { > + error_report("Failed to set mem table"); > + } > dev->memory_changed =3D false; > return; > } > @@ -580,7 +585,9 @@ static void vhost_commit(MemoryListener *listener) > vhost_dev_log_resize(dev, log_size + VHOST_LOG_BUFFER); > } > r =3D dev->vhost_ops->vhost_set_mem_table(dev, dev->mem); > - assert(r >=3D 0); > + if (r < 0) { > + error_report("Failed to set mem table"); > + } > /* To log less, can only decrease log size after table update. */ > if (dev->log_size > log_size + VHOST_LOG_BUFFER) { > vhost_dev_log_resize(dev, log_size); > @@ -649,6 +656,7 @@ static int vhost_virtqueue_set_addr(struct vhost_de= v *dev, > }; > int r =3D dev->vhost_ops->vhost_set_vring_addr(dev, &addr); > if (r < 0) { > + error_report("Failed to set vring addr"); > return -errno; > } > return 0; > @@ -662,12 +670,15 @@ static int vhost_dev_set_features(struct vhost_de= v *dev, bool enable_log) > features |=3D 0x1ULL << VHOST_F_LOG_ALL; > } > r =3D dev->vhost_ops->vhost_set_features(dev, features); > + if (r < 0) { > + error_report("Failed to set features"); > + } > return r < 0 ? -errno : 0; > } > =20 > static int vhost_dev_set_log(struct vhost_dev *dev, bool enable_log) > { > - int r, t, i, idx; > + int r, i, idx; > r =3D vhost_dev_set_features(dev, enable_log); > if (r < 0) { > goto err_features; > @@ -684,12 +695,10 @@ static int vhost_dev_set_log(struct vhost_dev *de= v, bool enable_log) > err_vq: > for (; i >=3D 0; --i) { > idx =3D dev->vhost_ops->vhost_get_vq_index(dev, dev->vq_index = + i); > - t =3D vhost_virtqueue_set_addr(dev, dev->vqs + i, idx, > - dev->log_enabled); > - assert(t >=3D 0); > + vhost_virtqueue_set_addr(dev, dev->vqs + i, idx, > + dev->log_enabled); > } > - t =3D vhost_dev_set_features(dev, dev->log_enabled); > - assert(t >=3D 0); > + vhost_dev_set_features(dev, dev->log_enabled); > err_features: > return r; > } > @@ -937,7 +946,6 @@ static void vhost_virtqueue_stop(struct vhost_dev *= dev, > } > } > =20 > - assert (r >=3D 0); > cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vde= v, idx), > 0, virtio_queue_get_ring_size(vdev, idx)= ); > cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vde= v, idx), > @@ -1191,7 +1199,9 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev,= VirtIODevice *vdev, int n, > =20 > file.index =3D hdev->vhost_ops->vhost_get_vq_index(hdev, n); > r =3D hdev->vhost_ops->vhost_set_vring_call(hdev, &file); > - assert(r >=3D 0); > + if (r < 0) { > + error_report("Failed to set vring call"); > + } > } > =20 > uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature= _bits, > --=20 > 2.9.0