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

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

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