From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59897) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxXV9-0008Hn-UF for qemu-devel@nongnu.org; Thu, 19 Jun 2014 04:15:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxXV5-0007u5-Ca for qemu-devel@nongnu.org; Thu, 19 Jun 2014 04:15:19 -0400 Message-ID: <53A29C07.7080908@redhat.com> Date: Thu, 19 Jun 2014 10:15:03 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1403165520-20897-1-git-send-email-ming.lei@canonical.com> In-Reply-To: <1403165520-20897-1-git-send-email-ming.lei@canonical.com> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] virtio-scsi: define dummy handle_output for vhost-scsi vqs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Ming Lei , Peter Maydell , qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org, Anthony Liguori , "Michael S. Tsirkin" Il 19/06/2014 10:12, Ming Lei ha scritto: > vhost userspace needn't to handle vq's notification from guest, > so define dummy handle_output callback for all vqs of vhost-scsi. > > In some corner cases(such as when handling vq's reset from VM), virtio-pci > still trys to handle pending virtio-scsi events, then object check failure > inside virtio_scsi_handle_event() for vhost-scsi can be triggered. > > The issue can be reproduced by 'rmmod virtio-scsi', 'system sleep' or reboot > inside VM. > > Cc: qemu-stable@nongnu.org > Cc: Anthony Liguori > Cc: "Michael S. Tsirkin" > Cc: Paolo Bonzini > Signed-off-by: Ming Lei > --- > hw/scsi/vhost-scsi.c | 8 +++++++- > hw/scsi/virtio-scsi.c | 14 +++++++++----- > include/hw/virtio/virtio-scsi.h | 7 ++++++- > 3 files changed, 22 insertions(+), 7 deletions(-) > > diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c > index 3983a5b..9099192 100644 > --- a/hw/scsi/vhost-scsi.c > +++ b/hw/scsi/vhost-scsi.c > @@ -196,6 +196,10 @@ static void vhost_scsi_set_status(VirtIODevice *vdev, uint8_t val) > } > } > > +static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) > +{ > +} > + > static void vhost_scsi_realize(DeviceState *dev, Error **errp) > { > VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev); > @@ -217,7 +221,9 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) > } > } > > - virtio_scsi_common_realize(dev, &err); > + virtio_scsi_common_realize(dev, &err, vhost_dummy_handle_output, > + vhost_dummy_handle_output, > + vhost_dummy_handle_output); > if (err != NULL) { > error_propagate(errp, err); > return; > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index b0d7517..c57cefb 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -595,7 +595,9 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { > .load_request = virtio_scsi_load_request, > }; > > -void virtio_scsi_common_realize(DeviceState *dev, Error **errp) > +void virtio_scsi_common_realize(DeviceState *dev, Error **errp, > + HandleOutput ctrl, HandleOutput evt, > + HandleOutput cmd) > { > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(dev); > @@ -609,12 +611,12 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp) > s->cdb_size = VIRTIO_SCSI_CDB_SIZE; > > s->ctrl_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, > - virtio_scsi_handle_ctrl); > + ctrl); > s->event_vq = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, > - virtio_scsi_handle_event); > + evt); > for (i = 0; i < s->conf.num_queues; i++) { > s->cmd_vqs[i] = virtio_add_queue(vdev, VIRTIO_SCSI_VQ_SIZE, > - virtio_scsi_handle_cmd); > + cmd); > } > } > > @@ -625,7 +627,9 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp) > static int virtio_scsi_id; > Error *err = NULL; > > - virtio_scsi_common_realize(dev, &err); > + virtio_scsi_common_realize(dev, &err, virtio_scsi_handle_ctrl, > + virtio_scsi_handle_event, > + virtio_scsi_handle_cmd); > if (err != NULL) { > error_propagate(errp, err); > return; > diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h > index 367afc6..a54b973 100644 > --- a/include/hw/virtio/virtio-scsi.h > +++ b/include/hw/virtio/virtio-scsi.h > @@ -185,7 +185,12 @@ typedef struct { > DEFINE_PROP_BIT("param_change", _state, _feature_field, \ > VIRTIO_SCSI_F_CHANGE, true) > > -void virtio_scsi_common_realize(DeviceState *dev, Error **errp); > +typedef void (*HandleOutput)(VirtIODevice *, VirtQueue *); > + > +void virtio_scsi_common_realize(DeviceState *dev, Error **errp, > + HandleOutput ctrl, HandleOutput evt, > + HandleOutput cmd); > + > void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp); > > #endif /* _QEMU_VIRTIO_SCSI_H */ > Thanks, applied to scsi-next branch. Paolo