1. Patchew
  2. QEMU
  3. [PATCH v3 0/5] Implement reconnect for vhost-user-scsi
  4. View patch

[PATCH v3 5/5] vhost-user-scsi: start vhost when guest kicks

Li Feng posted 5 patches 2 years, 6 months ago
Maintainers: Raphael Norwitz <raphael.norwitz@nutanix.com>, "Michael S. Tsirkin" <mst@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, "Alex Bennée" <alex.bennee@linaro.org>, Viresh Kumar <viresh.kumar@linaro.org>
There is a newer version of this series
[PATCH v3 5/5] vhost-user-scsi: start vhost when guest kicks
Posted by Li Feng 2 years, 6 months ago 
Let's keep the same behavior as vhost-user-blk.

Some old guests kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK.

Signed-off-by: Li Feng <fengli@smartx.com>
---
 hw/scsi/vhost-user-scsi.c | 48 +++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 5bf012461b..a7fa8e8df2 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -113,8 +113,48 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
     }
 }
 
-static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+static void vhost_user_scsi_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
+    VHostUserSCSI *s = (VHostUserSCSI *)vdev;
+    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
+    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
+    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+
+    Error *local_err = NULL;
+    int i, ret;
+
+    if (!vdev->start_on_kick) {
+        return;
+    }
+
+    if (!s->connected) {
+        return;
+    }
+
+    if (vhost_dev_is_started(&vsc->dev)) {
+        return;
+    }
+
+    /*
+     * Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
+     * vhost here instead of waiting for .set_status().
+     */
+    ret = vhost_user_scsi_start(s);
+    if (ret < 0) {
+        error_reportf_err(local_err, "vhost-user-scsi: vhost start failed: ");
+        qemu_chr_fe_disconnect(&vs->conf.chardev);
+        return;
+    }
+
+    /* Kick right away to begin processing requests already in vring */
+    for (i = 0; i < vsc->dev.nvqs; i++) {
+        VirtQueue *kick_vq = virtio_get_queue(vdev, i);
+
+        if (!virtio_queue_get_desc_addr(vdev, i)) {
+            continue;
+        }
+        event_notifier_set(virtio_queue_get_host_notifier(kick_vq));
+    }
 }
 
 static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
@@ -243,9 +283,9 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    virtio_scsi_common_realize(dev, vhost_dummy_handle_output,
-                               vhost_dummy_handle_output,
-                               vhost_dummy_handle_output, &err);
+    virtio_scsi_common_realize(dev, vhost_user_scsi_handle_output,
+                               vhost_user_scsi_handle_output,
+                               vhost_user_scsi_handle_output, &err);
     if (err != NULL) {
         error_propagate(errp, err);
         return;
-- 
2.41.0
Re: [PATCH v3 5/5] vhost-user-scsi: start vhost when guest kicks
Posted by Markus Armbruster 2 years, 5 months ago 
Li Feng <fengli@smartx.com> writes:

> Let's keep the same behavior as vhost-user-blk.
>
> Some old guests kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK.
>
> Signed-off-by: Li Feng <fengli@smartx.com>
> ---
>  hw/scsi/vhost-user-scsi.c | 48 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 5bf012461b..a7fa8e8df2 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -113,8 +113,48 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
>      }
>  }
>  
> -static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> +static void vhost_user_scsi_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>  {
> +    VHostUserSCSI *s = (VHostUserSCSI *)vdev;
> +    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
> +    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
> +
> +    Error *local_err = NULL;
> +    int i, ret;
> +
> +    if (!vdev->start_on_kick) {
> +        return;
> +    }
> +
> +    if (!s->connected) {
> +        return;
> +    }
> +
> +    if (vhost_dev_is_started(&vsc->dev)) {
> +        return;
> +    }
> +
> +    /*
> +     * Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> +     * vhost here instead of waiting for .set_status().
> +     */
> +    ret = vhost_user_scsi_start(s);
> +    if (ret < 0) {
> +        error_reportf_err(local_err, "vhost-user-scsi: vhost start failed: ");

Crashes, since @local_err is null.  Please test your error paths.

Obvious fix: drop this call.

> +        qemu_chr_fe_disconnect(&vs->conf.chardev);
> +        return;
> +    }
> +
> +    /* Kick right away to begin processing requests already in vring */
> +    for (i = 0; i < vsc->dev.nvqs; i++) {
> +        VirtQueue *kick_vq = virtio_get_queue(vdev, i);
> +
> +        if (!virtio_queue_get_desc_addr(vdev, i)) {
> +            continue;
> +        }
> +        event_notifier_set(virtio_queue_get_host_notifier(kick_vq));
> +    }
>  }
>  
>  static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
> @@ -243,9 +283,9 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> -    virtio_scsi_common_realize(dev, vhost_dummy_handle_output,
> -                               vhost_dummy_handle_output,
> -                               vhost_dummy_handle_output, &err);
> +    virtio_scsi_common_realize(dev, vhost_user_scsi_handle_output,
> +                               vhost_user_scsi_handle_output,
> +                               vhost_user_scsi_handle_output, &err);
>      if (err != NULL) {
>          error_propagate(errp, err);
>          return;
Re: [PATCH v3 5/5] vhost-user-scsi: start vhost when guest kicks
Posted by Li Feng 2 years, 5 months ago 

> On 1 Sep 2023, at 7:44 PM, Markus Armbruster <armbru@redhat.com> wrote:
> 
> Li Feng <fengli@smartx.com <mailto:fengli@smartx.com>> writes:
> 
>> Let's keep the same behavior as vhost-user-blk.
>> 
>> Some old guests kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK.
>> 
>> Signed-off-by: Li Feng <fengli@smartx.com>
>> ---
>> hw/scsi/vhost-user-scsi.c | 48 +++++++++++++++++++++++++++++++++++----
>> 1 file changed, 44 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
>> index 5bf012461b..a7fa8e8df2 100644
>> --- a/hw/scsi/vhost-user-scsi.c
>> +++ b/hw/scsi/vhost-user-scsi.c
>> @@ -113,8 +113,48 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
>>     }
>> }
>> 
>> -static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>> +static void vhost_user_scsi_handle_output(VirtIODevice *vdev, VirtQueue *vq)
>> {
>> +    VHostUserSCSI *s = (VHostUserSCSI *)vdev;
>> +    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
>> +    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
>> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
>> +
>> +    Error *local_err = NULL;
>> +    int i, ret;
>> +
>> +    if (!vdev->start_on_kick) {
>> +        return;
>> +    }
>> +
>> +    if (!s->connected) {
>> +        return;
>> +    }
>> +
>> +    if (vhost_dev_is_started(&vsc->dev)) {
>> +        return;
>> +    }
>> +
>> +    /*
>> +     * Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
>> +     * vhost here instead of waiting for .set_status().
>> +     */
>> +    ret = vhost_user_scsi_start(s);
>> +    if (ret < 0) {
>> +        error_reportf_err(local_err, "vhost-user-scsi: vhost start failed: ");
> 
> Crashes, since @local_err is null.  Please test your error paths.
> 
> Obvious fix: drop this call.
Emmm, actually I have tested the error path, so I find some issues that I have fixed
in the following patches.
I will merge the later series into this series.


> 
>> +        qemu_chr_fe_disconnect(&vs->conf.chardev);
>> +        return;
>> +    }
>> +
>> +    /* Kick right away to begin processing requests already in vring */
>> +    for (i = 0; i < vsc->dev.nvqs; i++) {
>> +        VirtQueue *kick_vq = virtio_get_queue(vdev, i);
>> +
>> +        if (!virtio_queue_get_desc_addr(vdev, i)) {
>> +            continue;
>> +        }
>> +        event_notifier_set(virtio_queue_get_host_notifier(kick_vq));
>> +    }
>> }
>> 
>> static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
>> @@ -243,9 +283,9 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
>>         return;
>>     }
>> 
>> -    virtio_scsi_common_realize(dev, vhost_dummy_handle_output,
>> -                               vhost_dummy_handle_output,
>> -                               vhost_dummy_handle_output, &err);
>> +    virtio_scsi_common_realize(dev, vhost_user_scsi_handle_output,
>> +                               vhost_user_scsi_handle_output,
>> +                               vhost_user_scsi_handle_output, &err);
>>     if (err != NULL) {
>>         error_propagate(errp, err);
>>         return;
Re: [PATCH v3 5/5] vhost-user-scsi: start vhost when guest kicks
Posted by Raphael Norwitz 2 years, 6 months ago 

> On Jul 31, 2023, at 8:10 AM, Li Feng <fengli@smartx.com> wrote:
> 
> Let's keep the same behavior as vhost-user-blk.
> 
> Some old guests kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK.
> 
> Signed-off-by: Li Feng <fengli@smartx.com>

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

> ---
> hw/scsi/vhost-user-scsi.c | 48 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
> index 5bf012461b..a7fa8e8df2 100644
> --- a/hw/scsi/vhost-user-scsi.c
> +++ b/hw/scsi/vhost-user-scsi.c
> @@ -113,8 +113,48 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
>     }
> }
> 
> -static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> +static void vhost_user_scsi_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> {
> +    VHostUserSCSI *s = (VHostUserSCSI *)vdev;
> +    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
> +    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
> +
> +    Error *local_err = NULL;
> +    int i, ret;
> +
> +    if (!vdev->start_on_kick) {
> +        return;
> +    }
> +
> +    if (!s->connected) {
> +        return;
> +    }
> +
> +    if (vhost_dev_is_started(&vsc->dev)) {
> +        return;
> +    }
> +
> +    /*
> +     * Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> +     * vhost here instead of waiting for .set_status().
> +     */
> +    ret = vhost_user_scsi_start(s);
> +    if (ret < 0) {
> +        error_reportf_err(local_err, "vhost-user-scsi: vhost start failed: ");
> +        qemu_chr_fe_disconnect(&vs->conf.chardev);
> +        return;
> +    }
> +
> +    /* Kick right away to begin processing requests already in vring */
> +    for (i = 0; i < vsc->dev.nvqs; i++) {
> +        VirtQueue *kick_vq = virtio_get_queue(vdev, i);
> +
> +        if (!virtio_queue_get_desc_addr(vdev, i)) {
> +            continue;
> +        }
> +        event_notifier_set(virtio_queue_get_host_notifier(kick_vq));
> +    }
> }
> 
> static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
> @@ -243,9 +283,9 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
>         return;
>     }
> 
> -    virtio_scsi_common_realize(dev, vhost_dummy_handle_output,
> -                               vhost_dummy_handle_output,
> -                               vhost_dummy_handle_output, &err);
> +    virtio_scsi_common_realize(dev, vhost_user_scsi_handle_output,
> +                               vhost_user_scsi_handle_output,
> +                               vhost_user_scsi_handle_output, &err);
>     if (err != NULL) {
>         error_propagate(errp, err);
>         return;
> -- 
> 2.41.0
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.