Re: [Xen-devel] Windows Bug Check 0x101 issue

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Kouya Shimura writes ("Re: [Xen-devel] Windows Bug Check 0x101 issue"):
> Samuel Thibault writes:
> > Ian Jackson, le Tue 25 Mar 2008 11:28:32 +0000, a écrit :
> > > The other is that the IDE flush necessarily blocks.
> > 
> > What do you mean by that?  In a real machine, the processor doesn't
> > block while the flush is being done, and the OS just expects to see an
> > irq some time after.  In that regard his patch should work fine.

I must have misread the IDE spec I was reading.

> > That said it can't be applied as is because of the other points you
> > raised, of course.
> 
> Anyway, I remade the patch as you point out. Is it enough?

This one is much better but I still have a comment ...

> To be honest, I'm skeptical about the necessity of the flush
> for a *emulated* IDE disk but shared SCSI disk.

I'm not sure I follow.  (What do you mean by `shared SCSI disk'?
We're discussing the IDE driver here ...)

> +BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, 
> +                                 BlockDriverCompletionFunc *cb, void *opaque)
> +{
> +    BlockDriver *drv = bs->drv;
> +
> +    if (!drv)
> +        return NULL;
> +    if (!drv->bdrv_aio_flush)
> +        return NULL;
> +
> +    return drv->bdrv_aio_flush(bs, cb, opaque);
> +}

This function should emulate the asynchronous operation with the
synchronous one: so it should call the synchronous ->bdrv_flush and
then call the callback.

That will make it work like bdrv_aio_{read,write}.  (Although there is
I think no need to replicate the baroque bdrv_aio_{read,write}_em
emulation methods; that's an overly-complex way to do things.)

If you do that then callers who get NULL from bdrv_aio_flush will know
that it is actually an error.  Your current code, as well as needing
the non-aio emulation at each call site, will try the fallback after
errors as well as lack of aio support.

> +static void ide_flush_cb(void *opaque, int ret)
> +{
> +    IDEState *s = opaque;
> +
> +    s->status = READY_STAT;
> +    ide_set_irq(s);

You need to check the return value (ret) and set an appropriate IDE
error status if the operation failed.  ide_abort_command may be of
some use.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel