[PATCH] ioemu : add flush in ide_unplug_harddisks()

[PATCH] ioemu : add flush in ide_unplug_harddisks()

[Tomonari Horikoshi]

[-- Attachment #1: Mail message body --]
[-- Type: text/plain, Size: 234 bytes --]


Hi

I think the "flush" is necessary. 
When "ioemu ide device" changes into "vbd" by "ide_unplug_harddisks()". 

What do you think?


Signed-off-by: Tomonari Horikoshi <t.horikoshi@jp.fujitsu.com>

Best regards,
 Tomonari Horikoshi


[-- Attachment #2: ioemu_add_ideflush_in_unplug.patch --]
[-- Type: application/octet-stream, Size: 451 bytes --]

diff --git a/hw/ide.c b/hw/ide.c
index da7057c..d3e35b0 100644
--- a/hw/ide.c
+++ b/hw/ide.c
@@ -2928,6 +2928,10 @@ static void _ide_unplug_harddisks(int start)
         for (j = 0; j < nb_drives; j++)
             if (drives_table[j].bdrv == s->bs)
                 drives_table[j].bdrv = NULL;
+
+        bdrv_flush(s->bs);
+        bdrv_aio_flush(s->bs, ide_flush_cb, s);
+
         bdrv_close(s->bs);
         s->bs = NULL;
         ide_reset(s);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: [PATCH] ioemu : add flush in ide_unplug_harddisks()

[Stefano Stabellini]

Tomonari Horikoshi wrote:

> Hi
> 
> I think the "flush" is necessary. 
> When "ioemu ide device" changes into "vbd" by "ide_unplug_harddisks()". 
> 
> What do you think?
> 
> 

I don't think that calling bdrv_aio_flush is a good idea since it is
asyncronous and right after the call we are going to close the device
without waiting for completion.

Re: [PATCH] ioemu : add flush in ide_unplug_harddisks()

[Ian Jackson]

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] ioemu : add flush in ide_unplug_harddisks()"):
> Tomonari Horikoshi wrote:
> > I think the "flush" is necessary. 
> > When "ioemu ide device" changes into "vbd" by "ide_unplug_harddisks()". 
> 
> I don't think that calling bdrv_aio_flush is a good idea since it is
> asyncronous and right after the call we are going to close the device
> without waiting for completion.

Yes, bdrv_aio_flush is wrong I think.

This seems related to the discussion we had in January.  As I wrote
in
 http://lists.xensource.com/archives/html/xen-devel/2009-01/msg00165.html
it seems to me that the guest should know about be able to flush
for itself ?  After all we implement the IDE flush cache command.

Ian.

Re: [PATCH] ioemu : add flush in ide_unplug_harddisks()

[Tomonari Horikoshi]

Hi.

Thank you for the comment. 

I have understood that it is a best approach to use the IDE flush cache command.  

I'm sorry I had overlooked the thread. 


Best regards,
 Tomonari Horikoshi


Ian Jackson san wrote:----------------------
Sent:    Mon, 23 Feb 2009 16:41:16 +0000
Subject: Re: [Xen-devel] [PATCH] ioemu : add flush in ide_unplug_harddisks()

> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] ioemu : add flush in ide_unplug_harddisks()
> "):
> > Tomonari Horikoshi wrote:
> > > I think the "flush" is necessary. 
> > > When "ioemu ide device" changes into "vbd" by "ide_unplug_harddisks()". 
> > 
> > I don't think that calling bdrv_aio_flush is a good idea since it is
> > asyncronous and right after the call we are going to close the device
> > without waiting for completion.
> 
> Yes, bdrv_aio_flush is wrong I think.
> 
> This seems related to the discussion we had in January.  As I wrote
> in
>  http://lists.xensource.com/archives/html/xen-devel/2009-01/msg00165.html
> it seems to me that the guest should know about be able to flush
> for itself ?  After all we implement the IDE flush cache command.
> 
> Ian.
>