Re: [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized

2020-05-11 Thread Maxim Levitsky 
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 
> > Signed-off-by: Maxim Levitsky 
> > ---
> >  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)
> > 
> 
>

Re: [PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized

2020-05-04 Thread Paolo Bonzini 
On 16/04/20 22:36, Maxim Levitsky wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399
> 
> Suggested-by: Paolo Bonzini 
> Signed-off-by: Maxim Levitsky 
> ---
>  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)
>

[PATCH 4/4] virtio-scsi: don't touch scsi devices that are not yet realized

2020-04-16 Thread Maxim Levitsky 
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399

Suggested-by: Paolo Bonzini 
Signed-off-by: Maxim Levitsky 
---
 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.
+ */
+
+if (!device || !atomic_read(&device->qdev.realized)) {
+return NULL;
+}
+
+return device;
 }
 
 void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req)
-- 
2.17.2