On Thu, Mar 29, 2012 at 7:42 AM, Michael Tokarev <m...@tls.msk.ru> 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
