diff --git a/backends/cryptodev-vhost.c b/backends/cryptodev-vhost.c index 8231e7f1bc..bc13e466b4 100644 --- a/backends/cryptodev-vhost.c +++ b/backends/cryptodev-vhost.c @@ -52,6 +52,7 @@ cryptodev_vhost_init( { int r; CryptoDevBackendVhost *crypto; + Error *local_err = NULL; crypto = g_new(CryptoDevBackendVhost, 1); crypto->dev.max_queues = 1; @@ -66,8 +67,10 @@ cryptodev_vhost_init( /* vhost-user needs vq_index to initiate a specific queue pair */ crypto->dev.vq_index = crypto->cc->queue_index * crypto->dev.nvqs; - r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type, 0); + r = vhost_dev_init(&crypto->dev, options->opaque, options->backend_type, 0, + &local_err); if (r < 0) { + error_report_err(local_err); goto fail; } diff --git a/backends/vhost-user.c b/backends/vhost-user.c index b366610e16..10b39992d2 100644 --- a/backends/vhost-user.c +++ b/backends/vhost-user.c @@ -48,9 +48,9 @@ vhost_user_backend_dev_init(VhostUserBackend *b, VirtIODevice *vdev, b->dev.nvqs = nvqs; b->dev.vqs = g_new0(struct vhost_virtqueue, nvqs); - ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 0); + ret = vhost_dev_init(&b->dev, &b->vhost_user, VHOST_BACKEND_TYPE_USER, 0, + errp); if (ret < 0) { - error_setg_errno(errp, -ret, "vhost initialization failed"); return -1; } diff --git a/block.c b/block.c index 1d37f133a8..acd35cb0cb 100644 --- a/block.c +++ b/block.c @@ -84,12 +84,15 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, static void bdrv_replace_child_noperm(BdrvChild *child, BlockDriverState *new_bs); +static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, + BdrvChild *child, + Transaction *tran); static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs, Transaction *tran); static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, - Transaction *set_backings_tran, Error **errp); + Transaction *change_child_tran, Error **errp); static void bdrv_reopen_commit(BDRVReopenState *reopen_state); static void bdrv_reopen_abort(BDRVReopenState *reopen_state); @@ -2249,12 +2252,14 @@ static TransactionActionDrv bdrv_replace_child_drv = { }; /* - * bdrv_replace_child + * bdrv_replace_child_tran * * Note: real unref of old_bs is done only on commit. + * + * The function doesn't update permissions, caller is responsible for this. */ -static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs, - Transaction *tran) +static void bdrv_replace_child_tran(BdrvChild *child, BlockDriverState *new_bs, + Transaction *tran) { BdrvReplaceChildState *s = g_new(BdrvReplaceChildState, 1); *s = (BdrvReplaceChildState) { @@ -2768,6 +2773,8 @@ static TransactionActionDrv bdrv_attach_child_common_drv = { * @child is saved to a new entry of @tran, so that *@child could be reverted to * NULL on abort(). So referenced variable must live at least until transaction * end. + * + * Function doesn't update permissions, caller is responsible for this. */ static int bdrv_attach_child_common(BlockDriverState *child_bs, const char *child_name, @@ -2846,6 +2853,8 @@ static int bdrv_attach_child_common(BlockDriverState *child_bs, /* * Variable referenced by @child must live at least until transaction end. * (see bdrv_attach_child_common() doc for details) + * + * Function doesn't update permissions, caller is responsible for this. */ static int bdrv_attach_child_noperm(BlockDriverState *parent_bs, BlockDriverState *child_bs, @@ -3111,54 +3120,104 @@ static BdrvChildRole bdrv_backing_role(BlockDriverState *bs) } /* - * Sets the bs->backing link of a BDS. A new reference is created; callers - * which don't need their own reference any more must call bdrv_unref(). + * Sets the bs->backing or bs->file link of a BDS. A new reference is created; + * callers which don't need their own reference any more must call bdrv_unref(). + * + * Function doesn't update permissions, caller is responsible for this. */ -static int bdrv_set_backing_noperm(BlockDriverState *bs, - BlockDriverState *backing_hd, - Transaction *tran, Error **errp) +static int bdrv_set_file_or_backing_noperm(BlockDriverState *parent_bs, + BlockDriverState *child_bs, + bool is_backing, + Transaction *tran, Error **errp) { int ret = 0; - bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) && - bdrv_inherits_from_recursive(backing_hd, bs); + bool update_inherits_from = + bdrv_inherits_from_recursive(child_bs, parent_bs); + BdrvChild *child = is_backing ? parent_bs->backing : parent_bs->file; + BdrvChildRole role; - if (bdrv_is_backing_chain_frozen(bs, child_bs(bs->backing), errp)) { + if (!parent_bs->drv) { + /* + * Node without drv is an object without a class :/. TODO: finally fix + * qcow2 driver to never clear bs->drv and implement format corruption + * handling in other way. + */ + error_setg(errp, "Node corrupted"); + return -EINVAL; + } + + if (child && child->frozen) { + error_setg(errp, "Cannot change frozen '%s' link from '%s' to '%s'", + child->name, parent_bs->node_name, child->bs->node_name); return -EPERM; } - if (bs->backing) { - /* Cannot be frozen, we checked that above */ - bdrv_unset_inherits_from(bs, bs->backing, tran); - bdrv_remove_filter_or_cow_child(bs, tran); + if (is_backing && !parent_bs->drv->is_filter && + !parent_bs->drv->supports_backing) + { + error_setg(errp, "Driver '%s' of node '%s' does not support backing " + "files", parent_bs->drv->format_name, parent_bs->node_name); + return -EINVAL; } - if (!backing_hd) { + if (parent_bs->drv->is_filter) { + role = BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY; + } else if (is_backing) { + role = BDRV_CHILD_COW; + } else { + /* + * We only can use same role as it is in existing child. We don't have + * infrastructure to determine role of file child in generic way + */ + if (!child) { + error_setg(errp, "Cannot set file child to format node without " + "file child"); + return -EINVAL; + } + role = child->role; + } + + if (child) { + bdrv_unset_inherits_from(parent_bs, child, tran); + bdrv_remove_file_or_backing_child(parent_bs, child, tran); + } + + if (!child_bs) { goto out; } - ret = bdrv_attach_child_noperm(bs, backing_hd, "backing", - &child_of_bds, bdrv_backing_role(bs), - &bs->backing, tran, errp); + ret = bdrv_attach_child_noperm(parent_bs, child_bs, + is_backing ? "backing" : "file", + &child_of_bds, role, + is_backing ? &parent_bs->backing : + &parent_bs->file, + tran, errp); if (ret < 0) { return ret; } /* - * If backing_hd was already part of bs's backing chain, and - * inherits_from pointed recursively to bs then let's update it to + * If inherits_from pointed recursively to bs then let's update it to * point directly to bs (else it will become NULL). */ if (update_inherits_from) { - bdrv_set_inherits_from(backing_hd, bs, tran); + bdrv_set_inherits_from(child_bs, parent_bs, tran); } out: - bdrv_refresh_limits(bs, tran, NULL); + bdrv_refresh_limits(parent_bs, tran, NULL); return 0; } +static int bdrv_set_backing_noperm(BlockDriverState *bs, + BlockDriverState *backing_hd, + Transaction *tran, Error **errp) +{ + return bdrv_set_file_or_backing_noperm(bs, backing_hd, true, tran, errp); +} + int bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, Error **errp) { @@ -4089,6 +4148,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) refresh_list = bdrv_topological_dfs(refresh_list, found, state->old_backing_bs); } + if (state->old_file_bs) { + refresh_list = bdrv_topological_dfs(refresh_list, found, + state->old_file_bs); + } } /* @@ -4164,29 +4227,6 @@ int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, return ret; } -static bool bdrv_reopen_can_attach(BlockDriverState *parent, - BdrvChild *child, - BlockDriverState *new_child, - Error **errp) -{ - AioContext *parent_ctx = bdrv_get_aio_context(parent); - AioContext *child_ctx = bdrv_get_aio_context(new_child); - GSList *ignore; - bool ret; - - ignore = g_slist_prepend(NULL, child); - ret = bdrv_can_set_aio_context(new_child, parent_ctx, &ignore, NULL); - g_slist_free(ignore); - if (ret) { - return ret; - } - - ignore = g_slist_prepend(NULL, child); - ret = bdrv_can_set_aio_context(parent, child_ctx, &ignore, errp); - g_slist_free(ignore); - return ret; -} - /* * Take a BDRVReopenState and check if the value of 'backing' in the * reopen_state->options QDict is valid or not. @@ -4204,115 +4244,81 @@ static bool bdrv_reopen_can_attach(BlockDriverState *parent, * * Return 0 on success, otherwise return < 0 and set @errp. */ -static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state, - Transaction *set_backings_tran, - Error **errp) +static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state, + bool is_backing, Transaction *tran, + Error **errp) { BlockDriverState *bs = reopen_state->bs; - BlockDriverState *overlay_bs, *below_bs, *new_backing_bs; + BlockDriverState *new_child_bs; + BlockDriverState *old_child_bs = is_backing ? child_bs(bs->backing) : + child_bs(bs->file); + const char *child_name = is_backing ? "backing" : "file"; QObject *value; const char *str; - value = qdict_get(reopen_state->options, "backing"); + value = qdict_get(reopen_state->options, child_name); if (value == NULL) { return 0; } switch (qobject_type(value)) { case QTYPE_QNULL: - new_backing_bs = NULL; + assert(is_backing); /* The 'file' option does not allow a null value */ + new_child_bs = NULL; break; case QTYPE_QSTRING: str = qstring_get_str(qobject_to(QString, value)); - new_backing_bs = bdrv_lookup_bs(NULL, str, errp); - if (new_backing_bs == NULL) { + new_child_bs = bdrv_lookup_bs(NULL, str, errp); + if (new_child_bs == NULL) { return -EINVAL; - } else if (bdrv_recurse_has_child(new_backing_bs, bs)) { - error_setg(errp, "Making '%s' a backing file of '%s' " - "would create a cycle", str, bs->node_name); + } else if (bdrv_recurse_has_child(new_child_bs, bs)) { + error_setg(errp, "Making '%s' a %s child of '%s' would create a " + "cycle", str, child_name, bs->node_name); return -EINVAL; } break; default: - /* 'backing' does not allow any other data type */ + /* + * The options QDict has been flattened, so 'backing' and 'file' + * do not allow any other data type here. + */ g_assert_not_reached(); } - /* - * Check AioContext compatibility so that the bdrv_set_backing_hd() call in - * bdrv_reopen_commit() won't fail. - */ - if (new_backing_bs) { - if (!bdrv_reopen_can_attach(bs, bs->backing, new_backing_bs, errp)) { - return -EINVAL; + if (old_child_bs == new_child_bs) { + return 0; + } + + if (old_child_bs) { + if (bdrv_skip_implicit_filters(old_child_bs) == new_child_bs) { + return 0; + } + + if (old_child_bs->implicit) { + error_setg(errp, "Cannot replace implicit %s child of %s", + child_name, bs->node_name); + return -EPERM; } } - /* - * Ensure that @bs can really handle backing files, because we are - * about to give it one (or swap the existing one) - */ - if (bs->drv->is_filter) { - /* Filters always have a file or a backing child */ - if (!bs->backing) { - error_setg(errp, "'%s' is a %s filter node that does not support a " - "backing child", bs->node_name, bs->drv->format_name); - return -EINVAL; - } - } else if (!bs->drv->supports_backing) { - error_setg(errp, "Driver '%s' of node '%s' does not support backing " - "files", bs->drv->format_name, bs->node_name); + if (bs->drv->is_filter && !old_child_bs) { + /* + * Filters always have a file or a backing child, so we are trying to + * change wrong child + */ + error_setg(errp, "'%s' is a %s filter node that does not support a " + "%s child", bs->node_name, bs->drv->format_name, child_name); return -EINVAL; } - /* - * Find the "actual" backing file by skipping all links that point - * to an implicit node, if any (e.g. a commit filter node). - * We cannot use any of the bdrv_skip_*() functions here because - * those return the first explicit node, while we are looking for - * its overlay here. - */ - overlay_bs = bs; - for (below_bs = bdrv_filter_or_cow_bs(overlay_bs); - below_bs && below_bs->implicit; - below_bs = bdrv_filter_or_cow_bs(overlay_bs)) - { - overlay_bs = below_bs; + if (is_backing) { + reopen_state->old_backing_bs = old_child_bs; + } else { + reopen_state->old_file_bs = old_child_bs; } - /* If we want to replace the backing file we need some extra checks */ - if (new_backing_bs != bdrv_filter_or_cow_bs(overlay_bs)) { - int ret; - - /* Check for implicit nodes between bs and its backing file */ - if (bs != overlay_bs) { - error_setg(errp, "Cannot change backing link if '%s' has " - "an implicit backing file", bs->node_name); - return -EPERM; - } - /* - * Check if the backing link that we want to replace is frozen. - * Note that - * bdrv_filter_or_cow_child(overlay_bs) == overlay_bs->backing, - * because we know that overlay_bs == bs, and that @bs - * either is a filter that uses ->backing or a COW format BDS - * with bs->drv->supports_backing == true. - */ - if (bdrv_is_backing_chain_frozen(overlay_bs, - child_bs(overlay_bs->backing), errp)) - { - return -EPERM; - } - reopen_state->replace_backing_bs = true; - reopen_state->old_backing_bs = bs->backing ? bs->backing->bs : NULL; - ret = bdrv_set_backing_noperm(bs, new_backing_bs, set_backings_tran, - errp); - if (ret < 0) { - return ret; - } - } - - return 0; + return bdrv_set_file_or_backing_noperm(bs, new_child_bs, is_backing, + tran, errp); } /* @@ -4334,7 +4340,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state, */ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, - Transaction *set_backings_tran, Error **errp) + Transaction *change_child_tran, Error **errp) { int ret = -1; int old_flags; @@ -4454,12 +4460,21 @@ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state, * either a reference to an existing node (using its node name) * or NULL to simply detach the current backing file. */ - ret = bdrv_reopen_parse_backing(reopen_state, set_backings_tran, errp); + ret = bdrv_reopen_parse_file_or_backing(reopen_state, true, + change_child_tran, errp); if (ret < 0) { goto error; } qdict_del(reopen_state->options, "backing"); + /* Allow changing the 'file' option. In this case NULL is not allowed */ + ret = bdrv_reopen_parse_file_or_backing(reopen_state, false, + change_child_tran, errp); + if (ret < 0) { + goto error; + } + qdict_del(reopen_state->options, "file"); + /* Options that are not handled are only okay if they are unchanged * compared to the old state. It is expected that some options are only * used for the initial open, but not reopen (e.g. filename) */ @@ -4564,17 +4579,16 @@ static void bdrv_reopen_commit(BDRVReopenState *reopen_state) bs->open_flags = reopen_state->flags; bs->detect_zeroes = reopen_state->detect_zeroes; - if (reopen_state->replace_backing_bs) { - qdict_del(bs->explicit_options, "backing"); - qdict_del(bs->options, "backing"); - } - /* Remove child references from bs->options and bs->explicit_options. * Child options were already removed in bdrv_reopen_queue_child() */ QLIST_FOREACH(child, &bs->children, next) { qdict_del(bs->explicit_options, child->name); qdict_del(bs->options, child->name); } + /* backing is probably removed, so it's not handled by previous loop */ + qdict_del(bs->explicit_options, "backing"); + qdict_del(bs->options, "backing"); + bdrv_refresh_limits(bs, NULL, NULL); } @@ -4766,7 +4780,7 @@ static void bdrv_remove_filter_or_cow_child_abort(void *opaque) } /* - * We don't have to restore child->bs here to undo bdrv_replace_child() + * We don't have to restore child->bs here to undo bdrv_replace_child_tran() * because that function is transactionable and it registered own completion * entries in @tran, so .abort() for bdrv_replace_child_safe() will be * called automatically. @@ -4787,22 +4801,23 @@ static TransactionActionDrv bdrv_remove_filter_or_cow_child_drv = { }; /* - * A function to remove backing-chain child of @bs if exists: cow child for - * format nodes (always .backing) and filter child for filters (may be .file or - * .backing) + * A function to remove backing or file child of @bs. + * Function doesn't update permissions, caller is responsible for this. */ -static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs, - Transaction *tran) +static void bdrv_remove_file_or_backing_child(BlockDriverState *bs, + BdrvChild *child, + Transaction *tran) { BdrvRemoveFilterOrCowChild *s; - BdrvChild *child = bdrv_filter_or_cow_child(bs); + + assert(child == bs->backing || child == bs->file); if (!child) { return; } if (child->bs) { - bdrv_replace_child(child, NULL, tran); + bdrv_replace_child_tran(child, NULL, tran); } s = g_new(BdrvRemoveFilterOrCowChild, 1); @@ -4820,6 +4835,17 @@ static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs, } } +/* + * A function to remove backing-chain child of @bs if exists: cow child for + * format nodes (always .backing) and filter child for filters (may be .file or + * .backing) + */ +static void bdrv_remove_filter_or_cow_child(BlockDriverState *bs, + Transaction *tran) +{ + bdrv_remove_file_or_backing_child(bs, bdrv_filter_or_cow_child(bs), tran); +} + static int bdrv_replace_node_noperm(BlockDriverState *from, BlockDriverState *to, bool auto_skip, Transaction *tran, @@ -4842,7 +4868,7 @@ static int bdrv_replace_node_noperm(BlockDriverState *from, c->name, from->node_name); return -EPERM; } - bdrv_replace_child(c, to, tran); + bdrv_replace_child_tran(c, to, tran); } return 0; @@ -4866,7 +4892,7 @@ static int bdrv_replace_node_common(BlockDriverState *from, Transaction *tran = tran_new(); g_autoptr(GHashTable) found = NULL; g_autoptr(GSList) refresh_list = NULL; - BlockDriverState *to_cow_parent; + BlockDriverState *to_cow_parent = NULL; int ret; if (detach_subchain) { @@ -6553,9 +6579,13 @@ void bdrv_img_create(const char *filename, const char *fmt, } assert(full_backing); - /* backing files always opened read-only */ + /* + * No need to do I/O here, which allows us to open encrypted + * backing images without needing the secret + */ back_flags = flags; back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING); + back_flags |= BDRV_O_NO_IO; backing_options = qdict_new(); if (backing_fmt) { diff --git a/block/commit.c b/block/commit.c index b7f0c7c061..42792b4556 100644 --- a/block/commit.c +++ b/block/commit.c @@ -119,24 +119,24 @@ static int coroutine_fn commit_run(Job *job, Error **errp) uint64_t delay_ns = 0; int ret = 0; int64_t n = 0; /* bytes */ - void *buf = NULL; + QEMU_AUTO_VFREE void *buf = NULL; int64_t len, base_len; - ret = len = blk_getlength(s->top); + len = blk_getlength(s->top); if (len < 0) { - goto out; + return len; } job_progress_set_remaining(&s->common.job, len); - ret = base_len = blk_getlength(s->base); + base_len = blk_getlength(s->base); if (base_len < 0) { - goto out; + return base_len; } if (base_len < len) { ret = blk_truncate(s->base, len, false, PREALLOC_MODE_OFF, 0, NULL); if (ret) { - goto out; + return ret; } } @@ -174,7 +174,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp) block_job_error_action(&s->common, s->on_error, error_in_source, -ret); if (action == BLOCK_ERROR_ACTION_REPORT) { - goto out; + return ret; } else { n = 0; continue; @@ -190,12 +190,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp) } } - ret = 0; - -out: - qemu_vfree(buf); - - return ret; + return 0; } static const BlockJobDriver commit_job_driver = { @@ -435,7 +430,7 @@ int bdrv_commit(BlockDriverState *bs) int ro; int64_t n; int ret = 0; - uint8_t *buf = NULL; + QEMU_AUTO_VFREE uint8_t *buf = NULL; Error *local_err = NULL; if (!drv) @@ -556,8 +551,6 @@ int bdrv_commit(BlockDriverState *bs) ret = 0; ro_cleanup: - qemu_vfree(buf); - blk_unref(backing); if (bdrv_cow_bs(bs) != backing_file_bs) { bdrv_set_backing_hd(bs, backing_file_bs, &error_abort); diff --git a/block/io.c b/block/io.c index dd93364258..cf177a9d2d 100644 --- a/block/io.c +++ b/block/io.c @@ -3392,6 +3392,11 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, return old_size; } + if (bdrv_is_read_only(bs)) { + error_setg(errp, "Image is read-only"); + return -EACCES; + } + if (offset > old_size) { new_bytes = offset - old_size; } else { @@ -3408,11 +3413,6 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact, if (new_bytes) { bdrv_make_request_serialising(&req, 1); } - if (bdrv_is_read_only(bs)) { - error_setg(errp, "Image is read-only"); - ret = -EACCES; - goto out; - } ret = bdrv_co_write_req_prepare(child, offset - new_bytes, new_bytes, &req, 0); if (ret < 0) { diff --git a/block/ssh.c b/block/ssh.c index b51a031620..d008caf059 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -442,6 +442,9 @@ static int check_host_key(BDRVSSHState *s, SshHostKeyCheck *hkc, Error **errp) } else if (hkc->u.hash.type == SSH_HOST_KEY_CHECK_HASH_TYPE_SHA1) { return check_host_key_hash(s, hkc->u.hash.hash, SSH_PUBLICKEY_HASH_SHA1, errp); + } else if (hkc->u.hash.type == SSH_HOST_KEY_CHECK_HASH_TYPE_SHA256) { + return check_host_key_hash(s, hkc->u.hash.hash, + SSH_PUBLICKEY_HASH_SHA256, errp); } g_assert_not_reached(); break; diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index c6210fad0c..ba13cb87e5 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -31,6 +31,8 @@ #include "sysemu/sysemu.h" #include "sysemu/runstate.h" +#define REALIZE_CONNECTION_RETRIES 3 + static const int user_feature_bits[] = { VIRTIO_BLK_F_SIZE_MAX, VIRTIO_BLK_F_SEG_MAX, @@ -91,11 +93,13 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev) int ret; struct virtio_blk_config blkcfg; VHostUserBlk *s = VHOST_USER_BLK(dev->vdev); + Error *local_err = NULL; ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg, - sizeof(struct virtio_blk_config)); + sizeof(struct virtio_blk_config), + &local_err); if (ret < 0) { - error_report("get config space failed"); + error_report_err(local_err); return -1; } @@ -113,7 +117,7 @@ const VhostDevConfigOps blk_ops = { .vhost_dev_config_notifier = vhost_user_blk_handle_config_change, }; -static int vhost_user_blk_start(VirtIODevice *vdev) +static int vhost_user_blk_start(VirtIODevice *vdev, Error **errp) { VHostUserBlk *s = VHOST_USER_BLK(vdev); BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev))); @@ -121,19 +125,19 @@ static int vhost_user_blk_start(VirtIODevice *vdev) int i, ret; if (!k->set_guest_notifiers) { - error_report("binding does not support guest notifiers"); + error_setg(errp, "binding does not support guest notifiers"); return -ENOSYS; } ret = vhost_dev_enable_notifiers(&s->dev, vdev); if (ret < 0) { - error_report("Error enabling host notifiers: %d", -ret); + error_setg_errno(errp, -ret, "Error enabling host notifiers"); return ret; } ret = k->set_guest_notifiers(qbus->parent, s->dev.nvqs, true); if (ret < 0) { - error_report("Error binding guest notifier: %d", -ret); + error_setg_errno(errp, -ret, "Error binding guest notifier"); goto err_host_notifiers; } @@ -141,27 +145,27 @@ static int vhost_user_blk_start(VirtIODevice *vdev) ret = vhost_dev_prepare_inflight(&s->dev, vdev); if (ret < 0) { - error_report("Error set inflight format: %d", -ret); + error_setg_errno(errp, -ret, "Error setting inflight format"); goto err_guest_notifiers; } if (!s->inflight->addr) { ret = vhost_dev_get_inflight(&s->dev, s->queue_size, s->inflight); if (ret < 0) { - error_report("Error get inflight: %d", -ret); + error_setg_errno(errp, -ret, "Error getting inflight"); goto err_guest_notifiers; } } ret = vhost_dev_set_inflight(&s->dev, s->inflight); if (ret < 0) { - error_report("Error set inflight: %d", -ret); + error_setg_errno(errp, -ret, "Error setting inflight"); goto err_guest_notifiers; } ret = vhost_dev_start(&s->dev, vdev); if (ret < 0) { - error_report("Error starting vhost: %d", -ret); + error_setg_errno(errp, -ret, "Error starting vhost"); goto err_guest_notifiers; } s->started_vu = true; @@ -214,6 +218,7 @@ static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status) { VHostUserBlk *s = VHOST_USER_BLK(vdev); bool should_start = virtio_device_started(vdev, status); + Error *local_err = NULL; int ret; if (!vdev->vm_running) { @@ -229,10 +234,9 @@ static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status) } if (should_start) { - ret = vhost_user_blk_start(vdev); + ret = vhost_user_blk_start(vdev, &local_err); if (ret < 0) { - error_report("vhost-user-blk: vhost start failed: %s", - strerror(-ret)); + error_reportf_err(local_err, "vhost-user-blk: vhost start failed: "); qemu_chr_fe_disconnect(&s->chardev); } } else { @@ -270,6 +274,7 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev, static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) { VHostUserBlk *s = VHOST_USER_BLK(vdev); + Error *local_err = NULL; int i, ret; if (!vdev->start_on_kick) { @@ -287,10 +292,9 @@ static void vhost_user_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start * vhost here instead of waiting for .set_status(). */ - ret = vhost_user_blk_start(vdev); + ret = vhost_user_blk_start(vdev, &local_err); if (ret < 0) { - error_report("vhost-user-blk: vhost start failed: %s", - strerror(-ret)); + error_reportf_err(local_err, "vhost-user-blk: vhost start failed: "); qemu_chr_fe_disconnect(&s->chardev); return; } @@ -332,17 +336,16 @@ static int vhost_user_blk_connect(DeviceState *dev, Error **errp) vhost_dev_set_config_notifier(&s->dev, &blk_ops); - ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0); + ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER, 0, + errp); if (ret < 0) { - error_setg_errno(errp, -ret, "vhost initialization failed"); return ret; } /* restore vhost state */ if (virtio_device_started(vdev, vdev->status)) { - ret = vhost_user_blk_start(vdev); + ret = vhost_user_blk_start(vdev, errp); if (ret < 0) { - error_setg_errno(errp, -ret, "vhost start failed"); return ret; } } @@ -422,10 +425,42 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent event) } } +static int vhost_user_blk_realize_connect(VHostUserBlk *s, Error **errp) +{ + DeviceState *dev = &s->parent_obj.parent_obj; + int ret; + + s->connected = false; + + ret = qemu_chr_fe_wait_connected(&s->chardev, errp); + if (ret < 0) { + return ret; + } + + ret = vhost_user_blk_connect(dev, errp); + if (ret < 0) { + qemu_chr_fe_disconnect(&s->chardev); + return ret; + } + assert(s->connected); + + ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, + sizeof(struct virtio_blk_config), errp); + if (ret < 0) { + qemu_chr_fe_disconnect(&s->chardev); + vhost_dev_cleanup(&s->dev); + return ret; + } + + return 0; +} + static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) { + ERRP_GUARD(); VirtIODevice *vdev = VIRTIO_DEVICE(dev); VHostUserBlk *s = VHOST_USER_BLK(vdev); + int retries; int i, ret; if (!s->chardev.chr) { @@ -466,23 +501,20 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) s->inflight = g_new0(struct vhost_inflight, 1); s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues); - s->connected = false; - if (qemu_chr_fe_wait_connected(&s->chardev, errp) < 0) { - goto virtio_err; - } + retries = REALIZE_CONNECTION_RETRIES; + assert(!*errp); + do { + if (*errp) { + error_prepend(errp, "Reconnecting after error: "); + error_report_err(*errp); + *errp = NULL; + } + ret = vhost_user_blk_realize_connect(s, errp); + } while (ret == -EPROTO && retries--); - if (vhost_user_blk_connect(dev, errp) < 0) { - qemu_chr_fe_disconnect(&s->chardev); - goto virtio_err; - } - assert(s->connected); - - ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg, - sizeof(struct virtio_blk_config)); if (ret < 0) { - error_setg(errp, "vhost-user-blk: get block config failed"); - goto vhost_err; + goto virtio_err; } /* we're fully initialized, now we can operate, so add the handler */ @@ -491,8 +523,6 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp) NULL, true); return; -vhost_err: - vhost_dev_cleanup(&s->dev); virtio_err: g_free(s->vhost_vqs); s->vhost_vqs = NULL; diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c index 6cdaa1c73b..389199e6ca 100644 --- a/hw/display/vhost-user-gpu.c +++ b/hw/display/vhost-user-gpu.c @@ -415,14 +415,16 @@ vhost_user_gpu_get_config(VirtIODevice *vdev, uint8_t *config_data) VirtIOGPUBase *b = VIRTIO_GPU_BASE(vdev); struct virtio_gpu_config *vgconfig = (struct virtio_gpu_config *)config_data; + Error *local_err = NULL; int ret; memset(config_data, 0, sizeof(struct virtio_gpu_config)); ret = vhost_dev_get_config(&g->vhost->dev, - config_data, sizeof(struct virtio_gpu_config)); + config_data, sizeof(struct virtio_gpu_config), + &local_err); if (ret) { - error_report("vhost-user-gpu: get device config space failed"); + error_report_err(local_err); return; } diff --git a/hw/input/vhost-user-input.c b/hw/input/vhost-user-input.c index 63984a8ba7..273e96a7b1 100644 --- a/hw/input/vhost-user-input.c +++ b/hw/input/vhost-user-input.c @@ -49,13 +49,15 @@ static void vhost_input_get_config(VirtIODevice *vdev, uint8_t *config_data) { VirtIOInput *vinput = VIRTIO_INPUT(vdev); VHostUserInput *vhi = VHOST_USER_INPUT(vdev); + Error *local_err = NULL; int ret; memset(config_data, 0, vinput->cfg_size); - ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, vinput->cfg_size); + ret = vhost_dev_get_config(&vhi->vhost->dev, config_data, vinput->cfg_size, + &local_err); if (ret) { - error_report("vhost-user-input: get device config space failed"); + error_report_err(local_err); return; } } diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 44c1ed92dc..10a7780a13 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -22,6 +22,7 @@ #include "standard-headers/linux/vhost_types.h" #include "hw/virtio/virtio-net.h" #include "net/vhost_net.h" +#include "qapi/error.h" #include "qemu/error-report.h" #include "qemu/main-loop.h" @@ -116,7 +117,7 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features) int vhost_net_get_config(struct vhost_net *net, uint8_t *config, uint32_t config_len) { - return vhost_dev_get_config(&net->dev, config, config_len); + return vhost_dev_get_config(&net->dev, config, config_len, NULL); } int vhost_net_set_config(struct vhost_net *net, const uint8_t *data, uint32_t offset, uint32_t size, uint32_t flags) @@ -157,6 +158,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) bool backend_kernel = options->backend_type == VHOST_BACKEND_TYPE_KERNEL; struct vhost_net *net = g_new0(struct vhost_net, 1); uint64_t features = 0; + Error *local_err = NULL; if (!options->net_backend) { fprintf(stderr, "vhost-net requires net backend to be setup\n"); @@ -187,8 +189,10 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) } r = vhost_dev_init(&net->dev, options->opaque, - options->backend_type, options->busyloop_timeout); + options->backend_type, options->busyloop_timeout, + &local_err); if (r < 0) { + error_report_err(local_err); goto fail; } if (backend_kernel) { diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 4d70fa036b..8c611bfd2d 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -219,10 +219,8 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) vsc->dev.backend_features = 0; ret = vhost_dev_init(&vsc->dev, (void *)(uintptr_t)vhostfd, - VHOST_BACKEND_TYPE_KERNEL, 0); + VHOST_BACKEND_TYPE_KERNEL, 0, errp); if (ret < 0) { - error_setg(errp, "vhost-scsi: vhost initialization failed: %s", - strerror(-ret)); goto free_vqs; } diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 4666019442..1b2f7eed98 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -122,10 +122,8 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp) vqs = vsc->dev.vqs; ret = vhost_dev_init(&vsc->dev, &s->vhost_user, - VHOST_BACKEND_TYPE_USER, 0); + VHOST_BACKEND_TYPE_USER, 0, errp); if (ret < 0) { - error_setg(errp, "vhost-user-scsi: vhost initialization failed: %s", - strerror(-ret)); goto free_vhost; } diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c index 31b33bde37..594d770b75 100644 --- a/hw/virtio/vhost-backend.c +++ b/hw/virtio/vhost-backend.c @@ -24,13 +24,15 @@ static int vhost_kernel_call(struct vhost_dev *dev, unsigned long int request, void *arg) { int fd = (uintptr_t) dev->opaque; + int ret; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); - return ioctl(fd, request, arg); + ret = ioctl(fd, request, arg); + return ret < 0 ? -errno : ret; } -static int vhost_kernel_init(struct vhost_dev *dev, void *opaque) +static int vhost_kernel_init(struct vhost_dev *dev, void *opaque, Error **errp) { assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_KERNEL); diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c index 6f7f91533d..c595957983 100644 --- a/hw/virtio/vhost-user-fs.c +++ b/hw/virtio/vhost-user-fs.c @@ -235,9 +235,8 @@ static void vuf_device_realize(DeviceState *dev, Error **errp) fs->vhost_dev.nvqs = 1 + fs->conf.num_request_queues; fs->vhost_dev.vqs = g_new0(struct vhost_virtqueue, fs->vhost_dev.nvqs); ret = vhost_dev_init(&fs->vhost_dev, &fs->vhost_user, - VHOST_BACKEND_TYPE_USER, 0); + VHOST_BACKEND_TYPE_USER, 0, errp); if (ret < 0) { - error_setg_errno(errp, -ret, "vhost_dev_init failed"); goto err_virtio; } diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c index a6f08c26b9..6095ed7349 100644 --- a/hw/virtio/vhost-user-vsock.c +++ b/hw/virtio/vhost-user-vsock.c @@ -34,10 +34,12 @@ static void vuv_get_config(VirtIODevice *vdev, uint8_t *config) static int vuv_handle_config_change(struct vhost_dev *dev) { VHostUserVSock *vsock = VHOST_USER_VSOCK(dev->vdev); + Error *local_err = NULL; int ret = vhost_dev_get_config(dev, (uint8_t *)&vsock->vsockcfg, - sizeof(struct virtio_vsock_config)); + sizeof(struct virtio_vsock_config), + &local_err); if (ret < 0) { - error_report("get config space failed"); + error_report_err(local_err); return -1; } @@ -108,16 +110,14 @@ static void vuv_device_realize(DeviceState *dev, Error **errp) vhost_dev_set_config_notifier(&vvc->vhost_dev, &vsock_ops); ret = vhost_dev_init(&vvc->vhost_dev, &vsock->vhost_user, - VHOST_BACKEND_TYPE_USER, 0); + VHOST_BACKEND_TYPE_USER, 0, errp); if (ret < 0) { - error_setg_errno(errp, -ret, "vhost_dev_init failed"); goto err_virtio; } ret = vhost_dev_get_config(&vvc->vhost_dev, (uint8_t *)&vsock->vsockcfg, - sizeof(struct virtio_vsock_config)); + sizeof(struct virtio_vsock_config), errp); if (ret < 0) { - error_setg_errno(errp, -ret, "get config space failed"); goto err_vhost_dev; } diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index ee57abe045..1ac4a2ebec 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -1353,7 +1353,11 @@ static int vhost_user_get_u64(struct vhost_dev *dev, int request, uint64_t *u64) static int vhost_user_get_features(struct vhost_dev *dev, uint64_t *features) { - return vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features); + if (vhost_user_get_u64(dev, VHOST_USER_GET_FEATURES, features) < 0) { + return -EPROTO; + } + + return 0; } static int vhost_user_set_owner(struct vhost_dev *dev) @@ -1364,7 +1368,7 @@ static int vhost_user_set_owner(struct vhost_dev *dev) }; if (vhost_user_write(dev, &msg, NULL, 0) < 0) { - return -1; + return -EPROTO; } return 0; @@ -1856,7 +1860,8 @@ static int vhost_user_postcopy_notifier(NotifierWithReturn *notifier, return 0; } -static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) +static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque, + Error **errp) { uint64_t features, protocol_features, ram_slots; struct vhost_user *u; @@ -1880,7 +1885,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) err = vhost_user_get_u64(dev, VHOST_USER_GET_PROTOCOL_FEATURES, &protocol_features); if (err < 0) { - return err; + return -EPROTO; } dev->protocol_features = @@ -1891,14 +1896,14 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) dev->protocol_features &= ~(1ULL << VHOST_USER_PROTOCOL_F_CONFIG); } else if (!(protocol_features & (1ULL << VHOST_USER_PROTOCOL_F_CONFIG))) { - error_report("Device expects VHOST_USER_PROTOCOL_F_CONFIG " - "but backend does not support it."); - return -1; + error_setg(errp, "Device expects VHOST_USER_PROTOCOL_F_CONFIG " + "but backend does not support it."); + return -EINVAL; } err = vhost_user_set_protocol_features(dev, dev->protocol_features); if (err < 0) { - return err; + return -EPROTO; } /* query the max queues we support if backend supports Multiple Queue */ @@ -1906,12 +1911,12 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) err = vhost_user_get_u64(dev, VHOST_USER_GET_QUEUE_NUM, &dev->max_queues); if (err < 0) { - return err; + return -EPROTO; } } if (dev->num_queues && dev->max_queues < dev->num_queues) { - error_report("The maximum number of queues supported by the " - "backend is %" PRIu64, dev->max_queues); + error_setg(errp, "The maximum number of queues supported by the " + "backend is %" PRIu64, dev->max_queues); return -EINVAL; } @@ -1920,9 +1925,9 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) VHOST_USER_PROTOCOL_F_SLAVE_REQ) && virtio_has_feature(dev->protocol_features, VHOST_USER_PROTOCOL_F_REPLY_ACK))) { - error_report("IOMMU support requires reply-ack and " - "slave-req protocol features."); - return -1; + error_setg(errp, "IOMMU support requires reply-ack and " + "slave-req protocol features."); + return -EINVAL; } /* get max memory regions if backend supports configurable RAM slots */ @@ -1932,15 +1937,15 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) } else { err = vhost_user_get_max_memslots(dev, &ram_slots); if (err < 0) { - return err; + return -EPROTO; } if (ram_slots < u->user->memory_slots) { - error_report("The backend specified a max ram slots limit " - "of %" PRIu64", when the prior validated limit was %d. " - "This limit should never decrease.", ram_slots, - u->user->memory_slots); - return -1; + error_setg(errp, "The backend specified a max ram slots limit " + "of %" PRIu64", when the prior validated limit was " + "%d. This limit should never decrease.", ram_slots, + u->user->memory_slots); + return -EINVAL; } u->user->memory_slots = MIN(ram_slots, VHOST_USER_MAX_RAM_SLOTS); @@ -1958,7 +1963,7 @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque) if (dev->vq_index == 0) { err = vhost_setup_slave_channel(dev); if (err < 0) { - return err; + return -EPROTO; } } @@ -2112,7 +2117,7 @@ static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled) } static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, - uint32_t config_len) + uint32_t config_len, Error **errp) { VhostUserMsg msg = { .hdr.request = VHOST_USER_GET_CONFIG, @@ -2122,32 +2127,32 @@ static int vhost_user_get_config(struct vhost_dev *dev, uint8_t *config, if (!virtio_has_feature(dev->protocol_features, VHOST_USER_PROTOCOL_F_CONFIG)) { - return -1; + error_setg(errp, "VHOST_USER_PROTOCOL_F_CONFIG not supported"); + return -EINVAL; } - if (config_len > VHOST_USER_MAX_CONFIG_SIZE) { - return -1; - } + assert(config_len <= VHOST_USER_MAX_CONFIG_SIZE); msg.payload.config.offset = 0; msg.payload.config.size = config_len; if (vhost_user_write(dev, &msg, NULL, 0) < 0) { - return -1; + return -EPROTO; } if (vhost_user_read(dev, &msg) < 0) { - return -1; + return -EPROTO; } if (msg.hdr.request != VHOST_USER_GET_CONFIG) { - error_report("Received unexpected msg type. Expected %d received %d", - VHOST_USER_GET_CONFIG, msg.hdr.request); - return -1; + error_setg(errp, + "Received unexpected msg type. Expected %d received %d", + VHOST_USER_GET_CONFIG, msg.hdr.request); + return -EINVAL; } if (msg.hdr.size != VHOST_USER_CONFIG_HDR_SIZE + config_len) { - error_report("Received bad msg size."); - return -1; + error_setg(errp, "Received bad msg size."); + return -EINVAL; } memcpy(config, msg.payload.config.region, config_len); diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c index 61ba313331..4fa414feea 100644 --- a/hw/virtio/vhost-vdpa.c +++ b/hw/virtio/vhost-vdpa.c @@ -245,10 +245,12 @@ static int vhost_vdpa_call(struct vhost_dev *dev, unsigned long int request, { struct vhost_vdpa *v = dev->opaque; int fd = v->device_fd; + int ret; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); - return ioctl(fd, request, arg); + ret = ioctl(fd, request, arg); + return ret < 0 ? -errno : ret; } static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) @@ -265,7 +267,7 @@ static void vhost_vdpa_add_status(struct vhost_dev *dev, uint8_t status) vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, &s); } -static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque) +static int vhost_vdpa_init(struct vhost_dev *dev, void *opaque, Error **errp) { struct vhost_vdpa *v; assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_VDPA); @@ -521,7 +523,7 @@ static int vhost_vdpa_set_config(struct vhost_dev *dev, const uint8_t *data, } static int vhost_vdpa_get_config(struct vhost_dev *dev, uint8_t *config, - uint32_t config_len) + uint32_t config_len, Error **errp) { struct vhost_vdpa_config *v_config; unsigned long config_size = offsetof(struct vhost_vdpa_config, buf); diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c index 8ddfb9abfe..777cafe70d 100644 --- a/hw/virtio/vhost-vsock.c +++ b/hw/virtio/vhost-vsock.c @@ -170,9 +170,8 @@ static void vhost_vsock_device_realize(DeviceState *dev, Error **errp) vhost_vsock_common_realize(vdev, "vhost-vsock"); ret = vhost_dev_init(&vvc->vhost_dev, (void *)(uintptr_t)vhostfd, - VHOST_BACKEND_TYPE_KERNEL, 0); + VHOST_BACKEND_TYPE_KERNEL, 0, errp); if (ret < 0) { - error_setg_errno(errp, -ret, "vhost-vsock: vhost_dev_init failed"); goto err_virtio; } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 7b7bde7657..e8f85a5d2d 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1286,11 +1286,12 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) } int vhost_dev_init(struct vhost_dev *hdev, void *opaque, - VhostBackendType backend_type, uint32_t busyloop_timeout) + VhostBackendType backend_type, uint32_t busyloop_timeout, + Error **errp) { + ERRP_GUARD(); uint64_t features; int i, r, n_initialized_vqs = 0; - Error *local_err = NULL; hdev->vdev = NULL; hdev->migration_blocker = NULL; @@ -1298,26 +1299,30 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, r = vhost_set_backend_type(hdev, backend_type); assert(r >= 0); - r = hdev->vhost_ops->vhost_backend_init(hdev, opaque); + r = hdev->vhost_ops->vhost_backend_init(hdev, opaque, errp); if (r < 0) { + if (!*errp) { + error_setg_errno(errp, -r, "vhost_backend_init failed"); + } goto fail; } r = hdev->vhost_ops->vhost_set_owner(hdev); if (r < 0) { - VHOST_OPS_DEBUG("vhost_set_owner failed"); + error_setg_errno(errp, -r, "vhost_set_owner failed"); goto fail; } r = hdev->vhost_ops->vhost_get_features(hdev, &features); if (r < 0) { - VHOST_OPS_DEBUG("vhost_get_features failed"); + error_setg_errno(errp, -r, "vhost_get_features failed"); goto fail; } for (i = 0; i < hdev->nvqs; ++i, ++n_initialized_vqs) { r = vhost_virtqueue_init(hdev, hdev->vqs + i, hdev->vq_index + i); if (r < 0) { + error_setg_errno(errp, -r, "Failed to initialize virtqueue %d", i); goto fail; } } @@ -1327,6 +1332,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, r = vhost_virtqueue_set_busyloop_timeout(hdev, hdev->vq_index + i, busyloop_timeout); if (r < 0) { + error_setg_errno(errp, -r, "Failed to set busyloop timeout"); goto fail_busyloop; } } @@ -1365,9 +1371,8 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, } if (hdev->migration_blocker != NULL) { - r = migrate_add_blocker(hdev->migration_blocker, &local_err); - if (local_err) { - error_report_err(local_err); + r = migrate_add_blocker(hdev->migration_blocker, errp); + if (*errp) { error_free(hdev->migration_blocker); goto fail_busyloop; } @@ -1384,9 +1389,9 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, QLIST_INSERT_HEAD(&vhost_devices, hdev, entry); if (used_memslots > hdev->vhost_ops->vhost_backend_memslots_limit(hdev)) { - error_report("vhost backend memory slots limit is less" - " than current number of present memory slots"); - r = -1; + error_setg(errp, "vhost backend memory slots limit is less" + " than current number of present memory slots"); + r = -EINVAL; goto fail_busyloop; } @@ -1557,15 +1562,23 @@ void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, } int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config, - uint32_t config_len) + uint32_t config_len, Error **errp) { + ERRP_GUARD(); + int ret; + assert(hdev->vhost_ops); if (hdev->vhost_ops->vhost_get_config) { - return hdev->vhost_ops->vhost_get_config(hdev, config, config_len); + ret = hdev->vhost_ops->vhost_get_config(hdev, config, config_len, errp); + if (ret < 0 && !*errp) { + error_setg_errno(errp, -ret, "vhost_get_config failed"); + } + return ret; } - return -1; + error_setg(errp, "vhost_get_config not implemented"); + return -ENOTSUP; } int vhost_dev_set_config(struct vhost_dev *hdev, const uint8_t *data, diff --git a/include/block/block.h b/include/block/block.h index 8e707a83b7..7ec77ecb1a 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -208,8 +208,8 @@ typedef struct BDRVReopenState { int flags; BlockdevDetectZeroesOptions detect_zeroes; bool backing_missing; - bool replace_backing_bs; /* new_backing_bs is ignored if this is false */ BlockDriverState *old_backing_bs; /* keep pointer for permissions update */ + BlockDriverState *old_file_bs; /* keep pointer for permissions update */ QDict *options; QDict *explicit_options; void *opaque; diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h index 8a6f8e2a7a..8475c5a29d 100644 --- a/include/hw/virtio/vhost-backend.h +++ b/include/hw/virtio/vhost-backend.h @@ -37,7 +37,8 @@ struct vhost_scsi_target; struct vhost_iotlb_msg; struct vhost_virtqueue; -typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque); +typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque, + Error **errp); typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev); typedef int (*vhost_backend_memslots_limit)(struct vhost_dev *dev); @@ -97,7 +98,7 @@ typedef int (*vhost_set_config_op)(struct vhost_dev *dev, const uint8_t *data, uint32_t offset, uint32_t size, uint32_t flags); typedef int (*vhost_get_config_op)(struct vhost_dev *dev, uint8_t *config, - uint32_t config_len); + uint32_t config_len, Error **errp); typedef int (*vhost_crypto_create_session_op)(struct vhost_dev *dev, void *session_info, diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index 21a9a52088..045d0fd9f2 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -104,7 +104,7 @@ struct vhost_net { int vhost_dev_init(struct vhost_dev *hdev, void *opaque, VhostBackendType backend_type, - uint32_t busyloop_timeout); + uint32_t busyloop_timeout, Error **errp); void vhost_dev_cleanup(struct vhost_dev *hdev); int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev); void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev); @@ -130,8 +130,8 @@ int vhost_net_set_backend(struct vhost_dev *hdev, struct vhost_vring_file *file); int vhost_device_iotlb_miss(struct vhost_dev *dev, uint64_t iova, int write); -int vhost_dev_get_config(struct vhost_dev *dev, uint8_t *config, - uint32_t config_len); +int vhost_dev_get_config(struct vhost_dev *hdev, uint8_t *config, + uint32_t config_len, Error **errp); int vhost_dev_set_config(struct vhost_dev *dev, const uint8_t *data, uint32_t offset, uint32_t size, uint32_t flags); /* notifier callback in case vhost device config space changed diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index c3656b755a..c91a78b5e6 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -386,6 +386,21 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool shared, void qemu_vfree(void *ptr); void qemu_anon_ram_free(void *ptr, size_t size); +/* + * It's an analog of GLIB's g_autoptr_cleanup_generic_gfree(), used to define + * g_autofree macro. + */ +static inline void qemu_cleanup_generic_vfree(void *p) +{ + void **pp = (void **)p; + qemu_vfree(*pp); +} + +/* + * Analog of g_autofree, but qemu_vfree is called on cleanup instead of g_free. + */ +#define QEMU_AUTO_VFREE __attribute__((cleanup(qemu_cleanup_generic_vfree))) + /* * Abstraction of PROT_ and MAP_ flags as passed to mmap(), for example, * consumed by qemu_ram_mmap(). diff --git a/qapi/block-core.json b/qapi/block-core.json index a54f37dbef..3114ba69bb 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -3190,11 +3190,12 @@ # # @md5: The given hash is an md5 hash # @sha1: The given hash is an sha1 hash +# @sha256: The given hash is an sha256 hash # # Since: 2.12 ## { 'enum': 'SshHostKeyCheckHashType', - 'data': [ 'md5', 'sha1' ] } + 'data': [ 'md5', 'sha1', 'sha256' ] } ## # @SshHostKeyHash: diff --git a/tests/qemu-iotests/189 b/tests/qemu-iotests/189 index 4e463385b2..801494c6b9 100755 --- a/tests/qemu-iotests/189 +++ b/tests/qemu-iotests/189 @@ -67,7 +67,7 @@ echo "== verify pattern ==" $QEMU_IO --object $SECRET0 -c "read -P 0xa 0 $size" --image-opts $IMGSPECBASE | _filter_qemu_io | _filter_testdir echo "== create overlay ==" -_make_test_img --object $SECRET1 -o "encrypt.format=luks,encrypt.key-secret=sec1,encrypt.iter-time=10" -u -b "$TEST_IMG_BASE" -F $IMGFMT $size +_make_test_img --object $SECRET1 -o "encrypt.format=luks,encrypt.key-secret=sec1,encrypt.iter-time=10" -b "$TEST_IMG_BASE" -F $IMGFMT echo echo "== writing part of a cluster ==" diff --git a/tests/qemu-iotests/198 b/tests/qemu-iotests/198 index b333a8f281..1c93dea1f7 100755 --- a/tests/qemu-iotests/198 +++ b/tests/qemu-iotests/198 @@ -64,7 +64,7 @@ echo "== writing whole image base ==" $QEMU_IO --object $SECRET0 -c "write -P 0xa 0 $size" --image-opts $IMGSPECBASE | _filter_qemu_io | _filter_testdir echo "== create overlay ==" -_make_test_img --object $SECRET1 -o "encrypt.format=luks,encrypt.key-secret=sec1,encrypt.iter-time=10" -u -b "$TEST_IMG_BASE" -F $IMGFMT $size +_make_test_img --object $SECRET1 -o "encrypt.format=luks,encrypt.key-secret=sec1,encrypt.iter-time=10" -b "$TEST_IMG_BASE" -F $IMGFMT echo echo "== writing whole image layer ==" diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207 index f9f3fd7131..0f5c4bc8a0 100755 --- a/tests/qemu-iotests/207 +++ b/tests/qemu-iotests/207 @@ -73,6 +73,9 @@ with iotests.FilePath('t.img') as disk_path, \ iotests.log("=== Test host-key-check options ===") iotests.log("") + iotests.log("--- no host key checking --") + iotests.log("") + vm.launch() blockdev_create(vm, { 'driver': 'ssh', 'location': { @@ -90,6 +93,9 @@ with iotests.FilePath('t.img') as disk_path, \ iotests.img_info_log(remote_path) + iotests.log("--- known_hosts key checking --") + iotests.log("") + vm.launch() blockdev_create(vm, { 'driver': 'ssh', 'location': { @@ -115,6 +121,7 @@ with iotests.FilePath('t.img') as disk_path, \ # Mappings of base64 representations to digests md5_keys = {} sha1_keys = {} + sha256_keys = {} for key in keys: md5_keys[key] = subprocess.check_output( @@ -125,6 +132,10 @@ with iotests.FilePath('t.img') as disk_path, \ 'echo %s | base64 -d | sha1sum -b | cut -d" " -f1' % key, shell=True).rstrip().decode('ascii') + sha256_keys[key] = subprocess.check_output( + 'echo %s | base64 -d | sha256sum -b | cut -d" " -f1' % key, + shell=True).rstrip().decode('ascii') + vm.launch() # Find correct key first @@ -150,6 +161,9 @@ with iotests.FilePath('t.img') as disk_path, \ vm.shutdown() iotests.notrun('Did not find a key that fits 127.0.0.1') + iotests.log("--- explicit md5 key checking --") + iotests.log("") + blockdev_create(vm, { 'driver': 'ssh', 'location': { 'path': disk_path, @@ -164,6 +178,7 @@ with iotests.FilePath('t.img') as disk_path, \ } }, 'size': 2097152 }) + blockdev_create(vm, { 'driver': 'ssh', 'location': { 'path': disk_path, @@ -182,6 +197,9 @@ with iotests.FilePath('t.img') as disk_path, \ iotests.img_info_log(remote_path) + iotests.log("--- explicit sha1 key checking --") + iotests.log("") + vm.launch() blockdev_create(vm, { 'driver': 'ssh', 'location': { @@ -215,6 +233,42 @@ with iotests.FilePath('t.img') as disk_path, \ iotests.img_info_log(remote_path) + iotests.log("--- explicit sha256 key checking --") + iotests.log("") + + vm.launch() + blockdev_create(vm, { 'driver': 'ssh', + 'location': { + 'path': disk_path, + 'server': { + 'host': '127.0.0.1', + 'port': '22' + }, + 'host-key-check': { + 'mode': 'hash', + 'type': 'sha256', + 'hash': 'wrong', + } + }, + 'size': 2097152 }) + blockdev_create(vm, { 'driver': 'ssh', + 'location': { + 'path': disk_path, + 'server': { + 'host': '127.0.0.1', + 'port': '22' + }, + 'host-key-check': { + 'mode': 'hash', + 'type': 'sha256', + 'hash': sha256_keys[matching_key], + } + }, + 'size': 4194304 }) + vm.shutdown() + + iotests.img_info_log(remote_path) + # # Invalid path and user # diff --git a/tests/qemu-iotests/207.out b/tests/qemu-iotests/207.out index 1239d9d648..aeb8569d77 100644 --- a/tests/qemu-iotests/207.out +++ b/tests/qemu-iotests/207.out @@ -16,6 +16,8 @@ virtual size: 4 MiB (4194304 bytes) === Test host-key-check options === +--- no host key checking -- + {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"mode": "none"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 8388608}}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job0"}} @@ -25,6 +27,8 @@ image: TEST_IMG file format: IMGFMT virtual size: 8 MiB (8388608 bytes) +--- known_hosts key checking -- + {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"mode": "known_hosts"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 4194304}}} {"return": {}} {"execute": "job-dismiss", "arguments": {"id": "job0"}} @@ -34,6 +38,8 @@ image: TEST_IMG file format: IMGFMT virtual size: 4 MiB (4194304 bytes) +--- explicit md5 key checking -- + {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"hash": "wrong", "mode": "hash", "type": "md5"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 2097152}}} {"return": {}} Job failed: remote host key does not match host_key_check 'wrong' @@ -49,6 +55,8 @@ image: TEST_IMG file format: IMGFMT virtual size: 8 MiB (8388608 bytes) +--- explicit sha1 key checking -- + {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"hash": "wrong", "mode": "hash", "type": "sha1"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 2097152}}} {"return": {}} Job failed: remote host key does not match host_key_check 'wrong' @@ -64,6 +72,23 @@ image: TEST_IMG file format: IMGFMT virtual size: 4 MiB (4194304 bytes) +--- explicit sha256 key checking -- + +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"hash": "wrong", "mode": "hash", "type": "sha256"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 2097152}}} +{"return": {}} +Job failed: remote host key does not match host_key_check 'wrong' +{"execute": "job-dismiss", "arguments": {"id": "job0"}} +{"return": {}} + +{"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"hash": "HASH", "mode": "hash", "type": "sha256"}, "path": "TEST_DIR/PID-t.img", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 4194304}}} +{"return": {}} +{"execute": "job-dismiss", "arguments": {"id": "job0"}} +{"return": {}} + +image: TEST_IMG +file format: IMGFMT +virtual size: 4 MiB (4194304 bytes) + === Invalid path and user === {"execute": "blockdev-create", "arguments": {"job-id": "job0", "options": {"driver": "ssh", "location": {"host-key-check": {"mode": "none"}, "path": "/this/is/not/an/existing/path", "server": {"host": "127.0.0.1", "port": "22"}}, "size": 4194304}}} diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 index fc5297e268..0295129cbb 100755 --- a/tests/qemu-iotests/245 +++ b/tests/qemu-iotests/245 @@ -79,7 +79,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): for line in log.split("\n"): if line.startswith("Pattern verification failed"): raise Exception("%s (command #%d)" % (line, found)) - if re.match("read .*/.* bytes at offset", line): + if re.match("(read|wrote) .*/.* bytes at offset", line): found += 1 self.assertEqual(found, self.total_io_cmds, "Expected output of %d qemu-io commands, found %d" % @@ -146,8 +146,8 @@ class TestBlockdevReopen(iotests.QMPTestCase): self.reopen(opts, {'driver': 'raw'}, "Cannot change the option 'driver'") self.reopen(opts, {'driver': ''}, "Invalid parameter ''") self.reopen(opts, {'driver': None}, "Invalid parameter type for 'driver', expected: string") - self.reopen(opts, {'file': 'not-found'}, "Cannot change the option 'file'") - self.reopen(opts, {'file': ''}, "Cannot change the option 'file'") + self.reopen(opts, {'file': 'not-found'}, "Cannot find device='' nor node-name='not-found'") + self.reopen(opts, {'file': ''}, "Cannot find device='' nor node-name=''") self.reopen(opts, {'file': None}, "Invalid parameter type for 'file', expected: BlockdevRef") self.reopen(opts, {'file.node-name': 'newname'}, "Cannot change the option 'node-name'") self.reopen(opts, {'file.driver': 'host_device'}, "Cannot change the option 'driver'") @@ -443,7 +443,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # Illegal operation: hd2 is a child of hd1 self.reopen(opts[2], {'backing': 'hd1'}, - "Making 'hd1' a backing file of 'hd2' would create a cycle") + "Making 'hd1' a backing child of 'hd2' would create a cycle") # hd2 <- hd0, hd2 <- hd1 self.reopen(opts[0], {'backing': 'hd2'}) @@ -454,8 +454,9 @@ class TestBlockdevReopen(iotests.QMPTestCase): # More illegal operations self.reopen(opts[2], {'backing': 'hd1'}, - "Making 'hd1' a backing file of 'hd2' would create a cycle") - self.reopen(opts[2], {'file': 'hd0-file'}, "Cannot change the option 'file'") + "Making 'hd1' a backing child of 'hd2' would create a cycle") + self.reopen(opts[2], {'file': 'hd0-file'}, + "Permission conflict on node 'hd0-file': permissions 'write, resize' are both required by node 'hd0' (uses node 'hd0-file' as 'file' child) and unshared by node 'hd2' (uses node 'hd0-file' as 'file' child).") result = self.vm.qmp('blockdev-del', conv_keys = True, node_name = 'hd2') self.assert_qmp(result, 'error/class', 'GenericError') @@ -497,18 +498,18 @@ class TestBlockdevReopen(iotests.QMPTestCase): # Illegal: hd2 is backed by hd1 self.reopen(opts[1], {'backing': 'hd2'}, - "Making 'hd2' a backing file of 'hd1' would create a cycle") + "Making 'hd2' a backing child of 'hd1' would create a cycle") # hd1 <- hd0 <- hd2 self.reopen(opts[2], {'backing': 'hd0'}) # Illegal: hd2 is backed by hd0, which is backed by hd1 self.reopen(opts[1], {'backing': 'hd2'}, - "Making 'hd2' a backing file of 'hd1' would create a cycle") + "Making 'hd2' a backing child of 'hd1' would create a cycle") # Illegal: hd1 cannot point to itself self.reopen(opts[1], {'backing': 'hd1'}, - "Making 'hd1' a backing file of 'hd1' would create a cycle") + "Making 'hd1' a backing child of 'hd1' would create a cycle") # Remove all backing files self.reopen(opts[0]) @@ -530,12 +531,119 @@ class TestBlockdevReopen(iotests.QMPTestCase): # Illegal: hd0 is a child of the blkverify node self.reopen(opts[0], {'backing': 'bv'}, - "Making 'bv' a backing file of 'hd0' would create a cycle") + "Making 'bv' a backing child of 'hd0' would create a cycle") # Delete the blkverify node result = self.vm.qmp('blockdev-del', conv_keys = True, node_name = 'bv') self.assert_qmp(result, 'return', {}) + # Replace the protocol layer ('file' parameter) of a disk image + def test_replace_file(self): + # Create two small raw images and add them to a running VM + qemu_img('create', '-f', 'raw', hd_path[0], '10k') + qemu_img('create', '-f', 'raw', hd_path[1], '10k') + + hd0_opts = {'driver': 'file', 'node-name': 'hd0-file', 'filename': hd_path[0] } + hd1_opts = {'driver': 'file', 'node-name': 'hd1-file', 'filename': hd_path[1] } + + result = self.vm.qmp('blockdev-add', conv_keys = False, **hd0_opts) + self.assert_qmp(result, 'return', {}) + result = self.vm.qmp('blockdev-add', conv_keys = False, **hd1_opts) + self.assert_qmp(result, 'return', {}) + + # Add a raw format layer that uses hd0-file as its protocol layer + opts = {'driver': 'raw', 'node-name': 'hd', 'file': 'hd0-file'} + + result = self.vm.qmp('blockdev-add', conv_keys = False, **opts) + self.assert_qmp(result, 'return', {}) + + # Fill the image with data + self.run_qemu_io("hd", "read -P 0 0 10k") + self.run_qemu_io("hd", "write -P 0xa0 0 10k") + + # Replace hd0-file with hd1-file and fill it with (different) data + self.reopen(opts, {'file': 'hd1-file'}) + self.run_qemu_io("hd", "read -P 0 0 10k") + self.run_qemu_io("hd", "write -P 0xa1 0 10k") + + # Use hd0-file again and check that it contains the expected data + self.reopen(opts, {'file': 'hd0-file'}) + self.run_qemu_io("hd", "read -P 0xa0 0 10k") + + # And finally do the same with hd1-file + self.reopen(opts, {'file': 'hd1-file'}) + self.run_qemu_io("hd", "read -P 0xa1 0 10k") + + # Insert (and remove) a throttle filter + def test_insert_throttle_filter(self): + # Add an image to the VM + hd0_opts = hd_opts(0) + result = self.vm.qmp('blockdev-add', conv_keys = False, **hd0_opts) + self.assert_qmp(result, 'return', {}) + + # Create a throttle-group object + opts = { 'qom-type': 'throttle-group', 'id': 'group0', + 'limits': { 'iops-total': 1000 } } + result = self.vm.qmp('object-add', conv_keys = False, **opts) + self.assert_qmp(result, 'return', {}) + + # Add a throttle filter with the group that we just created. + # The filter is not used by anyone yet + opts = { 'driver': 'throttle', 'node-name': 'throttle0', + 'throttle-group': 'group0', 'file': 'hd0-file' } + result = self.vm.qmp('blockdev-add', conv_keys = False, **opts) + self.assert_qmp(result, 'return', {}) + + # Insert the throttle filter between hd0 and hd0-file + self.reopen(hd0_opts, {'file': 'throttle0'}) + + # Remove the throttle filter from hd0 + self.reopen(hd0_opts, {'file': 'hd0-file'}) + + # Insert (and remove) a compress filter + def test_insert_compress_filter(self): + # Add an image to the VM: hd (raw) -> hd0 (qcow2) -> hd0-file (file) + opts = {'driver': 'raw', 'node-name': 'hd', 'file': hd_opts(0)} + result = self.vm.qmp('blockdev-add', conv_keys = False, **opts) + self.assert_qmp(result, 'return', {}) + + # Add a 'compress' filter + filter_opts = {'driver': 'compress', + 'node-name': 'compress0', + 'file': 'hd0'} + result = self.vm.qmp('blockdev-add', conv_keys = False, **filter_opts) + self.assert_qmp(result, 'return', {}) + + # Unmap the beginning of the image (we cannot write compressed + # data to an allocated cluster) + self.run_qemu_io("hd", "write -z -u 0 128k") + + # Write data to the first cluster + self.run_qemu_io("hd", "write -P 0xa0 0 64k") + + # Insert the filter then write to the second cluster + # hd -> compress0 -> hd0 -> hd0-file + self.reopen(opts, {'file': 'compress0'}) + self.run_qemu_io("hd", "write -P 0xa1 64k 64k") + + # Remove the filter then write to the third cluster + # hd -> hd0 -> hd0-file + self.reopen(opts, {'file': 'hd0'}) + self.run_qemu_io("hd", "write -P 0xa2 128k 64k") + + # Verify the data that we just wrote + self.run_qemu_io("hd", "read -P 0xa0 0 64k") + self.run_qemu_io("hd", "read -P 0xa1 64k 64k") + self.run_qemu_io("hd", "read -P 0xa2 128k 64k") + + self.vm.shutdown() + + # Check the first byte of the first three L2 entries and verify that + # the second one is compressed (0x40) while the others are not (0x80) + iotests.qemu_io_log('-f', 'raw', '-c', 'read -P 0x80 0x40000 1', + '-c', 'read -P 0x40 0x40008 1', + '-c', 'read -P 0x80 0x40010 1', hd_path[0]) + # Misc reopen tests with different block drivers @iotests.skip_if_unsupported(['quorum', 'throttle']) def test_misc_drivers(self): @@ -563,7 +671,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # You can't make quorum0 a backing file of hd0: # hd0 is already a child of quorum0. self.reopen(hd_opts(0), {'backing': 'quorum0'}, - "Making 'quorum0' a backing file of 'hd0' would create a cycle") + "Making 'quorum0' a backing child of 'hd0' would create a cycle") # Delete quorum0 result = self.vm.qmp('blockdev-del', conv_keys = True, node_name = 'quorum0') @@ -878,7 +986,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # We can't remove hd1 while the stream job is ongoing opts['backing'] = None - self.reopen(opts, {}, "Cannot change 'backing' link from 'hd0' to 'hd1'") + self.reopen(opts, {}, "Cannot change frozen 'backing' link from 'hd0' to 'hd1'") self.vm.run_job('stream0', auto_finalize = False, auto_dismiss = True) @@ -910,7 +1018,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # We can't remove hd2 while the stream job is ongoing opts['backing']['backing'] = None self.reopen(opts['backing'], {'read-only': False}, - "Cannot change 'backing' link from 'hd1' to 'hd2'") + "Cannot change frozen 'backing' link from 'hd1' to 'hd2'") # We can detach hd1 from hd0 because it doesn't affect the stream job opts['backing'] = None @@ -933,11 +1041,11 @@ class TestBlockdevReopen(iotests.QMPTestCase): # We can't remove hd2 while the commit job is ongoing opts['backing']['backing'] = None - self.reopen(opts, {}, "Cannot change 'backing' link from 'hd1' to 'hd2'") + self.reopen(opts, {}, "Cannot change frozen 'backing' link from 'hd1' to 'hd2'") # We can't remove hd1 while the commit job is ongoing opts['backing'] = None - self.reopen(opts, {}, "Cannot change 'backing' link from 'hd0' to 'hd1'") + self.reopen(opts, {}, "Cannot change frozen 'backing' link from 'hd0' to 'hd1'") event = self.vm.event_wait(name='BLOCK_JOB_READY') self.assert_qmp(event, 'data/device', 'commit0') @@ -969,7 +1077,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # We can't remove hd1 while the commit job is ongoing opts['backing'] = None - self.reopen(opts, {}, "Cannot change backing link if 'hd0' has an implicit backing file") + self.reopen(opts, {}, "Cannot replace implicit backing child of hd0") # hd2 <- hd0 self.vm.run_job('commit0', auto_finalize = False, auto_dismiss = True) diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out index 99c12f4f98..daf1e51922 100644 --- a/tests/qemu-iotests/245.out +++ b/tests/qemu-iotests/245.out @@ -10,8 +10,15 @@ {"return": {}} {"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} {"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} -............... +....read 1/1 bytes at offset 262144 +1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1/1 bytes at offset 262152 +1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1/1 bytes at offset 262160 +1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +.............. ---------------------------------------------------------------------- -Ran 21 tests +Ran 24 tests OK diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 892f7f47d8..ce071b5fc5 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -95,6 +95,7 @@ static int bdrv_test_change_backing_file(BlockDriverState *bs, static BlockDriver bdrv_test = { .format_name = "test", .instance_size = sizeof(BDRVTestState), + .supports_backing = true, .bdrv_close = bdrv_test_close, .bdrv_co_preadv = bdrv_test_co_preadv, diff --git a/tests/unit/test-bdrv-graph-mod.c b/tests/unit/test-bdrv-graph-mod.c index 88f25c0cdb..a6e3bb79be 100644 --- a/tests/unit/test-bdrv-graph-mod.c +++ b/tests/unit/test-bdrv-graph-mod.c @@ -41,6 +41,7 @@ static void no_perm_default_perms(BlockDriverState *bs, BdrvChild *c, static BlockDriver bdrv_no_perm = { .format_name = "no-perm", + .supports_backing = true, .bdrv_child_perm = no_perm_default_perms, };