Re: [PATCH v7 3/5] vhost-user-scsi: support reconnect to backend

[Li Feng]

Thanks for your comments, I will submit the v8.

> On 8 Oct 2023, at 6:46 PM, Manos Pitsidianakis 
> <manos.pitsidianakis@linaro.org> wrote:
> 
> Hello Li, I have some trivial style comments you could possibly address in a 
> next version:
> 
> On Sun, 08 Oct 2023 12:12, Li Feng <fengli@smartx.com> wrote:
>> diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
>> index df6b66cc1a..5df24faff4 100644
>> --- a/hw/scsi/vhost-user-scsi.c
>> +++ b/hw/scsi/vhost-user-scsi.c
>> @@ -39,26 +39,56 @@ static const int user_feature_bits[] = {
>>    VHOST_INVALID_FEATURE_BIT
>> };
>> +static int vhost_user_scsi_start(VHostUserSCSI *s, Error **errp)
>> +{
>> +    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
>> +    int ret;
>> +
>> +    ret = vhost_scsi_common_start(vsc, errp);
>> +    s->started_vu = (ret < 0 ? false : true);
> 
> -+    s->started_vu = (ret < 0 ? false : true);
> ++    s->started_vu = !(ret < 0);
> 
>> static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
>> {
>>    VHostUserSCSI *s = (VHostUserSCSI *)vdev;
>> +    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
> 
> -+    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
> ++    DeviceState *dev = DEVICE(vdev);
> 
>> +static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
>> +{
>> +    VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>> +    VHostUserSCSI *s = VHOST_USER_SCSI(vdev);
>> +    VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
>> +    VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
>> +    int ret = 0;
>> +
>> +    if (s->connected) {
>> +        return 0;
>> +    }
>> +    s->connected = true;
>> +
>> +    vsc->dev.num_queues = vs->conf.num_queues;
>> +    vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
>> +    vsc->dev.vqs = s->vhost_vqs;
>> +    vsc->dev.vq_index = 0;
>> +    vsc->dev.backend_features = 0;
>> +
>> +    ret = vhost_dev_init(&vsc->dev, &s->vhost_user, 
>> VHOST_BACKEND_TYPE_USER, 0,
>> +                         errp);
>> +    if (ret < 0) {
>> +        return ret;
>> +    }
>> +
>> +    /* restore vhost state */
>> +    if (virtio_device_started(vdev, vdev->status)) {
>> +        ret = vhost_user_scsi_start(s, errp);
>> +        if (ret < 0) {
>> +            return ret;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
> 
> 
> -+    if (virtio_device_started(vdev, vdev->status)) {
> -+        ret = vhost_user_scsi_start(s, errp);
> -+        if (ret < 0) {
> -+            return ret;
> -+        }
> -+    }
> -+
> -+    return 0;
> -+}
> ++    if (virtio_device_started(vdev, vdev->status)) {
> ++        ret = vhost_user_scsi_start(s, errp);
> ++    }
> ++
> ++    return ret;
> ++}
> 
> [skipping..]
> 
>> +static int vhost_user_scsi_realize_connect(VHostUserSCSI *s, Error **errp)
>> +{
>> +    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
> 
> 
> -+    DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
> ++    DeviceState *dev = DEVICE(s);
> 
>> diff --git a/include/hw/virtio/vhost-user-scsi.h 
>> b/include/hw/virtio/vhost-user-scsi.h
>> index 521b08e559..b405ec952a 100644
>> --- a/include/hw/virtio/vhost-user-scsi.h
>> +++ b/include/hw/virtio/vhost-user-scsi.h
>> @@ -29,6 +29,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(VHostUserSCSI, VHOST_USER_SCSI)
>> struct VHostUserSCSI {
>>    VHostSCSICommon parent_obj;
>>    VhostUserState vhost_user;
>> +    bool connected;
>> +    bool started_vu;
>> +
>> +    struct vhost_virtqueue *vhost_vqs;
> 
> +    bool connected;
> +    bool started_vu;
> -+
> +    struct vhost_virtqueue *vhost_vqs;
> 
> See 
> https://www.qemu.org/docs/master/devel/style.html#qemu-object-model-declarations
> 
> The definition should look like:
> 
> struct VHostUserSCSI {
>   VHostSCSICommon parent_obj;
> 
>   /* Properties */
>   bool connected;
>   bool started_vu;
> 
>   VhostUserState vhost_user;
>   struct vhost_virtqueue *vhost_vqs;
> }