Re: [Qemu-devel] [PATCH] Asynchronous reliable and configurable cache flush

[Paul Brook <paul@codesourcery.com>]

On Tuesday 01 April 2008, Ian Jackson wrote:
> +    if (!s->bs) return; /* yikes */

This comment is distinctly unhelpful. I guess this is the condition mentioned 
in ide_device_utterly_broken, but it's hard to tell.

> @@ -1021,6 +1031,13 @@ static void ide_write_dma_cb(void *opaque, int ret)
>  
>      /* end of transfer ? */
>      if (s->nsector == 0) {
> +       if (!s->write_cache) {
> +           ret = bdrv_flush(s->bs);
> +           if (ret != 0) {
> +               ide_dma_error(s);
> +               return;
> +           }
> +       }

By my reading this is adding a synchronous flush to the end of an async write 
operation, which in practice makes the whole operation synchronous.

> +static void scsi_flush_cb(void *opaque, int ret) {
> +    SCSIRequest *r = opaque;
> +    if (ret) {
> +        BADF("scsi-disc: IO error on flush: %s\n", strerror(-ret));

This is wrong for two reasons: BADF already adds a suitable prefix, and is for 
reporting qemu bugs (i.e. missing features or things that should never 
happen). This is just a normal IO error, which we correctly report to the 
guest.

Paul