| Message ID | 20230731121018.2856310-6-fengli@smartx.com |
|---|---|
| State | New |
| Headers | show |
| Series | Implement reconnect for vhost-user-scsi | expand |
> 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 >
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;
> 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;
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;
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(-)