Re: [Qemu-devel] [PATCH] Flush pending AIO on reboot and shutdown.

[Anthony Liguori]

Gleb Natapov wrote:
> On Wed, Aug 13, 2008 at 10:53:57AM -0500, Anthony Liguori wrote:
>   
> > Now that I think about it, I think your fixing the wrong problem.  The  
> > issue isn't that the IO requests need to be completed, but that they  
> > *will* complete which means that the IDE driver will receive a callback  
> > for a request that it no longer knows about (because it was reset).  So  
> >     
> Not exactly. IDE still knows about the request after reset (actually currently
> IDE doesn't know about reset), but the IDE code is written in such a way that 
> there can be only one outstanding request in progress. When guest issues
> another request before previous request is completed global data is
> modified and first request start to use wrong data and consequences are
> unpredictable. It may be crash, image corruption, infinity recursion.
> The fact that IDE code allows to issue another request wile IDE is still
> busy is also a bug.
>   

Yeah, let's fix this properly.  I worry that this could be a DoS on the 
part of the guest (or even worse).

> > what we really need to do is modify the IDE device such that when it is  
> > reset, it cancels any pending requests.
> >
> > The fact that this reset happens as a consequence of a system reset is  
> > really just a coincidence.
> >     
> That will also solve the problem of cause, but what bother me is that we
> consciously drop user data that we can easily save. Why? Real HW tries
> hard to save every bit of user data and we just decided to drop it. The
> difference between cancel or complete a request may be corrupted or not
> corrupted file system after a crash.
>   

I don't think of this as "saving user data".  From the guest's 
perspective, data is only written to disk after a write completion has 
be issued.  I think it's worse for data to end up being written to disk 
without that completion ever being seen by the guest.

Regards,

Anthony Liguori

> I'll send updated patch.
>
> --
>                         Gleb.
>
>
>