[PATCH 16/33] vhost: simplify vhost_dev_init() error-path

Vladimir Sementsov-Ogievskiy posted 33 patches 3 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Zhenwei Pi <pizhenwei@bytedance.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Raphael Norwitz <raphael@enfabrica.net>, Jason Wang <jasowang@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PATCH 16/33] vhost: simplify vhost_dev_init() error-path
Posted by Vladimir Sementsov-Ogievskiy 3 months ago
No reason to rollback setting up busyloop timeout on failure.
We don't do such rollback for other things we setup in backend.
Also, look at vhost_net_init() in hw/net/vhost_net.c: we may fail
after successfully called vhost_dev_init(), and in this case we'll
just call vhost_dev_cleanup(), which doesn't rollback busyloop
timeout.

So, let's keep it simple.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
 hw/virtio/vhost.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index f6ee59425f..a3620c82d8 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1602,7 +1602,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                                                      busyloop_timeout);
             if (r < 0) {
                 error_setg_errno(errp, -r, "Failed to set busyloop timeout");
-                goto fail_busyloop;
+                goto fail;
             }
         }
     }
@@ -1642,7 +1642,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
     if (hdev->migration_blocker != NULL) {
         r = migrate_add_blocker_normal(&hdev->migration_blocker, errp);
         if (r < 0) {
-            goto fail_busyloop;
+            goto fail;
         }
     }
 
@@ -1674,17 +1674,11 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
                    " than current number of used (%d) and reserved (%d)"
                    " memory slots for memory devices.", limit, used, reserved);
         r = -EINVAL;
-        goto fail_busyloop;
+        goto fail;
     }
 
     return 0;
 
-fail_busyloop:
-    if (busyloop_timeout) {
-        while (--i >= 0) {
-            vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
-        }
-    }
 fail:
     hdev->nvqs = n_initialized_vqs;
     vhost_dev_cleanup(hdev);
-- 
2.48.1
Re: [PATCH 16/33] vhost: simplify vhost_dev_init() error-path
Posted by Raphael Norwitz 1 month ago
Acked-by: Raphael Norwitz <raphael.s.norwitz@gmail.com>

On Wed, Aug 13, 2025 at 12:54 PM Vladimir Sementsov-Ogievskiy
<vsementsov@yandex-team.ru> wrote:
>
> No reason to rollback setting up busyloop timeout on failure.
> We don't do such rollback for other things we setup in backend.
> Also, look at vhost_net_init() in hw/net/vhost_net.c: we may fail
> after successfully called vhost_dev_init(), and in this case we'll
> just call vhost_dev_cleanup(), which doesn't rollback busyloop
> timeout.
>
> So, let's keep it simple.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  hw/virtio/vhost.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index f6ee59425f..a3620c82d8 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1602,7 +1602,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>                                                       busyloop_timeout);
>              if (r < 0) {
>                  error_setg_errno(errp, -r, "Failed to set busyloop timeout");
> -                goto fail_busyloop;
> +                goto fail;
>              }
>          }
>      }
> @@ -1642,7 +1642,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>      if (hdev->migration_blocker != NULL) {
>          r = migrate_add_blocker_normal(&hdev->migration_blocker, errp);
>          if (r < 0) {
> -            goto fail_busyloop;
> +            goto fail;
>          }
>      }
>
> @@ -1674,17 +1674,11 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque,
>                     " than current number of used (%d) and reserved (%d)"
>                     " memory slots for memory devices.", limit, used, reserved);
>          r = -EINVAL;
> -        goto fail_busyloop;
> +        goto fail;
>      }
>
>      return 0;
>
> -fail_busyloop:
> -    if (busyloop_timeout) {
> -        while (--i >= 0) {
> -            vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, 0);
> -        }
> -    }
>  fail:
>      hdev->nvqs = n_initialized_vqs;
>      vhost_dev_cleanup(hdev);
> --
> 2.48.1
>
>