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

[Paolo Bonzini <pbonzini@redhat.com>]

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