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

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

[Gleb Natapov]

If there is outstanding IDE IO when BIOS starts execution then IDE
commands sent by BIOS will interfere with it and will leave IDE
subsystem in unpredictable state. This can happen when system reboots
unexpectedly without waiting for IO completion. Flushing IO before exit
prevents data lose.

Signed-off-by: Gleb Natapov <gleb@qumranet.com>
---

 vl.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index e42ae64..afa2a3a 100644
--- a/vl.c
+++ b/vl.c
@@ -7553,6 +7553,7 @@ static int main_loop(void)
             if (reset_requested) {
                 reset_requested = 0;
                 qemu_system_reset();
+                qemu_aio_flush();
                 ret = EXCP_INTERRUPT;
             }
             if (powerdown_requested) {
@@ -9178,6 +9179,7 @@ int main(int argc, char **argv)
     }
 
     main_loop();
+    qemu_aio_flush();
     quit_timers();
 
 #if !defined(_WIN32)

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

[Samuel Thibault]

Gleb Natapov, le Wed 13 Aug 2008 16:23:58 +0300, a écrit :
> If there is outstanding IDE IO when BIOS starts execution then IDE
> commands sent by BIOS will interfere with it and will leave IDE
> subsystem in unpredictable state. This can happen when system reboots
> unexpectedly without waiting for IO completion. Flushing IO before exit
> prevents data lose.

I'm wondering: isn't that what happens with real machines?

Samuel

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

[Gleb Natapov]

On Wed, Aug 13, 2008 at 02:46:08PM +0100, Samuel Thibault wrote:
> Gleb Natapov, le Wed 13 Aug 2008 16:23:58 +0300, a écrit :
> > If there is outstanding IDE IO when BIOS starts execution then IDE
> > commands sent by BIOS will interfere with it and will leave IDE
> > subsystem in unpredictable state. This can happen when system reboots
> > unexpectedly without waiting for IO completion. Flushing IO before exit
> > prevents data lose.
> 
> I'm wondering: isn't that what happens with real machines?
> 
With shutdown yes, but why not try harder. With reboot I don't think
that in real HW you can hang IDE interface after reboot ;)

--
			Gleb.

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

[Samuel Thibault]

Gleb Natapov, le Wed 13 Aug 2008 16:59:41 +0300, a écrit :
> On Wed, Aug 13, 2008 at 02:46:08PM +0100, Samuel Thibault wrote:
> > Gleb Natapov, le Wed 13 Aug 2008 16:23:58 +0300, a écrit :
> > > If there is outstanding IDE IO when BIOS starts execution then IDE
> > > commands sent by BIOS will interfere with it and will leave IDE
> > > subsystem in unpredictable state. This can happen when system reboots
> > > unexpectedly without waiting for IO completion. Flushing IO before exit
> > > prevents data lose.
> > 
> > I'm wondering: isn't that what happens with real machines?
> > 
> With shutdown yes, but why not try harder. With reboot I don't think
> that in real HW you can hang IDE interface after reboot ;)

Mmm, I couldn't understand.

With real hardware, if you reboot into the bios the board is not
resetted either, and thus the interference is the same.

Samuel

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

[Gleb Natapov]

On Wed, Aug 13, 2008 at 03:06:41PM +0100, Samuel Thibault wrote:
> Gleb Natapov, le Wed 13 Aug 2008 16:59:41 +0300, a écrit :
> > On Wed, Aug 13, 2008 at 02:46:08PM +0100, Samuel Thibault wrote:
> > > Gleb Natapov, le Wed 13 Aug 2008 16:23:58 +0300, a écrit :
> > > > If there is outstanding IDE IO when BIOS starts execution then IDE
> > > > commands sent by BIOS will interfere with it and will leave IDE
> > > > subsystem in unpredictable state. This can happen when system reboots
> > > > unexpectedly without waiting for IO completion. Flushing IO before exit
> > > > prevents data lose.
> > > 
> > > I'm wondering: isn't that what happens with real machines?
> > > 
> > With shutdown yes, but why not try harder. With reboot I don't think
> > that in real HW you can hang IDE interface after reboot ;)
> 
> Mmm, I couldn't understand.
> 
> With real hardware, if you reboot into the bios the board is not
> resetted either, and thus the interference is the same.
> 
On real hardware if you press reset button then HW reset is performed
(not power up reset) and qemu HW handles this in reset notifiers. I have
no idea what IDE does during reset with outstanding IO. It should either
cancel it or complete it. Currently BOCHS BIOS doesn't check that IDE is
busy (it doesn't even know that HW is present) and sends overlapping
command.

--
			Gleb.

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

[Samuel Thibault]

Gleb Natapov, le Wed 13 Aug 2008 17:25:58 +0300, a écrit :
> On Wed, Aug 13, 2008 at 03:06:41PM +0100, Samuel Thibault wrote:
> > Gleb Natapov, le Wed 13 Aug 2008 16:59:41 +0300, a écrit :
> > > On Wed, Aug 13, 2008 at 02:46:08PM +0100, Samuel Thibault wrote:
> > > > Gleb Natapov, le Wed 13 Aug 2008 16:23:58 +0300, a écrit :
> > > > > If there is outstanding IDE IO when BIOS starts execution then IDE
> > > > > commands sent by BIOS will interfere with it and will leave IDE
> > > > > subsystem in unpredictable state. This can happen when system reboots
> > > > > unexpectedly without waiting for IO completion. Flushing IO before exit
> > > > > prevents data lose.
> > > > 
> > > > I'm wondering: isn't that what happens with real machines?
> > > > 
> > > With shutdown yes, but why not try harder. With reboot I don't think
> > > that in real HW you can hang IDE interface after reboot ;)
> > 
> > Mmm, I couldn't understand.
> > 
> > With real hardware, if you reboot into the bios the board is not
> > resetted either, and thus the interference is the same.
> > 
> On real hardware if you press reset button

That's when you press the reset button.  When you have a triple fault
the IDE board won't be resetted.

> Currently BOCHS BIOS doesn't check that IDE is busy (it doesn't even
> know that HW is present) and sends overlapping command.

Then fix it.

Samuel

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

[Avi Kivity]

Samuel Thibault wrote:
>>
>> On real hardware if you press reset button
>>     
>
> That's when you press the reset button.  When you have a triple fault
> the IDE board won't be resetted.
>
>   

Why not?  IIUC triple fault is exported as a cpu pin, which the 
chipset/motherboard wires into the board-level reset line.

>> Currently BOCHS BIOS doesn't check that IDE is busy (it doesn't even
>> know that HW is present) and sends overlapping command.
>>     
>
> Then fix it.
>   

I think the bios can expect the IDE hardware (or any other device) to be 
quiescent on entry.  It doesn't make sense to initiate an explicit reset 
immediately after boot; that would slow things down.

-- 
error compiling committee.c: too many arguments to function

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

[Gleb Natapov]

On Wed, Aug 13, 2008 at 03:29:04PM +0100, Samuel Thibault wrote:
> Gleb Natapov, le Wed 13 Aug 2008 17:25:58 +0300, a écrit :
> > On Wed, Aug 13, 2008 at 03:06:41PM +0100, Samuel Thibault wrote:
> > > Gleb Natapov, le Wed 13 Aug 2008 16:59:41 +0300, a écrit :
> > > > On Wed, Aug 13, 2008 at 02:46:08PM +0100, Samuel Thibault wrote:
> > > > > Gleb Natapov, le Wed 13 Aug 2008 16:23:58 +0300, a écrit :
> > > > > > If there is outstanding IDE IO when BIOS starts execution then IDE
> > > > > > commands sent by BIOS will interfere with it and will leave IDE
> > > > > > subsystem in unpredictable state. This can happen when system reboots
> > > > > > unexpectedly without waiting for IO completion. Flushing IO before exit
> > > > > > prevents data lose.
> > > > > 
> > > > > I'm wondering: isn't that what happens with real machines?
> > > > > 
> > > > With shutdown yes, but why not try harder. With reboot I don't think
> > > > that in real HW you can hang IDE interface after reboot ;)
> > > 
> > > Mmm, I couldn't understand.
> > > 
> > > With real hardware, if you reboot into the bios the board is not
> > > resetted either, and thus the interference is the same.
> > > 
> > On real hardware if you press reset button
> 
> That's when you press the reset button.  When you have a triple fault
> the IDE board won't be resetted.
> 
As far as I see qemu doesn't handle triple faults yet and when it will
handle it we will flush aio there too.

> > Currently BOCHS BIOS doesn't check that IDE is busy (it doesn't even
> > know that HW is present) and sends overlapping command.
> 
> Then fix it.
> 
Wow you found one more reason to do nothing in qemu. Congratulations.
The real HW are much more complex then what qemu is implementing. The
real HW can even save you buffers after you unplugged your computer. And
for real HW it takes much less time to do it too.

--
			Gleb.

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

[Samuel Thibault]

Avi Kivity wrote:
> Why not? IIUC triple fault is exported as a cpu pin, which the chipset/
> motherboard wires into the board-level reset line.

Ah ok, I didn't know it was propagated further than the cpu.  Probably a
good thing actually :)

> I think the bios can expect the IDE hardware (or any other device) to be
> quiescent on entry. It doesn't make sense to initiate an explicit reset
> immediately after boot; that would slow things down.

Well, the Bios reset entry could be entered by "accident" too :)
Actually windows drivers used to break my video board so badly that I
had to manually reset my machine before booting Linux.

Well, anyway, I agree with Anthony: the IDE emulation should behave
like hardware: try to cancel pending requests and wait for everything
to finish, instead of possibly writing what the bios puts in the being
written pages :) And it should reset the DMA engines too.

The block layer itself could have a last sync check.

Samuel

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

[Anthony Liguori]

Gleb Natapov wrote:
> If there is outstanding IDE IO when BIOS starts execution then IDE
> commands sent by BIOS will interfere with it and will leave IDE
> subsystem in unpredictable state. This can happen when system reboots
> unexpectedly without waiting for IO completion. Flushing IO before exit
> prevents data lose.
>
> Signed-off-by: Gleb Natapov <gleb@qumranet.com>
> ---
>
>  vl.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index e42ae64..afa2a3a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -7553,6 +7553,7 @@ static int main_loop(void)
>              if (reset_requested) {
>                  reset_requested = 0;
>                  qemu_system_reset();
> +                qemu_aio_flush();
>   

Perhaps the aio block layer should do qemu_register_reset() with a 
handler that does a flush.

Regards,

Anthony Liguori

>                  ret = EXCP_INTERRUPT;
>              }
>              if (powerdown_requested) {
> @@ -9178,6 +9179,7 @@ int main(int argc, char **argv)
>      }
>  
>      main_loop();
> +    qemu_aio_flush();
>      quit_timers();
>  
>  #if !defined(_WIN32)
>
>
>
>   

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

[Gleb Natapov]

On Wed, Aug 13, 2008 at 08:52:50AM -0500, Anthony Liguori wrote:
> Gleb Natapov wrote:
>> If there is outstanding IDE IO when BIOS starts execution then IDE
>> commands sent by BIOS will interfere with it and will leave IDE
>> subsystem in unpredictable state. This can happen when system reboots
>> unexpectedly without waiting for IO completion. Flushing IO before exit
>> prevents data lose.
>>
>> Signed-off-by: Gleb Natapov <gleb@qumranet.com>
>> ---
>>
>>  vl.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index e42ae64..afa2a3a 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -7553,6 +7553,7 @@ static int main_loop(void)
>>              if (reset_requested) {
>>                  reset_requested = 0;
>>                  qemu_system_reset();
>> +                qemu_aio_flush();
>>   
>
> Perhaps the aio block layer should do qemu_register_reset() with a  
> handler that does a flush.
>
I though about doing it this way, but then I saw that qemu_register_reset()
is used for HW reset notification only. I don't think that hw/ide.c is the
right place to register the notifier though, so I decided to call it
explicitly. Do you think I should do qemu_register_reset() in block.c?

--
			Gleb.

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

[Anthony Liguori]

Gleb Natapov wrote:
> On Wed, Aug 13, 2008 at 08:52:50AM -0500, Anthony Liguori wrote:
>   
>> Gleb Natapov wrote:
>>     
>>> If there is outstanding IDE IO when BIOS starts execution then IDE
>>> commands sent by BIOS will interfere with it and will leave IDE
>>> subsystem in unpredictable state. This can happen when system reboots
>>> unexpectedly without waiting for IO completion. Flushing IO before exit
>>> prevents data lose.
>>>
>>> Signed-off-by: Gleb Natapov <gleb@qumranet.com>
>>> ---
>>>
>>>  vl.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/vl.c b/vl.c
>>> index e42ae64..afa2a3a 100644
>>> --- a/vl.c
>>> +++ b/vl.c
>>> @@ -7553,6 +7553,7 @@ static int main_loop(void)
>>>              if (reset_requested) {
>>>                  reset_requested = 0;
>>>                  qemu_system_reset();
>>> +                qemu_aio_flush();
>>>   
>>>       
>> Perhaps the aio block layer should do qemu_register_reset() with a  
>> handler that does a flush.
>>
>>     
> I though about doing it this way, but then I saw that qemu_register_reset()
> is used for HW reset notification only. I don't think that hw/ide.c is the
> right place to register the notifier though, so I decided to call it
> explicitly. Do you think I should do qemu_register_reset() in block.c?
>   

But shouldn't the various disk types be the ones to flush their 
outstanding IO on reset?

Regards,

Anthony Liguori

> --
> 			Gleb.
>
>
>   

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

[Avi Kivity]

Anthony Liguori wrote: 
>> I though about doing it this way, but then I saw that 
>> qemu_register_reset()
>> is used for HW reset notification only. I don't think that hw/ide.c 
>> is the
>> right place to register the notifier though, so I decided to call it
>> explicitly. Do you think I should do qemu_register_reset() in block.c?
>>   
>
> But shouldn't the various disk types be the ones to flush their 
> outstanding IO on reset?

Do you mean qemu block drivers, or disk controllers?

-- 
error compiling committee.c: too many arguments to function

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

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>> I though about doing it this way, but then I saw that 
>>> qemu_register_reset()
>>> is used for HW reset notification only. I don't think that hw/ide.c 
>>> is the
>>> right place to register the notifier though, so I decided to call it
>>> explicitly. Do you think I should do qemu_register_reset() in block.c?
>>>   
>>
>> But shouldn't the various disk types be the ones to flush their 
>> outstanding IO on reset?
>
> Do you mean qemu block drivers, or disk controllers?

The disk controllers.  It's a semantic of the disk controller itself 
(what happens on reset if there are outstanding requests).  Do those 
requests complete or are the cancelled?

Regards,

Anthony Liguori

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

[Avi Kivity]

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Anthony Liguori wrote:
>>>> I though about doing it this way, but then I saw that 
>>>> qemu_register_reset()
>>>> is used for HW reset notification only. I don't think that hw/ide.c 
>>>> is the
>>>> right place to register the notifier though, so I decided to call it
>>>> explicitly. Do you think I should do qemu_register_reset() in block.c?
>>>>   
>>>
>>> But shouldn't the various disk types be the ones to flush their 
>>> outstanding IO on reset?
>>
>> Do you mean qemu block drivers, or disk controllers?
>
> The disk controllers.  It's a semantic of the disk controller itself 
> (what happens on reset if there are outstanding requests).  Do those 
> requests complete or are the cancelled?
>

I imagine it depends on the controller, and that they are allowed to 
drop pending requests.  If software wants data on the disk, it should 
wait for completion.

-- 
error compiling committee.c: too many arguments to function

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

[Gleb Natapov]

On Wed, Aug 13, 2008 at 10:07:06AM -0500, Anthony Liguori wrote:
>>> Perhaps the aio block layer should do qemu_register_reset() with a   
>>> handler that does a flush.
>>>
>>>     
>> I though about doing it this way, but then I saw that qemu_register_reset()
>> is used for HW reset notification only. I don't think that hw/ide.c is the
>> right place to register the notifier though, so I decided to call it
>> explicitly. Do you think I should do qemu_register_reset() in block.c?
>>   
>
> But shouldn't the various disk types be the ones to flush their  
> outstanding IO on reset?
>
Yes, conceptually this is what should be done. But all outstanding block IO
requests are stored in one global list in block-raw.c and this file also
provides a global facility to flush all request from this global queue
that is why I used it. Otherwise each subsisted will have to be touched
to have desired affect.

--
			Gleb.

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

[Anthony Liguori]

Gleb Natapov wrote:
> On Wed, Aug 13, 2008 at 10:07:06AM -0500, Anthony Liguori wrote:
>   
> Yes, conceptually this is what should be done. But all outstanding block IO
> requests are stored in one global list in block-raw.c and this file also
> provides a global facility to flush all request from this global queue
> that is why I used it. Otherwise each subsisted will have to be touched
> to have desired affect.
>   

What happens on reset to outstanding disk requests is a semantic of the 
controller itself.  Really, I think the right thing to do is cancel any 
request that can be cancelled.  If a request cannot be cancelled then we 
should wait for it to complete.

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 
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.

Regards,

Anthony Lgiuori

> --
> 			Gleb.
>
>
>   

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

[Gleb Natapov]

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.

> 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'll send updated patch.

--
			Gleb.

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

[Avi Kivity]

Gleb Natapov wrote:
> 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.
>
>   

If a guest didn't wait for an I/O to complete, it shouldn't expect it to 
be on disk.

Of course, we can have the IDE layer wait instead of cancelling, which 
will get the request onto the disk.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

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

[Gleb Natapov]

On Wed, Aug 13, 2008 at 09:40:34PM +0300, Avi Kivity wrote:
> Gleb Natapov wrote:
>> 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.
>>
>>   
>
> If a guest didn't wait for an I/O to complete, it shouldn't expect it to  
> be on disk.
If you'll save your swiss bank account details in file and suddenly
your guest will crash, you'll surely appreciate if qemu will save it
for you :)

>
> Of course, we can have the IDE layer wait instead of cancelling, which  
> will get the request onto the disk.
>
That is the current approach, but the wait is done for all block IO not
just IDE.

--
			Gleb.

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

[Anthony Liguori]

Gleb Natapov wrote:
> On Wed, Aug 13, 2008 at 09:40:34PM +0300, Avi Kivity wrote:
>   
>> Gleb Natapov wrote:
>>     
>>> 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.
>>>
>>>   
>>>       
>> If a guest didn't wait for an I/O to complete, it shouldn't expect it to  
>> be on disk.
>>     
> If you'll save your swiss bank account details in file and suddenly
> your guest will crash, you'll surely appreciate if qemu will save it
> for you :)
>
>   
>> Of course, we can have the IDE layer wait instead of cancelling, which  
>> will get the request onto the disk.
>>
>>     
> That is the current approach, but the wait is done for all block IO not
> just IDE.
>   

If the guest hasn't seen confirmation that the data is on disk, as far 
as the guest is concerned, it's not on disk.

If you have a journalled file system, for instance, unless it receives 
that notification, it's going to replay whatever has happened in the 
journal since the last successful write operation.  Practically 
speaking, I don't think you're saving data.

Regards,

Anthony Liguori

> --
> 			Gleb.
>
>
>   

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

[Jamie Lokier]

Anthony Liguori wrote:
> Gleb Natapov wrote:
> >On Wed, Aug 13, 2008 at 09:40:34PM +0300, Avi Kivity wrote:
> >  
> >>Gleb Natapov wrote:
> >>    
> >>>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.
> >>>
> >>>  
> >>>      
> >>If a guest didn't wait for an I/O to complete, it shouldn't expect it to  
> >>be on disk.
> >>    
> >If you'll save your swiss bank account details in file and suddenly
> >your guest will crash, you'll surely appreciate if qemu will save it
> >for you :)
> >
> >  
> >>Of course, we can have the IDE layer wait instead of cancelling, which  
> >>will get the request onto the disk.
> >>
> >>    
> >That is the current approach, but the wait is done for all block IO not
> >just IDE.
> >  
> 
> If the guest hasn't seen confirmation that the data is on disk, as far 
> as the guest is concerned, it's not on disk.
> 
> If you have a journalled file system, for instance, unless it receives 
> that notification, it's going to replay whatever has happened in the 
> journal since the last successful write operation.  Practically 
> speaking, I don't think you're saving data.

Except for old guests.  Old guests don't have journalled filesystems
and use the "pray it works" method.  With old hardware it generally
does work, with newer hardware its a bit less reliable.  E.g. old
disks and old BIOSes don't enable disk write cache, and old guests
don't know anything about it.

-- Jamie

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.
>
>
>   

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

[Samuel Thibault]

Anthony Liguori, le Wed 13 Aug 2008 14:03:13 -0500, a écrit :
> 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.

Well, the guest is notified asynchronously with real hardware, so it may
miss a completion too.

Samuel