Re: [Qemu-devel] [PATCH v1 2/2] virtio-scsi: dataplane: notify guest as batch

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

On Tue, Nov 11, 2014 at 09:17:10AM +0800, Ming Lei wrote:
> +static void notify_guest_bh(void *opaque)
> +{
> +    VirtIOSCSI *s = opaque;
> +    unsigned int qid;
> +    uint64_t pending = s->pending_guest_notify;
> +
> +    s->pending_guest_notify = 0;
> +
> +    while ((qid = ffsl(pending))) {
> +        qid--;
> +        event_notifier_set(&s->cmd_vrings[qid]->guest_notifier);
> +        pending &= ~(1 << qid);
> +    }
> +}

Looks like we're not honoring virtio's usual interrupt mitigation
mechanism (e.g. EVENT_IDX) for virtio-scsi.  Why is
vring_should_notify() not used?

> +
>  static void virtio_scsi_bad_req(void)
>  {
>      error_report("wrong size for virtio-scsi headers");
> @@ -824,7 +839,12 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
>      }
>  
>      if (s->conf.iothread) {
> -        virtio_scsi_set_iothread(VIRTIO_SCSI(s), s->conf.iothread);
> +        VirtIOSCSI *vis = VIRTIO_SCSI(s);
> +
> +        QEMU_BUILD_BUG_ON(VIRTIO_PCI_QUEUE_MAX > 64);
> +        virtio_scsi_set_iothread(vis, s->conf.iothread);
> +        vis->pending_guest_notify = 0;
> +        vis->guest_notify_bh = aio_bh_new(vis->ctx, notify_guest_bh, vis);

Have you checked state transitions like PCI or virtio reset?  They need
to cancel the BH and clear ->pending_guest_notify.

There is also a transition from dataplane to non-dataplane mode for live
migration.

Please make sure no interrupts are dropped or stale data is kept across
these transitions.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]