Re: [PATCH v2 7/7] virtio-scsi: use scsi_device_get

2020-07-09 Thread Maxim Levitsky
On Wed, 2020-05-27 at 16:50 +0100, Stefan Hajnoczi wrote:
> On Mon, May 11, 2020 at 07:09:51PM +0300, Maxim Levitsky wrote:
> > This will help us to avoid the scsi device disappearing
> > after we took a reference to it.
> > 
> > It doesn't by itself forbid case when we try to access
> > an unrealized device
> > 
> > Suggested-by: Stefan Hajnoczi 
> > Signed-off-by: Maxim Levitsky 
> > ---
> >  hw/scsi/virtio-scsi.c | 23 +++
> >  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> I'm not very familiar with the SCSI emulation code, but this looks
> correct. My understanding of what this patch does:
> 
> This patch keeps SCSIDevice alive between scsi_device_find() and
> scsi_req_new(). Previously no SCSIDevice ref was taken so the device
> could have been freed before scsi_req_new() had a chance to take a ref.
Yep, I also verified now that this is what happens.

> 
> The TMF case is similar: the SCSIDevice ref must be held during
> virtio_scsi_do_tmf(). We don't need to worry about the async cancel
> notifiers because the request being canceled already holds a ref.
This code I understand less to be honest, but in the worst case,
the patch shoudn't make it worse.

Best regards,
Maxim Levitsky






Re: [PATCH v2 7/7] virtio-scsi: use scsi_device_get

2020-05-27 Thread Stefan Hajnoczi
On Mon, May 11, 2020 at 07:09:51PM +0300, Maxim Levitsky wrote:
> This will help us to avoid the scsi device disappearing
> after we took a reference to it.
> 
> It doesn't by itself forbid case when we try to access
> an unrealized device
> 
> Suggested-by: Stefan Hajnoczi 
> Signed-off-by: Maxim Levitsky 
> ---
>  hw/scsi/virtio-scsi.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)

I'm not very familiar with the SCSI emulation code, but this looks
correct. My understanding of what this patch does:

This patch keeps SCSIDevice alive between scsi_device_find() and
scsi_req_new(). Previously no SCSIDevice ref was taken so the device
could have been freed before scsi_req_new() had a chance to take a ref.

The TMF case is similar: the SCSIDevice ref must be held during
virtio_scsi_do_tmf(). We don't need to worry about the async cancel
notifiers because the request being canceled already holds a ref.


signature.asc
Description: PGP signature


Re: [PATCH v2 7/7] virtio-scsi: use scsi_device_get

2020-05-27 Thread Stefan Hajnoczi
On Mon, May 11, 2020 at 07:09:51PM +0300, Maxim Levitsky wrote:
> This will help us to avoid the scsi device disappearing
> after we took a reference to it.
> 
> It doesn't by itself forbid case when we try to access
> an unrealized device
> 
> Suggested-by: Stefan Hajnoczi 
> Signed-off-by: Maxim Levitsky 
> ---
>  hw/scsi/virtio-scsi.c | 23 +++
>  1 file changed, 15 insertions(+), 8 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature