Re: [Qemu-devel] [PATCH 1.2 6/7] ide: support enable/disable write cache

[Kevin Wolf]

Am 22.05.2012 18:17, schrieb Paolo Bonzini:
> Enabling or disabling the write cache is done with the SET FEATURES
> command.  The command can be issued with sg_sat_set_features from
> sg3-utils.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/ide/core.c |   18 +++++++++++++++---
>  1 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 9785d5f..7c50567 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1047,6 +1047,7 @@ static bool ide_cmd_permitted(IDEState *s, uint32_t cmd)
>  
>  void ide_exec_cmd(IDEBus *bus, uint32_t val)
>  {
> +    uint16_t *identify_data;
>      IDEState *s;
>      int n;
>      int lba48 = 0;
> @@ -1231,10 +1232,21 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
>              goto abort_cmd;
>          /* XXX: valid for CDROM ? */
>          switch(s->feature) {
> -        case 0xcc: /* reverting to power-on defaults enable */
> -        case 0x66: /* reverting to power-on defaults disable */
>          case 0x02: /* write cache enable */
> +            bdrv_set_enable_write_cache(s->bs, true);
> +            identify_data = (uint16_t *)s->identify_data;
> +            put_le16(identify_data + 85, (1 << 14) | (1 << 5) | 1);

How about just s->identify_set = 0; instead, so that all IDENTIFY logic
stays together in one place?

> +            s->status = READY_STAT | SEEK_STAT;
> +            ide_set_irq(s->bus);
> +            break;
>          case 0x82: /* write cache disable */
> +            bdrv_set_enable_write_cache(s->bs, false);
> +            identify_data = (uint16_t *)s->identify_data;
> +            put_le16(identify_data + 85, (1 << 14) | 1);
> +            ide_flush_cache(s);

Now I see, you're doing it this way because then the flush is AIO. Hm...
Making it bdrv_aio_set_enable_write_cache() looks a bit extreme.

Kevin