| Message ID | 20200416203624.32366-5-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC/WIP: Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread | expand |
On 16/04/20 22:36, Maxim Levitsky wrote: > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399 > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > hw/scsi/virtio-scsi.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index b0f4a35f81..e360b4e03e 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun) > > static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) > { > + SCSIDevice *device = NULL; > + > if (lun[0] != 1) { > return NULL; > } > if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) { > return NULL; > } > - return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); > + > + device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); > + > + /* This function might run on the IO thread and we might race against > + * main thread hot-plugging the device. > + * > + * We assume that as soon as .realized is set to true we can let > + * the user access the device. > + */ Likewise this needs to be load-acquire. Paolo > + if (!device || !atomic_read(&device->qdev.realized)) { > + return NULL; > + } > + > + return device; > } > > void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) >
On Mon, 2020-05-04 at 13:24 +0200, Paolo Bonzini wrote: > On 16/04/20 22:36, Maxim Levitsky wrote: > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399 > > > > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > hw/scsi/virtio-scsi.c | 18 +++++++++++++++++- > > 1 file changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > > index b0f4a35f81..e360b4e03e 100644 > > --- a/hw/scsi/virtio-scsi.c > > +++ b/hw/scsi/virtio-scsi.c > > @@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun) > > > > static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) > > { > > + SCSIDevice *device = NULL; > > + > > if (lun[0] != 1) { > > return NULL; > > } > > if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) { > > return NULL; > > } > > - return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); > > + > > + device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); > > + > > + /* This function might run on the IO thread and we might race against > > + * main thread hot-plugging the device. > > + * > > + * We assume that as soon as .realized is set to true we can let > > + * the user access the device. > > + */ > > Likewise this needs to be load-acquire. Done. Best regards, Maxim Levitsky > > Paolo > > > + if (!device || !atomic_read(&device->qdev.realized)) { > > + return NULL; > > + } > > + > > + return device; > > } > > > > void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) > > > >
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index b0f4a35f81..e360b4e03e 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -35,13 +35,29 @@ static inline int virtio_scsi_get_lun(uint8_t *lun) static inline SCSIDevice *virtio_scsi_device_find(VirtIOSCSI *s, uint8_t *lun) { + SCSIDevice *device = NULL; + if (lun[0] != 1) { return NULL; } if (lun[2] != 0 && !(lun[2] >= 0x40 && lun[2] < 0x80)) { return NULL; } - return scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); + + device = scsi_device_find(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); + + /* This function might run on the IO thread and we might race against + * main thread hot-plugging the device. + * + * We assume that as soon as .realized is set to true we can let + * the user access the device. + */ + + if (!device || !atomic_read(&device->qdev.realized)) { + return NULL; + } + + return device; } void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399 Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- hw/scsi/virtio-scsi.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)