Re: [Qemu-devel] [PATCH] ide: cleanup warnings

[Kevin Wolf <kwolf@redhat.com>]

Am 04.05.2011 15:41, schrieb Andrea Arcangeli:
> On Wed, May 04, 2011 at 10:08:12AM +0200, Kevin Wolf wrote:
>> Isn't it a bug that qemu_aio_flush() doesn't clear aiocb/status? Should
>> we move the ide_set_inactive() call from ide_dma_error to ide_dma_cb?
> 
> How would that make a difference, it's still running in aio context,
> running it a bit earlier won't move the needle?

Yes, sorry, you're right. I was thinking of the werror=stop case, but
this isn't your case and ide_set_inactive would even be wrong there.

> I think it's more
> likely an error path currently not covered by ide_set_inactive that
> may have to be covered. It doesn't seem fatal but I tend to agree if
> we can make that warning go away without putting it under #ifdef like
> usptream, we should do that too.
> 
> Maybe something like this will make it go away?
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 90f553b..b81f1d7 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -377,6 +377,7 @@ void ide_set_sector(IDEState *s, int64_t sector_num)
>  
>  static void ide_rw_error(IDEState *s) {
>      ide_abort_command(s);
> +    ide_set_inactive(s);
>      ide_set_irq(s->bus);
>  }

No, this looks wrong. ide_rw_error is only used for PIO, and
ide_set_inactive() resets the DMA status.

I can't see how you could leave ide_dma_cb without either scheduling
another AIO request or setting aiocb = NULL in ide_set_inactive. I guess
I need to reproduce this and do some debugging...

Kevin