Re: [Qemu-devel] [PATCH 15/35] scsi: remove devs array from SCSIBus

[Paolo Bonzini]

On 10/21/2011 02:31 PM, Kevin Wolf wrote:
> > >  diff --git a/hw/esp.c b/hw/esp.c
> > >  index d3fb1c6..8e17005 100644
> > >  --- a/hw/esp.c
> > >  +++ b/hw/esp.c
> > >  @@ -217,7 +217,8 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
> > >            s->async_len = 0;
> > >        }
> > >
> > >  -    if (target>= ESP_MAX_DEVS || !s->bus.devs[target]) {
> > >  +    s->current_dev = scsi_device_find(&s->bus, target, 0);
> > >  +    if (!s->current_dev) {
> > >            // No such drive
> > >            s->rregs[ESP_RSTAT] = 0;
> > >            s->rregs[ESP_RINTR] = INTR_DC;
> > >  @@ -225,7 +226,6 @@ static uint32_t get_cmd(ESPState *s, uint8_t *buf)
> > >            esp_raise_irq(s);
> > >            return 0;
> > >        }
> > >  -    s->current_dev = s->bus.devs[target];
> > >        return dmalen;
> > >    }
> > >
> > >  @@ -236,6 +236,7 @@ static void do_busid_cmd(ESPState *s, uint8_t *buf, 
> > uint8_t busid)
> > >
> > >        trace_esp_do_busid_cmd(busid);
> > >        lun = busid&  7;
> > >  +    s->current_dev = scsi_device_find(&s->bus, s->current_dev->id, lun);
> This is new, and I can't see an explanation in the commit log.

It isn't really new; up until now the lun was hard-coded to zero and so 
s->current_dev could be set in get_cmd.  Now we have to delay it until 
do_busid_cmd because we actually have a place to pass the LUN.

That said, s->current_dev is never really used outside do_busid_cmd, so 
I can instead do something like:

-   s->current_req = scsi_req_new(s->current_dev, 0, lun, buf, NULL);
+   SCSIDevice *current_lun;
+
+   current_lun = scsi_device_find(&s->bus, 0, s->current_dev->id, lun);
+   s->current_req = scsi_req_new(current_lun, 0, lun, buf, NULL);

That would be the same as the code I have above.

Paolo