On Tue, 5 Nov 2024 at 16:19, Stefano Garzarella <sgarz...@redhat.com> wrote: > VHOST_OPS_DEBUG() is usually used in the error path when calling a > `dev->vhost_ops` callback. In addition vhost_device_iotlb_miss() is > already reporting error through error_report() in the error path, so I > think we can remove this line.
* Okay. > Should we add a new label in the error path and call > `hdev->vhost_ops->vhost_dev_start` with `false`? > > Maybe we need to call also `hdev->vhost_ops->vhost_set_iotlb_callback` > with `false`. === diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index a70b7422b5..089eff438e 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -2137,15 +2137,18 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings) goto fail_log; } } + + bool start = true; +dev_restart: if (hdev->vhost_ops->vhost_dev_start) { - r = hdev->vhost_ops->vhost_dev_start(hdev, true); + r = hdev->vhost_ops->vhost_dev_start(hdev, start); if (r) { goto fail_start; } } if (vhost_dev_has_iommu(hdev) && hdev->vhost_ops->vhost_set_iotlb_callback) { - hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true); + hdev->vhost_ops->vhost_set_iotlb_callback(hdev, start); /* Update used ring information for IOTLB to work correctly, * vhost-kernel code requires for this.*/ @@ -2154,7 +2157,8 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings) r = vhost_device_iotlb_miss(hdev, vq->used_phys, true); if (r) { VHOST_OPS_DEBUG(r, "vhost_device_iotlb_miss failed"); - goto fail_start; + start = false; + goto dev_restart; } } } === * Something like above? === static int vhost_user_dev_start(struct vhost_dev *dev, bool started) { ... if (started) { return vhost_user_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK); } else { return 0; } } static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) { /* No-op as the receive channel is not dedicated to IOTLB messages. */ } === * Calling vhost_user_dev_start() and vhost_user_set_iotlb_callback() with 'false' does not seem to do much. Not sure how'll that help. If we 'goto fail_start;', libvirtd(8) might restart the guest and thus start the vhost device again. ...wdyt? Thank you. --- - Prasad