Am 20.05.2011 19:43, schrieb Paolo Bonzini:
> On 05/20/2011 06:04 PM, Christoph Hellwig wrote:
>>> -void scsi_req_enqueue(SCSIRequest *req)
>>> +int32_t scsi_req_enqueue(SCSIRequest *req, uint8_t *buf)
>>> {
>>> + int32_t rc;
>>> assert(!req->enqueued);
>>> scsi_req_ref(req);
>>> req->enqueued = true;
>>> QTAILQ_INSERT_TAIL(&req->dev->requests, req, next);
>>> +
>>> + /* Make sure the request doesn't disappear under send_command's feet.
>>> */
>>> + scsi_req_ref(req);
>>> + rc = req->dev->info->send_command(req, buf);
>>> + scsi_req_unref(req);
>>> + return rc;
>>
>> How would it disappear given that we grabbed another reference just before?
>
> Welcome to the wonderful world of nested callbacks. :(
>
> Suppose send_command completes a request. scsi_req_complete then
> dequeues it (which undoes the reference above), and calls the device who
> owned it. The device who owned the request then presumably NULLs a
> pointer and drops the last reference. The request is then freed.
Maybe the callback should be done from a BH then? It sounds like this
could cause more bugs than what you're fixing here.
Kevin
