Message ID | 1344846917-7411-7-git-send-email-nab@linux-iscsi.org |
---|---|
State | New |
Headers | show |
On Mon, 2012-08-13 at 12:02 +0300, Michael S. Tsirkin wrote: > On Mon, Aug 13, 2012 at 08:35:17AM +0000, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger <nab@linux-iscsi.org> > > > > This patch fixes bug in the definition of VirtIOSCSI->cmd_vqs[0], > > where the return of virtio_add_queue() in virtio_scsi_init() ends up > > overwriting past the end of ->cmd_vqs[0]. > > > > Since virtio_scsi currently assumes a single vqs for data, this patch > > simply changes ->cmd_vqs[1] to handle the single VirtQueue. > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > > Cc: Michael S. Tsirkin <mst@redhat.com> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> > > This is a bugfix we need even without vhost, right? > I believe so, as it appears to be stomping past the end of memory for every virtio-scsi initialization regardless of vhost usage.. Paolo, can you pickup this fix now for stable so it can be dropped from RFC-v3..? --nab > > --- > > hw/virtio-scsi.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c > > index 5e2ff6b..2c70f89 100644 > > --- a/hw/virtio-scsi.c > > +++ b/hw/virtio-scsi.c > > @@ -150,7 +150,7 @@ typedef struct { > > bool events_dropped; > > VirtQueue *ctrl_vq; > > VirtQueue *event_vq; > > - VirtQueue *cmd_vqs[0]; > > + VirtQueue *cmd_vqs[1]; > > > > bool vhost_started; > > VHostSCSI *vhost_scsi; > > -- > > 1.7.2.5 > -- > To unsubscribe from this list: send the line "unsubscribe target-devel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 14/08/2012 22:20, Nicholas A. Bellinger ha scritto: >>> > > Since virtio_scsi currently assumes a single vqs for data, this patch >>> > > simply changes ->cmd_vqs[1] to handle the single VirtQueue. Wrong, multiqueue works just fine. :) It's just the kernel driver that doesn't support it yet. >>> > > Cc: Paolo Bonzini <pbonzini@redhat.com> >>> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> >>> > > Cc: Michael S. Tsirkin <mst@redhat.com> >>> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> >> > >> > This is a bugfix we need even without vhost, right? >> > > I believe so, as it appears to be stomping past the end of memory for > every virtio-scsi initialization regardless of vhost usage.. You just did a wrong merge. When commit d2ad7dd (virtio-scsi: add multiqueue capability, 2012-04-06) changed cmd_vq from pointer to array of pointers, you should have moved the following fields to the middle of the struct, just like that commit did. Paolo
On Sat, 2012-08-18 at 20:52 +0200, Paolo Bonzini wrote: > Il 14/08/2012 22:20, Nicholas A. Bellinger ha scritto: > >>> > > Since virtio_scsi currently assumes a single vqs for data, this patch > >>> > > simply changes ->cmd_vqs[1] to handle the single VirtQueue. > > Wrong, multiqueue works just fine. :) It's just the kernel driver that > doesn't support it yet. > <nod> > >>> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > >>> > > Cc: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > >>> > > Cc: Michael S. Tsirkin <mst@redhat.com> > >>> > > Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org> > >> > > >> > This is a bugfix we need even without vhost, right? > >> > > > I believe so, as it appears to be stomping past the end of memory for > > every virtio-scsi initialization regardless of vhost usage.. > > You just did a wrong merge. When commit d2ad7dd (virtio-scsi: add > multiqueue capability, 2012-04-06) changed cmd_vq from pointer to array > of pointers, you should have moved the following fields to the middle of > the struct, just like that commit did. Ahh, I see how virtio_scsi_init() -> virtio_common_init() are setting up the memory now.. Apologies, my mistake. So moving the vhost-scsi related structure members ahead of the VirtQueue releated definitions for RFC-v3, and dropping this patch. Thanks Paolo! --nab
diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 5e2ff6b..2c70f89 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -150,7 +150,7 @@ typedef struct { bool events_dropped; VirtQueue *ctrl_vq; VirtQueue *event_vq; - VirtQueue *cmd_vqs[0]; + VirtQueue *cmd_vqs[1]; bool vhost_started; VHostSCSI *vhost_scsi;