diff mbox

[07/11] blockdev: flip default cache mode from writethrough to writeback

Message ID 1344617249-6620-8-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf Aug. 10, 2012, 4:47 p.m. UTC 
From: Paolo Bonzini <pbonzini@redhat.com>

Now all major device models (IDE, SCSI, virtio) can choose between
writethrough and writeback at run-time, and virtio will even revert
to writethrough if the guest is not capable of sending flushes.  So
we can change the default to writeback at last.

Tested, for lack of a better idea, with a breakpoint on bdrv_open
and all cache choices one by one.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 blockdev.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

Comments

Artyom Tarasenko March 27, 2013, 3:16 p.m. UTC | #1 
On Fri, Aug 10, 2012 at 6:47 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
>
> Now all major device models (IDE, SCSI, virtio) can choose between
> writethrough and writeback at run-time, and virtio will even revert
> to writethrough if the guest is not capable of sending flushes.  So
> we can change the default to writeback at last.
>
> Tested, for lack of a better idea, with a breakpoint on bdrv_open
> and all cache choices one by one.

This patch breaks shutting down of a sparc32 guest (or at least the
Debian-4 image I have):

$ sparc-softmmu/qemu-system-sparc -M SS-5 -nographic -hda  ../disk-debian-4
[...]
Debian GNU/Linux 4.0 debian ttyS0

debian login: root
Password:
Linux debian 2.6.18-6-sparc32 #1 Tue Nov 10 00:31:37 UTC 2009 sparc
# poweroff
[...]
Will now halt.
Synchronizing SCSI cache for disk sda:
esp0: Aborting command
esp0: dumping state
esp0: dma -- cond_reg<a4400010> addr<f000000b>
esp0: SW [sreg<03> sstep<04> ireg<10>]
esp0: HW reread [sreg<03> sstep<00> ireg<08>]
esp0: current command [tgt<00> lun<00> pphase<MSGINDONE> cphase<CLUELESS>]
esp0: disconnected
esp0: Aborting command
esp0: dumping state
esp0: dma -- cond_reg<a4400010> addr<f000000b>
esp0: SW [sreg<03> sstep<04> ireg<10>]
esp0: HW reread [sreg<03> sstep<04> ireg<00>]
esp0: current command [tgt<00> lun<00> pphase<UNISSUED> cphase<UNISSUED>]
esp0: disconnected
esp0: Resetting scsi bus
esp0: SCSI bus reset interrupt
esp0: no command in esp_handle()
Kernel panic - not syncing: esp_handle: current_SC == penguin within interrupt!
 <0>Press Stop-A (L1-A) to return to the boot prom


Without the patch, the line "Synchronizing SCSI cache for disk sda"
doesn't come up, so the patch probably just unveils a bug somewhere
else (esp?).

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  blockdev.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index 8669142..7c83baa 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -377,6 +377,7 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>         }
>      }
>
> +    bdrv_flags |= BDRV_O_CACHE_WB;
>      if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
>          if (bdrv_parse_cache_flags(buf, &bdrv_flags) != 0) {
>              error_report("invalid cache option");
> --
> 1.7.6.5
>
>
Paolo Bonzini March 27, 2013, 3:19 p.m. UTC | #2 
Il 27/03/2013 16:16, Artyom Tarasenko ha scritto:
> This patch breaks shutting down of a sparc32 guest (or at least the
> Debian-4 image I have):
> 
> $ sparc-softmmu/qemu-system-sparc -M SS-5 -nographic -hda  ../disk-debian-4
> [...]
> Debian GNU/Linux 4.0 debian ttyS0
> 
> debian login: root
> Password:
> Linux debian 2.6.18-6-sparc32 #1 Tue Nov 10 00:31:37 UTC 2009 sparc
> # poweroff
> [...]
> Will now halt.
> Synchronizing SCSI cache for disk sda:
> esp0: Aborting command
> esp0: dumping state
> esp0: dma -- cond_reg<a4400010> addr<f000000b>
> esp0: SW [sreg<03> sstep<04> ireg<10>]
> esp0: HW reread [sreg<03> sstep<00> ireg<08>]
> esp0: current command [tgt<00> lun<00> pphase<MSGINDONE> cphase<CLUELESS>]
> esp0: disconnected
> esp0: Aborting command
> esp0: dumping state
> esp0: dma -- cond_reg<a4400010> addr<f000000b>
> esp0: SW [sreg<03> sstep<04> ireg<10>]
> esp0: HW reread [sreg<03> sstep<04> ireg<00>]
> esp0: current command [tgt<00> lun<00> pphase<UNISSUED> cphase<UNISSUED>]
> esp0: disconnected
> esp0: Resetting scsi bus
> esp0: SCSI bus reset interrupt
> esp0: no command in esp_handle()
> Kernel panic - not syncing: esp_handle: current_SC == penguin within interrupt!
>  <0>Press Stop-A (L1-A) to return to the boot prom
> 
> 
> Without the patch, the line "Synchronizing SCSI cache for disk sda"
> doesn't come up, so the patch probably just unveils a bug somewhere
> else (esp?).

It doesn't come up because, with a writethrough cache, there is no need
to flush the cache.  The bug should be reproducible before this patch
with -drive file=../disk-debian-4,cache=writeback.

Paolo
diff mbox

Patch

diff --git a/blockdev.c b/blockdev.c
index 8669142..7c83baa 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -377,6 +377,7 @@  DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
 	}
     }
 
+    bdrv_flags |= BDRV_O_CACHE_WB;
     if ((buf = qemu_opt_get(opts, "cache")) != NULL) {
         if (bdrv_parse_cache_flags(buf, &bdrv_flags) != 0) {
             error_report("invalid cache option");