Re: [Qemu-block] [Qemu-devel] [PATCH 1/5] ide: push call to end_transfer_func out of start_transfer callback

[Paolo Bonzini]

On 20/03/2018 22:46, John Snow wrote:
>>      }
>> -    if (s->bus->dma->ops->start_transfer) {
>> -        s->bus->dma->ops->start_transfer(s->bus->dma);
>> +    if (!s->bus->dma->ops->start_transfer) {
>> +        s->end_transfer_func = end_transfer_func;
>> +        return;
>>      }
>> +    s->bus->dma->ops->start_transfer(s->bus->dma);
>> +    end_transfer_func(s);
> wow, this makes me really uneasy to look at. The assumption now is: "If
> you have a start_transfer method, that method if successful implies that
> the transfer is *COMPLETED*."
> 
> It's implicit here, but I think anyone but the two of us would probably
> not understand that implication. (Or me in three months.) What can we do
> about that? Since AHCI is the only interface that uses this callback, I
> wonder if we can't just rename it to something more obvious.

You are completely right, it should be renamed to pio_transfer.

> Under normal circumstances this function really does just "start" a
> transfer and it's not obvious from context that some adapters have
> synchronous methods to finish the transfer without further PIO pingpong
> with the guest.

Yeah, though it is already doing it---the patch is only unhiding the
mutual recursion by pulling it one level up.

Paolo