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

[Manos Pitsidianakis]

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;
}