[Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Ming Lei]

In case of vhost-scsi, the object type of VirtIODevice isn't
VirtIOSCSI, so use the cast trick to fix the problem like
in virtio_scsi_handle_cmd().

Also 'events_dropped' need to be accessed in the path, so
move it into VirtIOSCSICommon.

Cc: qemu-stable@nongnu.org
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 hw/scsi/virtio-scsi.c           |   14 ++++++++------
 include/hw/virtio/virtio-scsi.h |    3 ++-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index b0d7517..081c226 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -461,7 +461,7 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
 
     vs->sense_size = VIRTIO_SCSI_SENSE_SIZE;
     vs->cdb_size = VIRTIO_SCSI_CDB_SIZE;
-    s->events_dropped = false;
+    vs->events_dropped = false;
 }
 
 /* The device does not have anything to save beyond the virtio data.
@@ -499,7 +499,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
     }
 
     if (!req) {
-        s->events_dropped = true;
+        vs->events_dropped = true;
         return;
     }
 
@@ -507,9 +507,9 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
         virtio_scsi_bad_req();
     }
 
-    if (s->events_dropped) {
+    if (vs->events_dropped) {
         event |= VIRTIO_SCSI_T_EVENTS_MISSED;
-        s->events_dropped = false;
+        vs->events_dropped = false;
     }
 
     in_size = req->elem.in_sg[0].iov_len;
@@ -538,9 +538,11 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
 
 static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
 {
-    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+    /* use non-QOM casts in the data path */
+    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
+    VirtIOSCSICommon *vs = &s->parent_obj;
 
-    if (s->events_dropped) {
+    if (vs->events_dropped) {
         virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
     }
 }
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 367afc6..e60a44a 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -164,6 +164,8 @@ typedef struct VirtIOSCSICommon {
     VirtQueue *ctrl_vq;
     VirtQueue *event_vq;
     VirtQueue **cmd_vqs;
+
+    bool events_dropped;
 } VirtIOSCSICommon;
 
 typedef struct {
@@ -171,7 +173,6 @@ typedef struct {
 
     SCSIBus bus;
     int resetting;
-    bool events_dropped;
 } VirtIOSCSI;
 
 #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field)                     \
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Michael S. Tsirkin]

On Wed, Jun 18, 2014 at 11:32:33PM +0800, Ming Lei wrote:
> In case of vhost-scsi, the object type of VirtIODevice isn't
> VirtIOSCSI, so use the cast trick to fix the problem like
> in virtio_scsi_handle_cmd().
> 
> Also 'events_dropped' need to be accessed in the path,

in which path?

> so
> move it into VirtIOSCSICommon.
> 
> Cc: qemu-stable@nongnu.org
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  hw/scsi/virtio-scsi.c           |   14 ++++++++------
>  include/hw/virtio/virtio-scsi.h |    3 ++-
>  2 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index b0d7517..081c226 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -461,7 +461,7 @@ static void virtio_scsi_reset(VirtIODevice *vdev)
>  
>      vs->sense_size = VIRTIO_SCSI_SENSE_SIZE;
>      vs->cdb_size = VIRTIO_SCSI_CDB_SIZE;
> -    s->events_dropped = false;
> +    vs->events_dropped = false;
>  }
>  
>  /* The device does not have anything to save beyond the virtio data.
> @@ -499,7 +499,7 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
>      }
>  
>      if (!req) {
> -        s->events_dropped = true;
> +        vs->events_dropped = true;
>          return;
>      }
>  
> @@ -507,9 +507,9 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
>          virtio_scsi_bad_req();
>      }
>  
> -    if (s->events_dropped) {
> +    if (vs->events_dropped) {
>          event |= VIRTIO_SCSI_T_EVENTS_MISSED;
> -        s->events_dropped = false;
> +        vs->events_dropped = false;
>      }
>  
>      in_size = req->elem.in_sg[0].iov_len;
> @@ -538,9 +538,11 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
>  
>  static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq)
>  {
> -    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
> +    /* use non-QOM casts in the data path */
> +    VirtIOSCSI *s = (VirtIOSCSI *)vdev;
> +    VirtIOSCSICommon *vs = &s->parent_obj;
>  
> -    if (s->events_dropped) {
> +    if (vs->events_dropped) {
>          virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0);
>      }
>  }
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index 367afc6..e60a44a 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -164,6 +164,8 @@ typedef struct VirtIOSCSICommon {
>      VirtQueue *ctrl_vq;
>      VirtQueue *event_vq;
>      VirtQueue **cmd_vqs;
> +
> +    bool events_dropped;
>  } VirtIOSCSICommon;
>  
>  typedef struct {
> @@ -171,7 +173,6 @@ typedef struct {
>  
>      SCSIBus bus;
>      int resetting;
> -    bool events_dropped;
>  } VirtIOSCSI;
>  
>  #define DEFINE_VIRTIO_SCSI_PROPERTIES(_state, _conf_field)                     \
> -- 
> 1.7.9.5

Re: [Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Ming Lei]

On Wed, Jun 18, 2014 at 11:43 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Jun 18, 2014 at 11:32:33PM +0800, Ming Lei wrote:
>> In case of vhost-scsi, the object type of VirtIODevice isn't
>> VirtIOSCSI, so use the cast trick to fix the problem like
>> in virtio_scsi_handle_cmd().
>>
>> Also 'events_dropped' need to be accessed in the path,
>
> in which path?

I mean vq's notify path, virtio_scsi_handle_event().

Thanks,
--
Ming Lei

Re: [Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Paolo Bonzini]

Il 18/06/2014 17:48, Ming Lei ha scritto:
> On Wed, Jun 18, 2014 at 11:43 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Wed, Jun 18, 2014 at 11:32:33PM +0800, Ming Lei wrote:
>>> In case of vhost-scsi, the object type of VirtIODevice isn't
>>> VirtIOSCSI, so use the cast trick to fix the problem like
>>> in virtio_scsi_handle_cmd().
>>>
>>> Also 'events_dropped' need to be accessed in the path,
>>
>> in which path?
>
> I mean vq's notify path, virtio_scsi_handle_event().

A better way is this:

- create a VirtIOSCSICommonClass struct and use it (virtio_scsi_common_info)

- add three members virtio_scsi_handle_ctrl, virtio_scsi_handle_event, 
virtio_scsi_handle_cmd to VirtIOSCSICommonClass

- initialize them to the existing functions in virtio_scsi_class_init

- initialize them to dummy empty functions in vhost_scsi_class_init

Which I should have done in the first place when rebasing vhost-scsi, 
and I didn't only because I was lazy and thought this could not happen. 
:(  Turns out it only didn't because at the time the kernel did not 
support events.

Paolo

Re: [Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Ming Lei]

On Wed, Jun 18, 2014 at 11:53 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 18/06/2014 17:48, Ming Lei ha scritto:
>
>> On Wed, Jun 18, 2014 at 11:43 PM, Michael S. Tsirkin <mst@redhat.com>
>> wrote:
>>>
>>> On Wed, Jun 18, 2014 at 11:32:33PM +0800, Ming Lei wrote:
>>>>
>>>> In case of vhost-scsi, the object type of VirtIODevice isn't
>>>> VirtIOSCSI, so use the cast trick to fix the problem like
>>>> in virtio_scsi_handle_cmd().
>>>>
>>>> Also 'events_dropped' need to be accessed in the path,
>>>
>>>
>>> in which path?
>>
>>
>> I mean vq's notify path, virtio_scsi_handle_event().
>
>
> A better way is this:
>
> - create a VirtIOSCSICommonClass struct and use it (virtio_scsi_common_info)
>
> - add three members virtio_scsi_handle_ctrl, virtio_scsi_handle_event,
> virtio_scsi_handle_cmd to VirtIOSCSICommonClass
>
> - initialize them to the existing functions in virtio_scsi_class_init
>
> - initialize them to dummy empty functions in vhost_scsi_class_init

But vhost does need the three functions as vq's handle_out callback.

Thanks,
--
Ming Lei

Re: [Qemu-devel] [PATCH v1] virtio-scsi: fix object check failure

[Paolo Bonzini]

Il 18/06/2014 18:03, Ming Lei ha scritto:
>> >
>> > - initialize them to the existing functions in virtio_scsi_class_init
>> >
>> > - initialize them to dummy empty functions in vhost_scsi_class_init
> But vhost does need the three functions as vq's handle_out callback.

Yes, but they do not need to do anything, the kernel handles everything. 
  They just cannot be NULL.

Paolo