Thanks for your comments, I will submit the v8.
> On 8 Oct 2023, at 6:46 PM, Manos Pitsidianakis
> <manos.pitsidiana...@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 <fen...@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;
> }