Re: [Qemu-devel] [PATCH 1/2] ide: convert ide_sector_read() to asynchronous I/O

[Stefan Hajnoczi]

On Thu, Mar 29, 2012 at 7:42 AM, Michael Tokarev <address@hidden> wrote:
> On 28.03.2012 19:43, Stefan Hajnoczi wrote:
>>  void ide_sector_read(IDEState *s)
>>  {
> []
>> +    s->iov.iov_base = s->io_buffer;
>> +    s->iov.iov_len  = n * BDRV_SECTOR_SIZE;
>> +    qemu_iovec_init_external(&s->qiov, &s->iov, 1);
>> +
>> +    bdrv_acct_start(s->bs, &s->acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ);
>> +    bdrv_aio_readv(s->bs, sector_num, &s->qiov, n,
>> +                   ide_sector_read_cb, s);
>>  }
>
> Shouldn't this function be returning something and
> check the return value of bdrv_aio_readv() ?

I think you are right that something is missing here.  If the IDE
controller is reset then we need to cancel the pending aio request.
Otherwise we could end up with a dangling request across reset that
overwrites io_buffer.

About checking the return value: it used to be the case that the
return value needed to be checked.

This was kind of a wart in the interface because it created 2 error
handling paths: one immediate return and one callback with error.  I
think Paolo and Kevin were the ones to address this.  You can check
out this patch for starters but there are a few more related ones:

ad54ae80c7 ("block: bdrv_aio_* do not return NULL")

Today the return value does not need to be checked for NULL.

Stefan