Re: [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 7615 bytes --]

On 18.03.2016 19:21, Kevin Wolf wrote:
> The previous patches have successively made blk->enable_write_cache the
> true source for the information whether a writethrough mode must be
> implemented. The corresponding BDRV_O_CACHE_WB is only useless baggage
> we're carrying around, so now's the time to remove it.
> 
> At the same time, we remove the 'cache.writeback' option parsing on the
> BDS level as the only effect was setting the BDRV_O_CACHE_WB flag.
> 
> This change requires test cases that explicitly enabled the option to
> drop it. Other than that and the change of the error message when
> writethrough is enabled on the BDS level (from "Can't set writethrough
> mode" to "doesn't support the option"), there should be no change in
> behaviour.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                       | 48 ++-----------------------------------------
>  block/block-backend.c         | 11 ----------
>  block/vvfat.c                 |  3 +--
>  blockdev.c                    | 21 ++-----------------
>  include/block/block.h         |  3 +--
>  qemu-img.c                    |  2 +-
>  qemu-io-cmds.c                |  1 -
>  tests/qemu-iotests/051        |  2 +-
>  tests/qemu-iotests/051.pc.out | 10 ++++-----
>  tests/qemu-iotests/142        |  6 +++---
>  tests/qemu-iotests/142.out    | 36 ++++++++++++++++----------------
>  11 files changed, 34 insertions(+), 109 deletions(-)
> 

[...]

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 7de3754..35ee50b 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -2149,7 +2149,6 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
>      opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
>      qemu_opts_reset(&reopen_opts);
>  
> -    flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;

Well... :-P

>      brq = bdrv_reopen_queue(NULL, bs, opts, flags);
>      bdrv_reopen_multiple(brq, &local_err);
>      if (local_err) {
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 7bfe9ff..88b3d91 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -218,7 +218,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
>  
>  for cache in writeback writethrough unsafe invalid_value; do
>      echo -e "info block\ninfo block file\ninfo block backing\ninfo block backing-file" | \
> -    run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
> +    run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
>  done
>  
>  echo
> diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
> index 73cc15a..ec6d222 100644
> --- a/tests/qemu-iotests/051.pc.out
> +++ b/tests/qemu-iotests/051.pc.out

051.out needs the same changes.

Aside from that: Looks good.

Max

> @@ -239,7 +239,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
>  Testing: -drive driver=null-co,cache=invalid_value
>  QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
>  
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
>  QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
>  drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -259,7 +259,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
>      Cache mode:       writeback, ignore flushes
>  (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>  
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
>  QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
>  drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -279,7 +279,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
>      Cache mode:       writeback, ignore flushes
>  (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>  
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
>  QEMU X.Y.Z monitor - type 'help' for more information
>  (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
>  drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -299,8 +299,8 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
>      Cache mode:       writeback, ignore flushes
>  (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>  
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
>  
>  
>  === Specifying the protocol layer ===


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]