[PATCH 6/7] vhost-user-blk: Factor out vhost_user_blk_realize_connect()

Kevin Wolf posted 7 patches 4 years, 1 month ago
Maintainers: Kevin Wolf <kwolf@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Jason Wang <jasowang@redhat.com>, Raphael Norwitz <raphael.norwitz@nutanix.com>, Stefan Hajnoczi <stefanha@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Gonglei (Arei)" <arei.gonglei@huawei.com>, Gerd Hoffmann <kraxel@redhat.com>, Max Reitz <mreitz@redhat.com>, Fam Zheng <fam@euphon.net>
[PATCH 6/7] vhost-user-blk: Factor out vhost_user_blk_realize_connect()
Posted by Kevin Wolf 4 years, 1 month ago
This function is the part that we will want to retry if the connection
is lost during initialisation, so factor it out to keep the following
patch simpler.

The error path for vhost_dev_get_config() forgot disconnecting the
chardev, add this while touching the code.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/block/vhost-user-blk.c | 48 ++++++++++++++++++++++++++-------------
 1 file changed, 32 insertions(+), 16 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 3770f715da..e49d2e4c83 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -423,6 +423,36 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
     }
 }
 
+static int vhost_user_blk_realize_connect(VHostUserBlk *s, Error **errp)
+{
+    DeviceState *dev = &s->parent_obj.parent_obj;
+    int ret;
+
+    s->connected = false;
+
+    ret = qemu_chr_fe_wait_connected(&s->chardev, errp);
+    if (ret < 0) {
+        return ret;
+    }
+
+    ret = vhost_user_blk_connect(dev, errp);
+    if (ret < 0) {
+        qemu_chr_fe_disconnect(&s->chardev);
+        return ret;
+    }
+    assert(s->connected);
+
+    ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
+                               sizeof(struct virtio_blk_config), errp);
+    if (ret < 0) {
+        qemu_chr_fe_disconnect(&s->chardev);
+        vhost_dev_cleanup(&s->dev);
+        return ret;
+    }
+
+    return 0;
+}
+
 static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -467,22 +497,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
 
     s->inflight = g_new0(struct vhost_inflight, 1);
     s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues);
-    s->connected = false;
-
-    if (qemu_chr_fe_wait_connected(&s->chardev, errp) < 0) {
-        goto virtio_err;
-    }
 
-    if (vhost_user_blk_connect(dev, errp) < 0) {
-        qemu_chr_fe_disconnect(&s->chardev);
-        goto virtio_err;
-    }
-    assert(s->connected);
-
-    ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
-                               sizeof(struct virtio_blk_config), errp);
+    ret = vhost_user_blk_realize_connect(s, errp);
     if (ret < 0) {
-        goto vhost_err;
+        goto virtio_err;
     }
 
     /* we're fully initialized, now we can operate, so add the handler */
@@ -491,8 +509,6 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
                              NULL, true);
     return;
 
-vhost_err:
-    vhost_dev_cleanup(&s->dev);
 virtio_err:
     g_free(s->vhost_vqs);
     s->vhost_vqs = NULL;
-- 
2.30.2


Re: [PATCH 6/7] vhost-user-blk: Factor out vhost_user_blk_realize_connect()
Posted by Stefano Garzarella 4 years ago
On Wed, Jun 09, 2021 at 05:46:57PM +0200, Kevin Wolf wrote:
>This function is the part that we will want to retry if the connection
>is lost during initialisation, so factor it out to keep the following
>patch simpler.
>
>The error path for vhost_dev_get_config() forgot disconnecting the
>chardev, add this while touching the code.
>
>Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>---
> hw/block/vhost-user-blk.c | 48 ++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 16 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


Re: [PATCH 6/7] vhost-user-blk: Factor out vhost_user_blk_realize_connect()
Posted by Raphael Norwitz 4 years ago
On Wed, Jun 09, 2021 at 05:46:57PM +0200, Kevin Wolf wrote:
> This function is the part that we will want to retry if the connection
> is lost during initialisation, so factor it out to keep the following
> patch simpler.
> 
> The error path for vhost_dev_get_config() forgot disconnecting the
> chardev, add this while touching the code.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>

> ---
>  hw/block/vhost-user-blk.c | 48 ++++++++++++++++++++++++++-------------
>  1 file changed, 32 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 3770f715da..e49d2e4c83 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -423,6 +423,36 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event)
>      }
>  }
>  
> +static int vhost_user_blk_realize_connect(VHostUserBlk *s, Error **errp)
> +{
> +    DeviceState *dev = &s->parent_obj.parent_obj;
> +    int ret;
> +
> +    s->connected = false;
> +
> +    ret = qemu_chr_fe_wait_connected(&s->chardev, errp);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    ret = vhost_user_blk_connect(dev, errp);
> +    if (ret < 0) {
> +        qemu_chr_fe_disconnect(&s->chardev);
> +        return ret;
> +    }
> +    assert(s->connected);
> +
> +    ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
> +                               sizeof(struct virtio_blk_config), errp);
> +    if (ret < 0) {
> +        qemu_chr_fe_disconnect(&s->chardev);
> +        vhost_dev_cleanup(&s->dev);
> +        return ret;
> +    }
> +
> +    return 0;
> +}
> +
>  static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> @@ -467,22 +497,10 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>  
>      s->inflight = g_new0(struct vhost_inflight, 1);
>      s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues);
> -    s->connected = false;
> -
> -    if (qemu_chr_fe_wait_connected(&s->chardev, errp) < 0) {
> -        goto virtio_err;
> -    }
>  
> -    if (vhost_user_blk_connect(dev, errp) < 0) {
> -        qemu_chr_fe_disconnect(&s->chardev);
> -        goto virtio_err;
> -    }
> -    assert(s->connected);
> -
> -    ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
> -                               sizeof(struct virtio_blk_config), errp);
> +    ret = vhost_user_blk_realize_connect(s, errp);
>      if (ret < 0) {
> -        goto vhost_err;
> +        goto virtio_err;
>      }
>  
>      /* we're fully initialized, now we can operate, so add the handler */
> @@ -491,8 +509,6 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>                               NULL, true);
>      return;
>  
> -vhost_err:
> -    vhost_dev_cleanup(&s->dev);
>  virtio_err:
>      g_free(s->vhost_vqs);
>      s->vhost_vqs = NULL;
> -- 
> 2.30.2
>