diff mbox series

[v5,1/9] scsi/scsi_bus: switch search direction in scsi_device_find

Message ID 20200913160259.32145-2-mlevitsk@redhat.com
State New
Headers show
Series Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread | expand

Commit Message

Maxim Levitsky Sept. 13, 2020, 4:02 p.m. UTC 
This change will allow us to convert the bus children list to RCU,
while not changing the logic of this function

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/scsi/scsi-bus.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini Sept. 25, 2020, 1:36 p.m. UTC | #1 
On 13/09/20 18:02, Maxim Levitsky wrote:
> This change will allow us to convert the bus children list to RCU,

> while not changing the logic of this function

> 

> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>

> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

> ---

>  hw/scsi/scsi-bus.c | 12 ++++++++++--

>  1 file changed, 10 insertions(+), 2 deletions(-)

> 

> diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c

> index df65cc2223..f8adfbc2a5 100644

> --- a/hw/scsi/scsi-bus.c

> +++ b/hw/scsi/scsi-bus.c

> @@ -1572,7 +1572,7 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)

>      BusChild *kid;

>      SCSIDevice *target_dev = NULL;

>  

> -    QTAILQ_FOREACH_REVERSE(kid, &bus->qbus.children, sibling) {

> +    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {

>          DeviceState *qdev = kid->child;

>          SCSIDevice *dev = SCSI_DEVICE(qdev);

>  

> @@ -1580,7 +1580,15 @@ SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)

>              if (dev->lun == lun) {

>                  return dev;

>              }

> -            target_dev = dev;

> +

> +            /*

> +             * If we don't find exact match (channel/bus/lun),

> +             * we will return the first device which matches channel/bus

> +             */

> +

> +            if (!target_dev) {

> +                target_dev = dev;

> +            }

>          }

>      }

>      return target_dev;

> 


I think this breaks the detection of duplicate LUNs.  I'll play with it
and let you know.

Paolo
diff mbox series

Patch

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index df65cc2223..f8adfbc2a5 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1572,7 +1572,7 @@  SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
     BusChild *kid;
     SCSIDevice *target_dev = NULL;
 
-    QTAILQ_FOREACH_REVERSE(kid, &bus->qbus.children, sibling) {
+    QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
         DeviceState *qdev = kid->child;
         SCSIDevice *dev = SCSI_DEVICE(qdev);
 
@@ -1580,7 +1580,15 @@  SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
             if (dev->lun == lun) {
                 return dev;
             }
-            target_dev = dev;
+
+            /*
+             * If we don't find exact match (channel/bus/lun),
+             * we will return the first device which matches channel/bus
+             */
+
+            if (!target_dev) {
+                target_dev = dev;
+            }
         }
     }
     return target_dev;