block: Deprecate bdrv_set_read_only() and users

qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio

bdrv_set_read_only() is used by some block drivers to override the
read-only option given by the user. This is not how read-only images
generally work in QEMU: Instead of second guessing what the user really
meant (which currently includes making an image read-only even if the
user didn't only use the default, but explicitly said read-only=off), we
should error out if we can't provide what the user requested.

This adds deprecation warnings to all callers of bdrv_set_read_only() so
that the behaviour can be corrected after the usual deprecation period.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Kevin Wolf 8 years ago 398e6ad0 f66afbe2

+54 -16

7 changed files

expand all

block

bochs.c

cloop.c

dmg.c

rbd.c

vvfat.c

block.c

qapi

block-core.json

block.c

··· 261 261 return 0; 262 262 } 263 263 264 + /* TODO Remove (deprecated since 2.11) 265 + * Block drivers are not supposed to automatically change bs->read_only. 266 + * Instead, they should just check whether they can provide what the user 267 + * explicitly requested and error out if read-write is requested, but they can 268 + * only provide read-only access. */ 264 269 int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp) 265 270 { 266 271 int ret = 0;

+10 -3

block/bochs.c

··· 28 28 #include "block/block_int.h" 29 29 #include "qemu/module.h" 30 30 #include "qemu/bswap.h" 31 + #include "qemu/error-report.h" 31 32 32 33 /**************************************************************/ 33 34 ··· 110 111 return -EINVAL; 111 112 } 112 113 113 - ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */ 114 - if (ret < 0) { 115 - return ret; 114 + if (!bdrv_is_read_only(bs)) { 115 + error_report("Opening bochs images without an explicit read-only=on " 116 + "option is deprecated. Future versions will refuse to " 117 + "open the image instead of automatically marking the " 118 + "image read-only."); 119 + ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */ 120 + if (ret < 0) { 121 + return ret; 122 + } 116 123 } 117 124 118 125 ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs));

+10 -3

block/cloop.c

··· 23 23 */ 24 24 #include "qemu/osdep.h" 25 25 #include "qapi/error.h" 26 + #include "qemu/error-report.h" 26 27 #include "qemu-common.h" 27 28 #include "block/block_int.h" 28 29 #include "qemu/module.h" ··· 72 73 return -EINVAL; 73 74 } 74 75 75 - ret = bdrv_set_read_only(bs, true, errp); 76 - if (ret < 0) { 77 - return ret; 76 + if (!bdrv_is_read_only(bs)) { 77 + error_report("Opening cloop images without an explicit read-only=on " 78 + "option is deprecated. Future versions will refuse to " 79 + "open the image instead of automatically marking the " 80 + "image read-only."); 81 + ret = bdrv_set_read_only(bs, true, errp); 82 + if (ret < 0) { 83 + return ret; 84 + } 78 85 } 79 86 80 87 /* read header */

+9 -3

block/dmg.c

··· 419 419 return -EINVAL; 420 420 } 421 421 422 - ret = bdrv_set_read_only(bs, true, errp); 423 - if (ret < 0) { 424 - return ret; 422 + if (!bdrv_is_read_only(bs)) { 423 + error_report("Opening dmg images without an explicit read-only=on " 424 + "option is deprecated. Future versions will refuse to " 425 + "open the image instead of automatically marking the " 426 + "image read-only."); 427 + ret = bdrv_set_read_only(bs, true, errp); 428 + if (ret < 0) { 429 + return ret; 430 + } 425 431 } 426 432 427 433 block_module_load_one("dmg-bz2");

+10 -4

block/rbd.c

··· 665 665 /* If we are using an rbd snapshot, we must be r/o, otherwise 666 666 * leave as-is */ 667 667 if (s->snap != NULL) { 668 - r = bdrv_set_read_only(bs, true, &local_err); 669 - if (r < 0) { 670 - error_propagate(errp, local_err); 671 - goto failed_open; 668 + if (!bdrv_is_read_only(bs)) { 669 + error_report("Opening rbd snapshots without an explicit " 670 + "read-only=on option is deprecated. Future versions " 671 + "will refuse to open the image instead of " 672 + "automatically marking the image read-only."); 673 + r = bdrv_set_read_only(bs, true, &local_err); 674 + if (r < 0) { 675 + error_propagate(errp, local_err); 676 + goto failed_open; 677 + } 672 678 } 673 679 } 674 680

+5 -1

block/vvfat.c

··· 1259 1259 "Unable to set VVFAT to 'rw' when drive is read-only"); 1260 1260 goto fail; 1261 1261 } 1262 - } else { 1262 + } else if (!bdrv_is_read_only(bs)) { 1263 + error_report("Opening non-rw vvfat images without an explicit " 1264 + "read-only=on option is deprecated. Future versions " 1265 + "will refuse to open the image instead of " 1266 + "automatically marking the image read-only."); 1263 1267 /* read only is the default for safety */ 1264 1268 ret = bdrv_set_read_only(bs, true, &local_err); 1265 1269 if (ret < 0) {

+5 -2

qapi/block-core.json

··· 3134 3134 # This option is required on the top level of blockdev-add. 3135 3135 # @discard: discard-related options (default: ignore) 3136 3136 # @cache: cache-related options 3137 - # @read-only: whether the block device should be read-only 3138 - # (default: false) 3137 + # @read-only: whether the block device should be read-only (default: false). 3138 + # Note that some block drivers support only read-only access, 3139 + # either generally or in certain configurations. In this case, 3140 + # the default value does not work and the option must be 3141 + # specified explicitly. 3139 3142 # @detect-zeroes: detect and optimize zero writes (Since 2.1) 3140 3143 # (default: off) 3141 3144 # @force-share: force share all permission on added nodes.