diff options
author | Peter Maydell <peter.maydell@linaro.org> | 2022-01-14 15:56:30 +0000 |
---|---|---|
committer | Peter Maydell <peter.maydell@linaro.org> | 2022-01-14 15:56:30 +0000 |
commit | 1cd2ad11d37c48f284f557954e1df675b126264c (patch) | |
tree | 09d75bcd7b586c4aa78cef09364e4020a573a8d4 | |
parent | 0b3f07ebf2954d28debd7493fef551152e08875b (diff) | |
parent | e5e748739562268ef4063ee77bf53ad7040b25c7 (diff) |
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches
- qemu-storage-daemon: Add vhost-user-blk help
- block-backend: Fix use-after-free for BDS pointers after aio_poll()
- qemu-img: Fix sparseness of output image with unaligned ranges
- vvfat: Fix crashes in read-write mode
- Fix device deletion events with -device JSON syntax
- Code cleanups
# gpg: Signature made Fri 14 Jan 2022 13:50:16 GMT
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream:
iotests/testrunner.py: refactor test_field_width
block: drop BLK_PERM_GRAPH_MOD
qemu-img: make is_allocated_sectors() more efficient
iotests: Test qemu-img convert of zeroed data cluster
vvfat: Fix vvfat_write() for writes before the root directory
vvfat: Fix size of temporary qcow file
iotests/308: Fix for CAP_DAC_OVERRIDE
iotests/stream-error-on-reset: New test
block-backend: prevent dangling BDS pointers across aio_poll()
qapi/block: Restrict vhost-user-blk to CONFIG_VHOST_USER_BLK_SERVER
qemu-storage-daemon: Add vhost-user-blk help
docs: Correct 'vhost-user-blk' spelling
softmmu: fix device deletion events with -device JSON syntax
include/sysemu/blockdev.h: remove drive_get_max_devs
include/sysemu/blockdev.h: remove drive_mark_claimed_by_board and inline drive_def
block_int: make bdrv_backing_overridden static
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
28 files changed, 307 insertions, 104 deletions
@@ -103,6 +103,8 @@ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, static void bdrv_reopen_commit(BDRVReopenState *reopen_state); static void bdrv_reopen_abort(BDRVReopenState *reopen_state); +static bool bdrv_backing_overridden(BlockDriverState *bs); + /* If non-zero, use only whitelisted block drivers */ static int use_bdrv_whitelist; @@ -2483,7 +2485,6 @@ char *bdrv_perm_names(uint64_t perm) { BLK_PERM_WRITE, "write" }, { BLK_PERM_WRITE_UNCHANGED, "write unchanged" }, { BLK_PERM_RESIZE, "resize" }, - { BLK_PERM_GRAPH_MOD, "change children" }, { 0, NULL } }; @@ -2599,8 +2600,7 @@ static void bdrv_default_perms_for_cow(BlockDriverState *bs, BdrvChild *c, shared = 0; } - shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD | - BLK_PERM_WRITE_UNCHANGED; + shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED; if (bs->open_flags & BDRV_O_INACTIVE) { shared |= BLK_PERM_WRITE | BLK_PERM_RESIZE; @@ -2718,7 +2718,6 @@ uint64_t bdrv_qapi_perm_to_blk_perm(BlockPermission qapi_perm) [BLOCK_PERMISSION_WRITE] = BLK_PERM_WRITE, [BLOCK_PERMISSION_WRITE_UNCHANGED] = BLK_PERM_WRITE_UNCHANGED, [BLOCK_PERMISSION_RESIZE] = BLK_PERM_RESIZE, - [BLOCK_PERMISSION_GRAPH_MOD] = BLK_PERM_GRAPH_MOD, }; QEMU_BUILD_BUG_ON(ARRAY_SIZE(permissions) != BLOCK_PERMISSION__MAX); @@ -5544,8 +5543,6 @@ int bdrv_drop_intermediate(BlockDriverState *top, BlockDriverState *base, update_inherits_from = bdrv_inherits_from_recursive(base, explicit_top); /* success - we can delete the intermediate states, and link top->base */ - /* TODO Check graph modification op blockers (BLK_PERM_GRAPH_MOD) once - * we've figured out how they should work. */ if (!backing_file_str) { bdrv_refresh_filename(base); backing_file_str = base->filename; @@ -7475,7 +7472,7 @@ static bool append_strong_runtime_options(QDict *d, BlockDriverState *bs) /* Note: This function may return false positives; it may return true * even if opening the backing file specified by bs's image header * would result in exactly bs->backing. */ -bool bdrv_backing_overridden(BlockDriverState *bs) +static bool bdrv_backing_overridden(BlockDriverState *bs) { if (bs->backing) { return strcmp(bs->auto_backing_file, diff --git a/block/block-backend.c b/block/block-backend.c index 12ef80ea17..23e727199b 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -822,16 +822,22 @@ BlockBackend *blk_by_public(BlockBackendPublic *public) void blk_remove_bs(BlockBackend *blk) { ThrottleGroupMember *tgm = &blk->public.throttle_group_member; - BlockDriverState *bs; BdrvChild *root; notifier_list_notify(&blk->remove_bs_notifiers, blk); if (tgm->throttle_state) { - bs = blk_bs(blk); + BlockDriverState *bs = blk_bs(blk); + + /* + * Take a ref in case blk_bs() changes across bdrv_drained_begin(), for + * example, if a temporary filter node is removed by a blockjob. + */ + bdrv_ref(bs); bdrv_drained_begin(bs); throttle_group_detach_aio_context(tgm); throttle_group_attach_aio_context(tgm, qemu_get_aio_context()); bdrv_drained_end(bs); + bdrv_unref(bs); } blk_update_root_state(blk); @@ -1705,6 +1711,7 @@ void blk_drain(BlockBackend *blk) BlockDriverState *bs = blk_bs(blk); if (bs) { + bdrv_ref(bs); bdrv_drained_begin(bs); } @@ -1714,6 +1721,7 @@ void blk_drain(BlockBackend *blk) if (bs) { bdrv_drained_end(bs); + bdrv_unref(bs); } } @@ -2044,10 +2052,13 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context, int ret; if (bs) { + bdrv_ref(bs); + if (update_root_node) { ret = bdrv_child_try_set_aio_context(bs, new_context, blk->root, errp); if (ret < 0) { + bdrv_unref(bs); return ret; } } @@ -2057,6 +2068,8 @@ static int blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context, throttle_group_attach_aio_context(tgm, new_context); bdrv_drained_end(bs); } + + bdrv_unref(bs); } blk->ctx = new_context; @@ -2326,11 +2339,13 @@ void blk_io_limits_disable(BlockBackend *blk) ThrottleGroupMember *tgm = &blk->public.throttle_group_member; assert(tgm->throttle_state); if (bs) { + bdrv_ref(bs); bdrv_drained_begin(bs); } throttle_group_unregister_tgm(tgm); if (bs) { bdrv_drained_end(bs); + bdrv_unref(bs); } } diff --git a/block/commit.c b/block/commit.c index 10cc5ff451..b1fc7b908b 100644 --- a/block/commit.c +++ b/block/commit.c @@ -370,7 +370,6 @@ void commit_start(const char *job_id, BlockDriverState *bs, s->base = blk_new(s->common.job.aio_context, base_perms, BLK_PERM_CONSISTENT_READ - | BLK_PERM_GRAPH_MOD | BLK_PERM_WRITE_UNCHANGED); ret = blk_insert_bs(s->base, base, errp); if (ret < 0) { diff --git a/block/mirror.c b/block/mirror.c index 959e3dfbd6..69b2c1c697 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1139,10 +1139,7 @@ static void mirror_complete(Job *job, Error **errp) replace_aio_context = bdrv_get_aio_context(s->to_replace); aio_context_acquire(replace_aio_context); - /* TODO Translate this into permission system. Current definition of - * GRAPH_MOD would require to request it for the parents; they might - * not even be BlockDriverStates, however, so a BdrvChild can't address - * them. May need redefinition of GRAPH_MOD. */ + /* TODO Translate this into child freeze system. */ error_setg(&s->replace_blocker, "block device is in use by block-job-complete"); bdrv_op_block_all(s->to_replace, s->replace_blocker); @@ -1666,7 +1663,7 @@ static BlockJob *mirror_start_job( s = block_job_create(job_id, driver, NULL, mirror_top_bs, BLK_PERM_CONSISTENT_READ, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | - BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed, + BLK_PERM_WRITE, speed, creation_flags, cb, opaque, errp); if (!s) { goto fail; @@ -1710,9 +1707,7 @@ static BlockJob *mirror_start_job( target_perms |= BLK_PERM_RESIZE; } - target_shared_perms |= BLK_PERM_CONSISTENT_READ - | BLK_PERM_WRITE - | BLK_PERM_GRAPH_MOD; + target_shared_perms |= BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE; } else if (bdrv_chain_contains(bs, bdrv_skip_filters(target))) { /* * We may want to allow this in the future, but it would @@ -1723,10 +1718,6 @@ static BlockJob *mirror_start_job( goto fail; } - if (backing_mode != MIRROR_LEAVE_BACKING_CHAIN) { - target_perms |= BLK_PERM_GRAPH_MOD; - } - s->target = blk_new(s->common.job.aio_context, target_perms, target_shared_perms); ret = blk_insert_bs(s->target, target, errp); diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c index 2ac4aedfff..bfb3c043a0 100644 --- a/block/monitor/block-hmp-cmds.c +++ b/block/monitor/block-hmp-cmds.c @@ -101,7 +101,7 @@ void hmp_drive_add(Monitor *mon, const QDict *qdict) return; } - opts = drive_def(optstr); + opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), optstr, false); if (!opts) return; diff --git a/block/vvfat.c b/block/vvfat.c index 5dacc6cfac..b2b58d93b8 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -882,7 +882,7 @@ static int read_directory(BDRVVVFATState* s, int mapping_index) return 0; } -static inline uint32_t sector2cluster(BDRVVVFATState* s,off_t sector_num) +static inline int32_t sector2cluster(BDRVVVFATState* s,off_t sector_num) { return (sector_num - s->offset_to_root_dir) / s->sectors_per_cluster; } @@ -1230,6 +1230,7 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, dirname, cyls, heads, secs)); s->sector_count = cyls * heads * secs - s->offset_to_bootsector; + bs->total_sectors = cyls * heads * secs; if (qemu_opt_get_bool(opts, "rw", false)) { if (!bdrv_is_read_only(bs)) { @@ -1250,8 +1251,6 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, } } - bs->total_sectors = cyls * heads * secs; - if (init_directories(s, dirname, heads, secs, errp)) { ret = -EIO; goto fail; @@ -2982,6 +2981,7 @@ static int vvfat_write(BlockDriverState *bs, int64_t sector_num, { BDRVVVFATState *s = bs->opaque; int i, ret; + int first_cluster, last_cluster; DLOG(checkpoint()); @@ -3000,9 +3000,20 @@ DLOG(checkpoint()); if (sector_num < s->offset_to_fat) return -1; - for (i = sector2cluster(s, sector_num); - i <= sector2cluster(s, sector_num + nb_sectors - 1);) { - mapping_t* mapping = find_mapping_for_cluster(s, i); + /* + * Values will be negative for writes to the FAT, which is located before + * the root directory. + */ + first_cluster = sector2cluster(s, sector_num); + last_cluster = sector2cluster(s, sector_num + nb_sectors - 1); + + for (i = first_cluster; i <= last_cluster;) { + mapping_t *mapping = NULL; + + if (i >= 0) { + mapping = find_mapping_for_cluster(s, i); + } + if (mapping) { if (mapping->read_only) { fprintf(stderr, "Tried to write to write-protected file %s\n", @@ -3042,8 +3053,9 @@ DLOG(checkpoint()); } } i = mapping->end; - } else + } else { i++; + } } /* @@ -3057,10 +3069,11 @@ DLOG(fprintf(stderr, "Write to qcow backend: %d + %d\n", (int)sector_num, nb_sec return ret; } - for (i = sector2cluster(s, sector_num); - i <= sector2cluster(s, sector_num + nb_sectors - 1); i++) - if (i >= 0) + for (i = first_cluster; i <= last_cluster; i++) { + if (i >= 0) { s->used_clusters[i] |= USED_ALLOCATED; + } + } DLOG(checkpoint()); /* TODO: add timeout */ @@ -3147,8 +3160,8 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) } opts = qemu_opts_create(bdrv_qcow->create_opts, NULL, 0, &error_abort); - qemu_opt_set_number(opts, BLOCK_OPT_SIZE, s->sector_count * 512, - &error_abort); + qemu_opt_set_number(opts, BLOCK_OPT_SIZE, + bs->total_sectors * BDRV_SECTOR_SIZE, &error_abort); qemu_opt_set(opts, BLOCK_OPT_BACKING_FILE, "fat:", &error_abort); ret = bdrv_create(bdrv_qcow, s->qcow_filename, opts, errp); diff --git a/blockdev.c b/blockdev.c index b5ff9b854e..8197165bb5 100644 --- a/blockdev.c +++ b/blockdev.c @@ -168,23 +168,6 @@ void blockdev_auto_del(BlockBackend *blk) } } -/** - * Returns the current mapping of how many units per bus - * a particular interface can support. - * - * A positive integer indicates n units per bus. - * 0 implies the mapping has not been established. - * -1 indicates an invalid BlockInterfaceType was given. - */ -int drive_get_max_devs(BlockInterfaceType type) -{ - if (type >= IF_IDE && type < IF_COUNT) { - return if_max_devs[type]; - } - - return -1; -} - static int drive_index_to_bus_id(BlockInterfaceType type, int index) { int max_devs = if_max_devs[type]; @@ -197,17 +180,12 @@ static int drive_index_to_unit_id(BlockInterfaceType type, int index) return max_devs ? index % max_devs : index; } -QemuOpts *drive_def(const char *optstr) -{ - return qemu_opts_parse_noisily(qemu_find_opts("drive"), optstr, false); -} - QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, const char *optstr) { QemuOpts *opts; - opts = drive_def(optstr); + opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), optstr, false); if (!opts) { return NULL; } diff --git a/docs/tools/qemu-storage-daemon.rst b/docs/tools/qemu-storage-daemon.rst index 3e5a9dc032..9b0eaba6e5 100644 --- a/docs/tools/qemu-storage-daemon.rst +++ b/docs/tools/qemu-storage-daemon.rst @@ -201,7 +201,7 @@ Export raw image file ``disk.img`` over NBD UNIX domain socket ``nbd.sock``:: --nbd-server addr.type=unix,addr.path=nbd.sock \ --export type=nbd,id=export,node-name=disk,writable=on -Export a qcow2 image file ``disk.qcow2`` as a vhosts-user-blk device over UNIX +Export a qcow2 image file ``disk.qcow2`` as a vhost-user-blk device over UNIX domain socket ``vhost-user-blk.sock``:: $ qemu-storage-daemon \ diff --git a/hw/block/block.c b/hw/block/block.c index d47ebf005a..25f45df723 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -171,8 +171,7 @@ bool blkconf_apply_backend_options(BlockConf *conf, bool readonly, perm |= BLK_PERM_WRITE; } - shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | - BLK_PERM_GRAPH_MOD; + shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED; if (resizable) { shared_perm |= BLK_PERM_RESIZE; } diff --git a/include/block/block.h b/include/block/block.h index e5dd22b034..9d4050220b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -269,12 +269,13 @@ enum { BLK_PERM_RESIZE = 0x08, /** - * This permission is required to change the node that this BdrvChild - * points to. + * There was a now-removed bit BLK_PERM_GRAPH_MOD, with value of 0x10. QEMU + * 6.1 and earlier may still lock the corresponding byte in block/file-posix + * locking. So, implementing some new permission should be very careful to + * not interfere with this old unused thing. */ - BLK_PERM_GRAPH_MOD = 0x10, - BLK_PERM_ALL = 0x1f, + BLK_PERM_ALL = 0x0f, DEFAULT_PERM_PASSTHROUGH = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE diff --git a/include/block/block_int.h b/include/block/block_int.h index f4c75e8ba9..27008cfb22 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1122,9 +1122,6 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, QDict *options); -bool bdrv_backing_overridden(BlockDriverState *bs); - - /** * bdrv_add_aio_context_notifier: * diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index a750f99b79..f9fb54d437 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -45,13 +45,10 @@ BlockBackend *blk_by_legacy_dinfo(DriveInfo *dinfo); void override_max_devs(BlockInterfaceType type, int max_devs); DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); -void drive_mark_claimed_by_board(void); void drive_check_orphaned(void); DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); int drive_get_max_bus(BlockInterfaceType type); -int drive_get_max_devs(BlockInterfaceType type); -QemuOpts *drive_def(const char *optstr); QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file, const char *optstr); DriveInfo *drive_new(QemuOpts *arg, BlockInterfaceType block_default_type, diff --git a/qapi/block-core.json b/qapi/block-core.json index bd0b285245..9a5a3641d0 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1878,14 +1878,11 @@ # # @resize: This permission is required to change the size of a block node. # -# @graph-mod: This permission is required to change the node that this -# BdrvChild points to. -# # Since: 4.0 ## { 'enum': 'BlockPermission', - 'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize', - 'graph-mod' ] } + 'data': [ 'consistent-read', 'write', 'write-unchanged', 'resize' ] } + ## # @XDbgBlockGraphEdge: # diff --git a/qapi/block-export.json b/qapi/block-export.json index c1b92ce1c1..f9ce79a974 100644 --- a/qapi/block-export.json +++ b/qapi/block-export.json @@ -277,7 +277,8 @@ # Since: 4.2 ## { 'enum': 'BlockExportType', - 'data': [ 'nbd', 'vhost-user-blk', + 'data': [ 'nbd', + { 'name': 'vhost-user-blk', 'if': 'CONFIG_VHOST_USER_BLK_SERVER' }, { 'name': 'fuse', 'if': 'CONFIG_FUSE' } ] } ## @@ -319,7 +320,8 @@ 'discriminator': 'type', 'data': { 'nbd': 'BlockExportOptionsNbd', - 'vhost-user-blk': 'BlockExportOptionsVhostUserBlk', + 'vhost-user-blk': { 'type': 'BlockExportOptionsVhostUserBlk', + 'if': 'CONFIG_VHOST_USER_BLK_SERVER' }, 'fuse': { 'type': 'BlockExportOptionsFuse', 'if': 'CONFIG_FUSE' } } } diff --git a/qapi/qdev.json b/qapi/qdev.json index 69656b14df..26cd10106b 100644 --- a/qapi/qdev.json +++ b/qapi/qdev.json @@ -44,6 +44,9 @@ # @json-cli: If present, the "-device" command line option supports JSON # syntax with a structure identical to the arguments of this # command. +# @json-cli-hotplug: If present, the "-device" command line option supports JSON +# syntax without the reference counting leak that broke +# hot-unplug # # Notes: # @@ -74,7 +77,7 @@ { 'command': 'device_add', 'data': {'driver': 'str', '*bus': 'str', '*id': 'str'}, 'gen': false, # so we can get the additional arguments - 'features': ['json-cli'] } + 'features': ['json-cli', 'json-cli-hotplug'] } ## # @device_del: diff --git a/qemu-img.c b/qemu-img.c index 21ba1e6800..6fe2466032 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1171,19 +1171,34 @@ static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum, } } + if (i == n) { + /* + * The whole buf is the same. + * No reason to split it into chunks, so return now. + */ + *pnum = i; + return !is_zero; + } + tail = (sector_num + i) & (alignment - 1); if (tail) { if (is_zero && i <= tail) { - /* treat unallocated areas which only consist - * of a small tail as allocated. */ + /* + * For sure next sector after i is data, and it will rewrite this + * tail anyway due to RMW. So, let's just write data now. + */ is_zero = false; } if (!is_zero) { - /* align up end offset of allocated areas. */ + /* If possible, align up end offset of allocated areas. */ i += alignment - tail; i = MIN(i, n); } else { - /* align down end offset of zero areas. */ + /* + * For sure next sector after i is data, and it will rewrite this + * tail anyway due to RMW. Better is avoid RMW and write zeroes up + * to aligned bound. + */ i -= tail; } } diff --git a/scripts/render_block_graph.py b/scripts/render_block_graph.py index da6acf050d..42288a3cfb 100755 --- a/scripts/render_block_graph.py +++ b/scripts/render_block_graph.py @@ -35,7 +35,6 @@ def perm(arr): s = 'w' if 'write' in arr else '_' s += 'r' if 'consistent-read' in arr else '_' s += 'u' if 'write-unchanged' in arr else '_' - s += 'g' if 'graph-mod' in arr else '_' s += 's' if 'resize' in arr else '_' return s diff --git a/softmmu/vl.c b/softmmu/vl.c index a8cad43691..5e1b35ba48 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2684,6 +2684,7 @@ static void qemu_create_cli_devices(void) qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, &error_fatal); QTAILQ_FOREACH(opt, &device_opts, next) { + DeviceState *dev; loc_push_restore(&opt->loc); /* * TODO Eventually we should call qmp_device_add() here to make sure it @@ -2692,7 +2693,8 @@ static void qemu_create_cli_devices(void) * from the start, so call qdev_device_add_from_qdict() directly for * now. */ - qdev_device_add_from_qdict(opt->opts, true, &error_fatal); + dev = qdev_device_add_from_qdict(opt->opts, true, &error_fatal); + object_unref(OBJECT(dev)); loc_pop(&opt->loc); } rom_reset_order_override(); @@ -2887,7 +2889,9 @@ void qemu_init(int argc, char **argv, char **envp) break; } case QEMU_OPTION_drive: - if (drive_def(optarg) == NULL) { + opts = qemu_opts_parse_noisily(qemu_find_opts("drive"), + optarg, false); + if (opts == NULL) { exit(1); } break; diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c index 52cf17e8ac..9d76d1114d 100644 --- a/storage-daemon/qemu-storage-daemon.c +++ b/storage-daemon/qemu-storage-daemon.c @@ -104,6 +104,19 @@ static void help(void) " export the specified block node over FUSE\n" "\n" #endif /* CONFIG_FUSE */ +#ifdef CONFIG_VHOST_USER_BLK_SERVER +" --export [type=]vhost-user-blk,id=<id>,node-name=<node-name>,\n" +" addr.type=unix,addr.path=<socket-path>[,writable=on|off]\n" +" [,logical-block-size=<block-size>][,num-queues=<num-queues>]\n" +" export the specified block node as a\n" +" vhost-user-blk device over UNIX domain socket\n" +" --export [type=]vhost-user-blk,id=<id>,node-name=<node-name>,\n" +" fd,addr.str=<fd>[,writable=on|off]\n" +" [,logical-block-size=<block-size>][,num-queues=<num-queues>]\n" +" export the specified block node as a\n" +" vhost-user-blk device over file descriptor\n" +"\n" +#endif /* CONFIG_VHOST_USER_BLK_SERVER */ " --monitor [chardev=]name[,mode=control][,pretty[=on|off]]\n" " configure a QMP monitor\n" "\n" diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122 index efb260d822..be0f6b79e5 100755 --- a/tests/qemu-iotests/122 +++ b/tests/qemu-iotests/122 @@ -251,6 +251,7 @@ $QEMU_IO -c "write -P 0 0 64k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_test $QEMU_IO -c "write 0 1k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir $QEMU_IO -c "write 8k 1k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir $QEMU_IO -c "write 17k 1k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir +$QEMU_IO -c "write -P 0 65k 1k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir for min_sparse in 4k 8k; do echo diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out index 8fbdac2b39..e18766e167 100644 --- a/tests/qemu-iotests/122.out +++ b/tests/qemu-iotests/122.out @@ -192,6 +192,8 @@ wrote 1024/1024 bytes at offset 8192 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) wrote 1024/1024 bytes at offset 17408 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 1024/1024 bytes at offset 66560 +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) convert -S 4k [{ "start": 0, "length": 4096, "depth": 0, "present": true, "zero": false, "data": true, "offset": OFFSET}, diff --git a/tests/qemu-iotests/273.out b/tests/qemu-iotests/273.out index 4e840b6730..6a74a8138b 100644 --- a/tests/qemu-iotests/273.out +++ b/tests/qemu-iotests/273.out @@ -204,7 +204,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev "name": "file", "parent": 5, "shared-perm": [ - "graph-mod", "write-unchanged", "consistent-read" ], @@ -219,7 +218,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev "name": "backing", "parent": 5, "shared-perm": [ - "graph-mod", "resize", "write-unchanged", "write", @@ -233,7 +231,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev "name": "file", "parent": 3, "shared-perm": [ - "graph-mod", "write-unchanged", "consistent-read" ], @@ -246,7 +243,6 @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev "name": "backing", "parent": 3, "shared-perm": [ - "graph-mod", "resize", "write-unchanged", "write", diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308 index 2e3f8f4282..bde4aac2fa 100755 --- a/tests/qemu-iotests/308 +++ b/tests/qemu-iotests/308 @@ -230,8 +230,29 @@ echo '=== Writable export ===' fuse_export_add 'export-mp' "'mountpoint': '$EXT_MP', 'writable': true" # Check that writing to the read-only export fails -$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" 2>&1 \ - | _filter_qemu_io | _filter_testdir | _filter_imgfmt +output=$($QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" 2>&1 \ + | _filter_qemu_io | _filter_testdir | _filter_imgfmt) + +# Expected reference output: Opening the file fails because it has no +# write permission +reference="Could not open 'TEST_DIR/t.IMGFMT': Permission denied" + +if echo "$output" | grep -q "$reference"; then + echo "Writing to read-only export failed: OK" +elif echo "$output" | grep -q "write failed: Permission denied"; then + # With CAP_DAC_OVERRIDE (e.g. when running this test as root), the export + # can be opened regardless of its file permissions, but writing will then + # fail. This is not the result for which we want to test, so count this as + # a SKIP. + _casenotrun "Opening RO export as R/W succeeded, perhaps because of" \ + "CAP_DAC_OVERRIDE" + + # Still, write this to the reference output to make the test pass + echo "Writing to read-only export failed: OK" +else + echo "Writing to read-only export failed: ERROR" + echo "$output" +fi # But here it should work $QEMU_IO -f raw -c 'write -P 42 1M 64k' "$EXT_MP" | _filter_qemu_io diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out index fc47bb11a2..e4467a10cf 100644 --- a/tests/qemu-iotests/308.out +++ b/tests/qemu-iotests/308.out @@ -95,7 +95,7 @@ virtual size: 0 B (0 bytes) 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true } } {"return": {}} -qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open 'TEST_DIR/t.IMGFMT': Permission denied +Writing to read-only export failed: OK wrote 65536/65536 bytes at offset 1048576 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) wrote 65536/65536 bytes at offset 1048576 diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py index 0feaa396d0..15788f919e 100644 --- a/tests/qemu-iotests/testrunner.py +++ b/tests/qemu-iotests/testrunner.py @@ -174,19 +174,17 @@ class TestRunner(ContextManager['TestRunner']): def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None: self._stack.close() - def test_print_one_line(self, test: str, starttime: str, + def test_print_one_line(self, test: str, + test_field_width: int, + starttime: str, endtime: Optional[str] = None, status: str = '...', lasttime: Optional[float] = None, thistime: Optional[float] = None, description: str = '', - test_field_width: Optional[int] = None, end: str = '\n') -> None: """ Print short test info before/after test run """ test = os.path.basename(test) - if test_field_width is None: - test_field_width = 8 - if self.makecheck and status != '...': if status and status != 'pass': status = f' [{status}]' @@ -328,7 +326,7 @@ class TestRunner(ContextManager['TestRunner']): casenotrun=casenotrun) def run_test(self, test: str, - test_field_width: Optional[int] = None, + test_field_width: int, mp: bool = False) -> TestResult: """ Run one test and print short status @@ -347,20 +345,21 @@ class TestRunner(ContextManager['TestRunner']): if not self.makecheck: self.test_print_one_line(test=test, + test_field_width=test_field_width, status = 'started' if mp else '...', starttime=start, lasttime=last_el, - end = '\n' if mp else '\r', - test_field_width=test_field_width) + end = '\n' if mp else '\r') res = self.do_run_test(test, mp) end = datetime.datetime.now().strftime('%H:%M:%S') - self.test_print_one_line(test=test, status=res.status, + self.test_print_one_line(test=test, + test_field_width=test_field_width, + status=res.status, starttime=start, endtime=end, lasttime=last_el, thistime=res.elapsed, - description=res.description, - test_field_width=test_field_width) + description=res.description) if res.casenotrun: print(res.casenotrun) diff --git a/tests/qemu-iotests/tests/stream-error-on-reset b/tests/qemu-iotests/tests/stream-error-on-reset new file mode 100755 index 0000000000..7eaedb24d7 --- /dev/null +++ b/tests/qemu-iotests/tests/stream-error-on-reset @@ -0,0 +1,140 @@ +#!/usr/bin/env python3 +# group: rw quick +# +# Test what happens when a stream job completes in a blk_drain(). +# +# Copyright (C) 2022 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. +# + +import os +import iotests +from iotests import imgfmt, qemu_img_create, qemu_io_silent, QMPTestCase + + +image_size = 1 * 1024 * 1024 +data_size = 64 * 1024 +base = os.path.join(iotests.test_dir, 'base.img') +top = os.path.join(iotests.test_dir, 'top.img') + + +# We want to test completing a stream job in a blk_drain(). +# +# The blk_drain() we are going to use is a virtio-scsi device resetting, +# which we can trigger by resetting the system. +# +# In order to have the block job complete on drain, we (1) throttle its +# base image so we can start the drain after it has begun, but before it +# completes, and (2) make it encounter an I/O error on the ensuing write. +# (If it completes regularly, the completion happens after the drain for +# some reason.) + +class TestStreamErrorOnReset(QMPTestCase): + def setUp(self) -> None: + """ + Create two images: + - base image {base} with {data_size} bytes allocated + - top image {top} without any data allocated + + And the following VM configuration: + - base image throttled to {data_size} + - top image with a blkdebug configuration so the first write access + to it will result in an error + - top image is attached to a virtio-scsi device + """ + assert qemu_img_create('-f', imgfmt, base, str(image_size)) == 0 + assert qemu_io_silent('-c', f'write 0 {data_size}', base) == 0 + assert qemu_img_create('-f', imgfmt, top, str(image_size)) == 0 + + self.vm = iotests.VM() + self.vm.add_args('-accel', 'tcg') # Make throttling work properly + self.vm.add_object(self.vm.qmp_to_opts({ + 'qom-type': 'throttle-group', + 'id': 'thrgr', + 'x-bps-total': str(data_size) + })) + self.vm.add_blockdev(self.vm.qmp_to_opts({ + 'driver': imgfmt, + 'node-name': 'base', + 'file': { + 'driver': 'throttle', + 'throttle-group': 'thrgr', + 'file': { + 'driver': 'file', + 'filename': base + } + } + })) + self.vm.add_blockdev(self.vm.qmp_to_opts({ + 'driver': imgfmt, + 'node-name': 'top', + 'file': { + 'driver': 'blkdebug', + 'node-name': 'top-blkdebug', + 'inject-error': [{ + 'event': 'pwritev', + 'immediately': 'true', + 'once': 'true' + }], + 'image': { + 'driver': 'file', + 'filename': top + } + }, + 'backing': 'base' + })) + self.vm.add_device(self.vm.qmp_to_opts({ + 'driver': 'virtio-scsi', + 'id': 'vscsi' + })) + self.vm.add_device(self.vm.qmp_to_opts({ + 'driver': 'scsi-hd', + 'bus': 'vscsi.0', + 'drive': 'top' + })) + self.vm.launch() + + def tearDown(self) -> None: + self.vm.shutdown() + os.remove(top) + os.remove(base) + + def test_stream_error_on_reset(self) -> None: + # Launch a stream job, which will take at least a second to + # complete, because the base image is throttled (so we can + # get in between it having started and it having completed) + res = self.vm.qmp('block-stream', job_id='stream', device='top') + self.assert_qmp(res, 'return', {}) + + while True: + ev = self.vm.event_wait('JOB_STATUS_CHANGE') + if ev['data']['status'] == 'running': + # Once the stream job is running, reset the system, which + # forces the virtio-scsi device to be reset, thus draining + # the stream job, and making it complete. Completing + # inside of that drain should not result in a segfault. + res = self.vm.qmp('system_reset') + self.assert_qmp(res, 'return', {}) + elif ev['data']['status'] == 'null': + # The test is done once the job is gone + break + + +if __name__ == '__main__': + # Passes with any format with backing file support, but qed and + # qcow1 do not seem to exercise the used-to-be problematic code + # path, so there is no point in having them in this list + iotests.main(supported_fmts=['qcow2', 'vmdk'], + supported_protocols=['file']) diff --git a/tests/qemu-iotests/tests/stream-error-on-reset.out b/tests/qemu-iotests/tests/stream-error-on-reset.out new file mode 100644 index 0000000000..ae1213e6f8 --- /dev/null +++ b/tests/qemu-iotests/tests/stream-error-on-reset.out @@ -0,0 +1,5 @@ +. +---------------------------------------------------------------------- +Ran 1 tests + +OK diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c index 559d47727a..ad79bd4c14 100644 --- a/tests/qtest/device-plug-test.c +++ b/tests/qtest/device-plug-test.c @@ -77,6 +77,23 @@ static void test_pci_unplug_request(void) qtest_quit(qtest); } +static void test_pci_unplug_json_request(void) +{ + QTestState *qtest = qtest_initf( + "-device '{\"driver\": \"virtio-mouse-pci\", \"id\": \"dev0\"}'"); + + /* + * Request device removal. As the guest is not running, the request won't + * be processed. However during system reset, the removal will be + * handled, removing the device. + */ + device_del(qtest, "dev0"); + system_reset(qtest); + wait_device_deleted_event(qtest, "dev0"); + + qtest_quit(qtest); +} + static void test_ccw_unplug(void) { QTestState *qtest = qtest_initf("-device virtio-balloon-ccw,id=dev0"); @@ -145,6 +162,8 @@ int main(int argc, char **argv) */ qtest_add_func("/device-plug/pci-unplug-request", test_pci_unplug_request); + qtest_add_func("/device-plug/pci-unplug-json-request", + test_pci_unplug_json_request); if (!strcmp(arch, "s390x")) { qtest_add_func("/device-plug/ccw-unplug", |