diff options
author | Peter Maydell <peter.maydell@linaro.org> | 2017-03-01 23:09:46 +0000 |
---|---|---|
committer | Peter Maydell <peter.maydell@linaro.org> | 2017-03-01 23:09:46 +0000 |
commit | b9fe31392bab329219260d846b3002a12667d992 (patch) | |
tree | 8fe0cafd5a4c75abdc82c262cde119f0542e8595 | |
parent | 1e0addb682c3c552fd97480037d4f8ff18e2b87e (diff) | |
parent | b2c2832c6140cfe3ddc0de2d77eeb0b77dea8fd3 (diff) | |
download | qemu-b9fe31392bab329219260d846b3002a12667d992.zip |
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches
# gpg: Signature made Tue 28 Feb 2017 20:35:32 GMT
# gpg: using RSA key 0x7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream: (46 commits)
block: Add Error parameter to bdrv_append()
block: Add Error parameter to bdrv_set_backing_hd()
block: Assertions for resize permission
block: Assertions for write permissions
block: Pass BdrvChild to bdrv_aligned_preadv/pwritev and copy-on-read
tests: Remove FIXME comments
nbd/server: Use real permissions for NBD exports
migration/block: Use real permissions
hmp: Request permissions in qemu-io
commit: Add filter-node-name to block-commit
mirror: Add filter-node-name to blockdev-mirror
stream: Use real permissions in streaming block job
mirror: Use real permissions in mirror/active commit block job
blockjob: Factor out block_job_remove_all_bdrv()
block: Allow backing file links in change_parent_backing_link()
block: BdrvChildRole.attach/detach() callbacks
block: Fix pending requests check in bdrv_append()
backup: Use real permissions in backup block job
commit: Use real permissions for HMP 'commit'
commit: Use real permissions in commit block job
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
71 files changed, 2039 insertions, 392 deletions
@@ -707,6 +707,12 @@ int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough) return 0; } +static char *bdrv_child_get_parent_desc(BdrvChild *c) +{ + BlockDriverState *parent = c->opaque; + return g_strdup(bdrv_get_device_or_node_name(parent)); +} + static void bdrv_child_cb_drained_begin(BdrvChild *child) { BlockDriverState *bs = child->opaque; @@ -774,6 +780,7 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options, } const BdrvChildRole child_file = { + .get_parent_desc = bdrv_child_get_parent_desc, .inherit_options = bdrv_inherited_options, .drained_begin = bdrv_child_cb_drained_begin, .drained_end = bdrv_child_cb_drained_end, @@ -794,11 +801,63 @@ static void bdrv_inherited_fmt_options(int *child_flags, QDict *child_options, } const BdrvChildRole child_format = { + .get_parent_desc = bdrv_child_get_parent_desc, .inherit_options = bdrv_inherited_fmt_options, .drained_begin = bdrv_child_cb_drained_begin, .drained_end = bdrv_child_cb_drained_end, }; +static void bdrv_backing_attach(BdrvChild *c) +{ + BlockDriverState *parent = c->opaque; + BlockDriverState *backing_hd = c->bs; + + assert(!parent->backing_blocker); + error_setg(&parent->backing_blocker, + "node is used as backing hd of '%s'", + bdrv_get_device_or_node_name(parent)); + + parent->open_flags &= ~BDRV_O_NO_BACKING; + pstrcpy(parent->backing_file, sizeof(parent->backing_file), + backing_hd->filename); + pstrcpy(parent->backing_format, sizeof(parent->backing_format), + backing_hd->drv ? backing_hd->drv->format_name : ""); + + bdrv_op_block_all(backing_hd, parent->backing_blocker); + /* Otherwise we won't be able to commit or stream */ + bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET, + parent->backing_blocker); + bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_STREAM, + parent->backing_blocker); + /* + * We do backup in 3 ways: + * 1. drive backup + * The target bs is new opened, and the source is top BDS + * 2. blockdev backup + * Both the source and the target are top BDSes. + * 3. internal backup(used for block replication) + * Both the source and the target are backing file + * + * In case 1 and 2, neither the source nor the target is the backing file. + * In case 3, we will block the top BDS, so there is only one block job + * for the top BDS and its backing chain. + */ + bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE, + parent->backing_blocker); + bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_TARGET, + parent->backing_blocker); +} + +static void bdrv_backing_detach(BdrvChild *c) +{ + BlockDriverState *parent = c->opaque; + + assert(parent->backing_blocker); + bdrv_op_unblock_all(c->bs, parent->backing_blocker); + error_free(parent->backing_blocker); + parent->backing_blocker = NULL; +} + /* * Returns the options and flags that bs->backing should get, based on the * given options and flags for the parent BDS @@ -823,7 +882,10 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options, *child_flags = flags; } -static const BdrvChildRole child_backing = { +const BdrvChildRole child_backing = { + .get_parent_desc = bdrv_child_get_parent_desc, + .attach = bdrv_backing_attach, + .detach = bdrv_backing_detach, .inherit_options = bdrv_backing_options, .drained_begin = bdrv_child_cb_drained_begin, .drained_end = bdrv_child_cb_drained_end, @@ -1326,15 +1388,352 @@ static int bdrv_fill_options(QDict **options, const char *filename, return 0; } -static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs) +/* + * Check whether permissions on this node can be changed in a way that + * @cumulative_perms and @cumulative_shared_perms are the new cumulative + * permissions of all its parents. This involves checking whether all necessary + * permission changes to child nodes can be performed. + * + * A call to this function must always be followed by a call to bdrv_set_perm() + * or bdrv_abort_perm_update(). + */ +static int bdrv_check_perm(BlockDriverState *bs, uint64_t cumulative_perms, + uint64_t cumulative_shared_perms, Error **errp) +{ + BlockDriver *drv = bs->drv; + BdrvChild *c; + int ret; + + /* Write permissions never work with read-only images */ + if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) && + bdrv_is_read_only(bs)) + { + error_setg(errp, "Block node is read-only"); + return -EPERM; + } + + /* Check this node */ + if (!drv) { + return 0; + } + + if (drv->bdrv_check_perm) { + return drv->bdrv_check_perm(bs, cumulative_perms, + cumulative_shared_perms, errp); + } + + /* Drivers that never have children can omit .bdrv_child_perm() */ + if (!drv->bdrv_child_perm) { + assert(QLIST_EMPTY(&bs->children)); + return 0; + } + + /* Check all children */ + QLIST_FOREACH(c, &bs->children, next) { + uint64_t cur_perm, cur_shared; + drv->bdrv_child_perm(bs, c, c->role, + cumulative_perms, cumulative_shared_perms, + &cur_perm, &cur_shared); + ret = bdrv_child_check_perm(c, cur_perm, cur_shared, errp); + if (ret < 0) { + return ret; + } + } + + return 0; +} + +/* + * Notifies drivers that after a previous bdrv_check_perm() call, the + * permission update is not performed and any preparations made for it (e.g. + * taken file locks) need to be undone. + * + * This function recursively notifies all child nodes. + */ +static void bdrv_abort_perm_update(BlockDriverState *bs) +{ + BlockDriver *drv = bs->drv; + BdrvChild *c; + + if (!drv) { + return; + } + + if (drv->bdrv_abort_perm_update) { + drv->bdrv_abort_perm_update(bs); + } + + QLIST_FOREACH(c, &bs->children, next) { + bdrv_child_abort_perm_update(c); + } +} + +static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms, + uint64_t cumulative_shared_perms) +{ + BlockDriver *drv = bs->drv; + BdrvChild *c; + + if (!drv) { + return; + } + + /* Update this node */ + if (drv->bdrv_set_perm) { + drv->bdrv_set_perm(bs, cumulative_perms, cumulative_shared_perms); + } + + /* Drivers that never have children can omit .bdrv_child_perm() */ + if (!drv->bdrv_child_perm) { + assert(QLIST_EMPTY(&bs->children)); + return; + } + + /* Update all children */ + QLIST_FOREACH(c, &bs->children, next) { + uint64_t cur_perm, cur_shared; + drv->bdrv_child_perm(bs, c, c->role, + cumulative_perms, cumulative_shared_perms, + &cur_perm, &cur_shared); + bdrv_child_set_perm(c, cur_perm, cur_shared); + } +} + +static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, + uint64_t *shared_perm) +{ + BdrvChild *c; + uint64_t cumulative_perms = 0; + uint64_t cumulative_shared_perms = BLK_PERM_ALL; + + QLIST_FOREACH(c, &bs->parents, next_parent) { + cumulative_perms |= c->perm; + cumulative_shared_perms &= c->shared_perm; + } + + *perm = cumulative_perms; + *shared_perm = cumulative_shared_perms; +} + +static char *bdrv_child_user_desc(BdrvChild *c) +{ + if (c->role->get_parent_desc) { + return c->role->get_parent_desc(c); + } + + return g_strdup("another user"); +} + +static char *bdrv_perm_names(uint64_t perm) +{ + struct perm_name { + uint64_t perm; + const char *name; + } permissions[] = { + { BLK_PERM_CONSISTENT_READ, "consistent read" }, + { BLK_PERM_WRITE, "write" }, + { BLK_PERM_WRITE_UNCHANGED, "write unchanged" }, + { BLK_PERM_RESIZE, "resize" }, + { BLK_PERM_GRAPH_MOD, "change children" }, + { 0, NULL } + }; + + char *result = g_strdup(""); + struct perm_name *p; + + for (p = permissions; p->name; p++) { + if (perm & p->perm) { + char *old = result; + result = g_strdup_printf("%s%s%s", old, *old ? ", " : "", p->name); + g_free(old); + } + } + + return result; +} + +/* + * Checks whether a new reference to @bs can be added if the new user requires + * @new_used_perm/@new_shared_perm as its permissions. If @ignore_child is set, + * this old reference is ignored in the calculations; this allows checking + * permission updates for an existing reference. + * + * Needs to be followed by a call to either bdrv_set_perm() or + * bdrv_abort_perm_update(). */ +static int bdrv_check_update_perm(BlockDriverState *bs, uint64_t new_used_perm, + uint64_t new_shared_perm, + BdrvChild *ignore_child, Error **errp) +{ + BdrvChild *c; + uint64_t cumulative_perms = new_used_perm; + uint64_t cumulative_shared_perms = new_shared_perm; + + /* There is no reason why anyone couldn't tolerate write_unchanged */ + assert(new_shared_perm & BLK_PERM_WRITE_UNCHANGED); + + QLIST_FOREACH(c, &bs->parents, next_parent) { + if (c == ignore_child) { + continue; + } + + if ((new_used_perm & c->shared_perm) != new_used_perm) { + char *user = bdrv_child_user_desc(c); + char *perm_names = bdrv_perm_names(new_used_perm & ~c->shared_perm); + error_setg(errp, "Conflicts with use by %s as '%s', which does not " + "allow '%s' on %s", + user, c->name, perm_names, bdrv_get_node_name(c->bs)); + g_free(user); + g_free(perm_names); + return -EPERM; + } + + if ((c->perm & new_shared_perm) != c->perm) { + char *user = bdrv_child_user_desc(c); + char *perm_names = bdrv_perm_names(c->perm & ~new_shared_perm); + error_setg(errp, "Conflicts with use by %s as '%s', which uses " + "'%s' on %s", + user, c->name, perm_names, bdrv_get_node_name(c->bs)); + g_free(user); + g_free(perm_names); + return -EPERM; + } + + cumulative_perms |= c->perm; + cumulative_shared_perms &= c->shared_perm; + } + + return bdrv_check_perm(bs, cumulative_perms, cumulative_shared_perms, errp); +} + +/* Needs to be followed by a call to either bdrv_child_set_perm() or + * bdrv_child_abort_perm_update(). */ +int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared, + Error **errp) +{ + return bdrv_check_update_perm(c->bs, perm, shared, c, errp); +} + +void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared) +{ + uint64_t cumulative_perms, cumulative_shared_perms; + + c->perm = perm; + c->shared_perm = shared; + + bdrv_get_cumulative_perm(c->bs, &cumulative_perms, + &cumulative_shared_perms); + bdrv_set_perm(c->bs, cumulative_perms, cumulative_shared_perms); +} + +void bdrv_child_abort_perm_update(BdrvChild *c) +{ + bdrv_abort_perm_update(c->bs); +} + +int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, + Error **errp) +{ + int ret; + + ret = bdrv_child_check_perm(c, perm, shared, errp); + if (ret < 0) { + bdrv_child_abort_perm_update(c); + return ret; + } + + bdrv_child_set_perm(c, perm, shared); + + return 0; +} + +#define DEFAULT_PERM_PASSTHROUGH (BLK_PERM_CONSISTENT_READ \ + | BLK_PERM_WRITE \ + | BLK_PERM_WRITE_UNCHANGED \ + | BLK_PERM_RESIZE) +#define DEFAULT_PERM_UNCHANGED (BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH) + +void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + if (c == NULL) { + *nperm = perm & DEFAULT_PERM_PASSTHROUGH; + *nshared = (shared & DEFAULT_PERM_PASSTHROUGH) | DEFAULT_PERM_UNCHANGED; + return; + } + + *nperm = (perm & DEFAULT_PERM_PASSTHROUGH) | + (c->perm & DEFAULT_PERM_UNCHANGED); + *nshared = (shared & DEFAULT_PERM_PASSTHROUGH) | + (c->shared_perm & DEFAULT_PERM_UNCHANGED); +} + +void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + bool backing = (role == &child_backing); + assert(role == &child_backing || role == &child_file); + + if (!backing) { + /* Apart from the modifications below, the same permissions are + * forwarded and left alone as for filters */ + bdrv_filter_default_perms(bs, c, role, perm, shared, &perm, &shared); + + /* Format drivers may touch metadata even if the guest doesn't write */ + if (!bdrv_is_read_only(bs)) { + perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE; + } + + /* bs->file always needs to be consistent because of the metadata. We + * can never allow other users to resize or write to it. */ + perm |= BLK_PERM_CONSISTENT_READ; + shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE); + } else { + /* We want consistent read from backing files if the parent needs it. + * No other operations are performed on backing files. */ + perm &= BLK_PERM_CONSISTENT_READ; + + /* If the parent can deal with changing data, we're okay with a + * writable and resizable backing file. */ + /* TODO Require !(perm & BLK_PERM_CONSISTENT_READ), too? */ + if (shared & BLK_PERM_WRITE) { + shared = BLK_PERM_WRITE | BLK_PERM_RESIZE; + } else { + shared = 0; + } + + shared |= BLK_PERM_CONSISTENT_READ | BLK_PERM_GRAPH_MOD | + BLK_PERM_WRITE_UNCHANGED; + } + + *nperm = perm; + *nshared = shared; +} + +static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs, + bool check_new_perm) { BlockDriverState *old_bs = child->bs; + uint64_t perm, shared_perm; if (old_bs) { if (old_bs->quiesce_counter && child->role->drained_end) { child->role->drained_end(child); } + if (child->role->detach) { + child->role->detach(child); + } QLIST_REMOVE(child, next_parent); + + /* Update permissions for old node. This is guaranteed to succeed + * because we're just taking a parent away, so we're loosening + * restrictions. */ + bdrv_get_cumulative_perm(old_bs, &perm, &shared_perm); + bdrv_check_perm(old_bs, perm, shared_perm, &error_abort); + bdrv_set_perm(old_bs, perm, shared_perm); } child->bs = new_bs; @@ -1344,23 +1743,46 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs) if (new_bs->quiesce_counter && child->role->drained_begin) { child->role->drained_begin(child); } + + bdrv_get_cumulative_perm(new_bs, &perm, &shared_perm); + if (check_new_perm) { + bdrv_check_perm(new_bs, perm, shared_perm, &error_abort); + } + bdrv_set_perm(new_bs, perm, shared_perm); + + if (child->role->attach) { + child->role->attach(child); + } } } BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, const char *child_name, const BdrvChildRole *child_role, - void *opaque) + uint64_t perm, uint64_t shared_perm, + void *opaque, Error **errp) { - BdrvChild *child = g_new(BdrvChild, 1); + BdrvChild *child; + int ret; + + ret = bdrv_check_update_perm(child_bs, perm, shared_perm, NULL, errp); + if (ret < 0) { + bdrv_abort_perm_update(child_bs); + return NULL; + } + + child = g_new(BdrvChild, 1); *child = (BdrvChild) { - .bs = NULL, - .name = g_strdup(child_name), - .role = child_role, - .opaque = opaque, + .bs = NULL, + .name = g_strdup(child_name), + .role = child_role, + .perm = perm, + .shared_perm = shared_perm, + .opaque = opaque, }; - bdrv_replace_child(child, child_bs); + /* This performs the matching bdrv_set_perm() for the above check. */ + bdrv_replace_child(child, child_bs, false); return child; } @@ -1368,10 +1790,24 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, BlockDriverState *child_bs, const char *child_name, - const BdrvChildRole *child_role) + const BdrvChildRole *child_role, + Error **errp) { - BdrvChild *child = bdrv_root_attach_child(child_bs, child_name, child_role, - parent_bs); + BdrvChild *child; + uint64_t perm, shared_perm; + + bdrv_get_cumulative_perm(parent_bs, &perm, &shared_perm); + + assert(parent_bs->drv); + parent_bs->drv->bdrv_child_perm(parent_bs, NULL, child_role, + perm, shared_perm, &perm, &shared_perm); + + child = bdrv_root_attach_child(child_bs, child_name, child_role, + perm, shared_perm, parent_bs, errp); + if (child == NULL) { + return NULL; + } + QLIST_INSERT_HEAD(&parent_bs->children, child, next); return child; } @@ -1383,7 +1819,7 @@ static void bdrv_detach_child(BdrvChild *child) child->next.le_prev = NULL; } - bdrv_replace_child(child, NULL); + bdrv_replace_child(child, NULL, false); g_free(child->name); g_free(child); @@ -1447,57 +1883,28 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs) * Sets the backing file link of a BDS. A new reference is created; callers * which don't need their own reference any more must call bdrv_unref(). */ -void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd) +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, + Error **errp) { if (backing_hd) { bdrv_ref(backing_hd); } if (bs->backing) { - assert(bs->backing_blocker); - bdrv_op_unblock_all(bs->backing->bs, bs->backing_blocker); bdrv_unref_child(bs, bs->backing); - } else if (backing_hd) { - error_setg(&bs->backing_blocker, - "node is used as backing hd of '%s'", - bdrv_get_device_or_node_name(bs)); } if (!backing_hd) { - error_free(bs->backing_blocker); - bs->backing_blocker = NULL; bs->backing = NULL; goto out; } - bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing); - bs->open_flags &= ~BDRV_O_NO_BACKING; - pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->filename); - pstrcpy(bs->backing_format, sizeof(bs->backing_format), - backing_hd->drv ? backing_hd->drv->format_name : ""); - bdrv_op_block_all(backing_hd, bs->backing_blocker); - /* Otherwise we won't be able to commit or stream */ - bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_COMMIT_TARGET, - bs->backing_blocker); - bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_STREAM, - bs->backing_blocker); - /* - * We do backup in 3 ways: - * 1. drive backup - * The target bs is new opened, and the source is top BDS - * 2. blockdev backup - * Both the source and the target are top BDSes. - * 3. internal backup(used for block replication) - * Both the source and the target are backing file - * - * In case 1 and 2, neither the source nor the target is the backing file. - * In case 3, we will block the top BDS, so there is only one block job - * for the top BDS and its backing chain. - */ - bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_SOURCE, - bs->backing_blocker); - bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_TARGET, - bs->backing_blocker); + bs->backing = bdrv_attach_child(bs, backing_hd, "backing", &child_backing, + errp); + if (!bs->backing) { + bdrv_unref(backing_hd); + } + out: bdrv_refresh_limits(bs, NULL); } @@ -1580,8 +1987,12 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, /* Hook up the backing file link; drop our reference, bs owns the * backing_hd reference now */ - bdrv_set_backing_hd(bs, backing_hd); + bdrv_set_backing_hd(bs, backing_hd, &local_err); bdrv_unref(backing_hd); + if (local_err) { + ret = -EINVAL; + goto free_exit; + } qdict_del(parent_options, bdref_key); @@ -1648,6 +2059,7 @@ BdrvChild *bdrv_open_child(const char *filename, const BdrvChildRole *child_role, bool allow_none, Error **errp) { + BdrvChild *c; BlockDriverState *bs; bs = bdrv_open_child_bs(filename, options, bdref_key, parent, child_role, @@ -1656,7 +2068,13 @@ BdrvChild *bdrv_open_child(const char *filename, return NULL; } - return bdrv_attach_child(parent, bs, bdref_key, child_role); + c = bdrv_attach_child(parent, bs, bdref_key, child_role, errp); + if (!c) { + bdrv_unref(bs); + return NULL; + } + + return c; } static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, @@ -1669,6 +2087,7 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, int64_t total_size; QemuOpts *opts = NULL; BlockDriverState *bs_snapshot; + Error *local_err = NULL; int ret; /* if snapshot, we create a temporary backing file and open it @@ -1718,7 +2137,12 @@ static BlockDriverState *bdrv_append_temp_snapshot(BlockDriverState *bs, * call bdrv_unref() on it), so in order to be able to return one, we have * to increase bs_snapshot's refcount here */ bdrv_ref(bs_snapshot); - bdrv_append(bs_snapshot, bs); + bdrv_append(bs_snapshot, bs, &local_err); + if (local_err) { + error_propagate(errp, local_err); + ret = -EINVAL; + goto out; + } g_free(tmp_filename); return bs_snapshot; @@ -1862,9 +2286,12 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, goto fail; } if (file_bs != NULL) { - file = blk_new(); - blk_insert_bs(file, file_bs); + file = blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL); + blk_insert_bs(file, file_bs, &local_err); bdrv_unref(file_bs); + if (local_err) { + goto fail; + } qdict_put(options, "file", qstring_from_str(bdrv_get_node_name(file_bs))); @@ -2405,7 +2832,7 @@ static void bdrv_close(BlockDriverState *bs) bs->drv->bdrv_close(bs); bs->drv = NULL; - bdrv_set_backing_hd(bs, NULL); + bdrv_set_backing_hd(bs, NULL, &error_abort); if (bs->file != NULL) { bdrv_unref_child(bs, bs->file); @@ -2465,10 +2892,13 @@ static void change_parent_backing_link(BlockDriverState *from, BdrvChild *c, *next, *to_c; QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { + if (c->role->stay_at_node) { + continue; + } if (c->role == &child_backing) { - /* @from is generally not allowed to be a backing file, except for - * when @to is the overlay. In that case, @from may not be replaced - * by @to as @to's backing node. */ + /* If @from is a backing file of @to, ignore the child to avoid + * creating a loop. We only want to change the pointer of other + * parents. */ QLIST_FOREACH(to_c, &to->children, next) { if (to_c == c) { break; @@ -2479,9 +2909,10 @@ static void change_parent_backing_link(BlockDriverState *from, } } - assert(c->role != &child_backing); bdrv_ref(to); - bdrv_replace_child(c, to); + /* FIXME Are we sure that bdrv_replace_child() can't run into + * &error_abort because of permissions? */ + bdrv_replace_child(c, to, true); bdrv_unref(from); } } @@ -2502,19 +2933,25 @@ static void change_parent_backing_link(BlockDriverState *from, * parents of bs_top after bdrv_append() returns. If the caller needs to keep a * reference of its own, it must call bdrv_ref(). */ -void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top) +void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, + Error **errp) { - assert(!bdrv_requests_pending(bs_top)); - assert(!bdrv_requests_pending(bs_new)); + Error *local_err = NULL; - bdrv_ref(bs_top); + assert(!atomic_read(&bs_top->in_flight)); + assert(!atomic_read(&bs_new->in_flight)); + + bdrv_set_backing_hd(bs_new, bs_top, &local_err); + if (local_err) { + error_propagate(errp, local_err); + goto out; + } change_parent_backing_link(bs_top, bs_new); - bdrv_set_backing_hd(bs_new, bs_top); - bdrv_unref(bs_top); /* bs_new is now referenced by its new parents, we don't need the * additional reference any more. */ +out: bdrv_unref(bs_new); } @@ -2658,6 +3095,7 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, BlockDriverState *base, const char *backing_file_str) { BlockDriverState *new_top_bs = NULL; + Error *local_err = NULL; int ret = -EIO; if (!top->drv || !base->drv) { @@ -2690,7 +3128,13 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, if (ret) { goto exit; } - bdrv_set_backing_hd(new_top_bs, base); + + bdrv_set_backing_hd(new_top_bs, base, &local_err); + if (local_err) { + ret = -EPERM; + error_report_err(local_err); + goto exit; + } ret = 0; exit: @@ -2705,6 +3149,9 @@ int bdrv_truncate(BdrvChild *child, int64_t offset) BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; int ret; + + assert(child->perm & BLK_PERM_RESIZE); + if (!drv) return -ENOMEDIUM; if (!drv->bdrv_truncate) diff --git a/block/backup.c b/block/backup.c index fe010e78e3..d1ab617c7e 100644 --- a/block/backup.c +++ b/block/backup.c @@ -618,14 +618,24 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, goto error; } - job = block_job_create(job_id, &backup_job_driver, bs, speed, - creation_flags, cb, opaque, errp); + /* job->common.len is fixed, so we can't allow resize */ + job = block_job_create(job_id, &backup_job_driver, bs, + BLK_PERM_CONSISTENT_READ, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD, + speed, creation_flags, cb, opaque, errp); if (!job) { goto error; } - job->target = blk_new(); - blk_insert_bs(job->target, target); + /* The target must match the source in size, so no resize here either */ + job->target = blk_new(BLK_PERM_WRITE, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE | + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_GRAPH_MOD); + ret = blk_insert_bs(job->target, target, errp); + if (ret < 0) { + goto error; + } job->on_source_error = on_source_error; job->on_target_error = on_target_error; @@ -652,7 +662,9 @@ BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs, job->cluster_size = MAX(BACKUP_CLUSTER_SIZE_DEFAULT, bdi.cluster_size); } - block_job_add_bdrv(&job->common, target); + /* Required permissions are already taken with target's blk_new() */ + block_job_add_bdrv(&job->common, "target", target, 0, BLK_PERM_ALL, + &error_abort); job->common.len = len; block_job_txn_add_job(txn, &job->common); diff --git a/block/blkdebug.c b/block/blkdebug.c index 6117ce5fca..67e8024e36 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -734,6 +734,8 @@ static BlockDriver bdrv_blkdebug = { .bdrv_file_open = blkdebug_open, .bdrv_close = blkdebug_close, .bdrv_reopen_prepare = blkdebug_reopen_prepare, + .bdrv_child_perm = bdrv_filter_default_perms, + .bdrv_getlength = blkdebug_getlength, .bdrv_truncate = blkdebug_truncate, .bdrv_refresh_filename = blkdebug_refresh_filename, diff --git a/block/blkreplay.c b/block/blkreplay.c index cfc8c5be02..e1102119fb 100755 --- a/block/blkreplay.c +++ b/block/blkreplay.c @@ -137,6 +137,7 @@ static BlockDriver bdrv_blkreplay = { .bdrv_file_open = blkreplay_open, .bdrv_close = blkreplay_close, + .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_getlength = blkreplay_getlength, .bdrv_co_preadv = blkreplay_co_preadv, diff --git a/block/blkverify.c b/block/blkverify.c index 43a940c2f5..9a1e21c6ad 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -320,6 +320,7 @@ static BlockDriver bdrv_blkverify = { .bdrv_parse_filename = blkverify_parse_filename, .bdrv_file_open = blkverify_open, .bdrv_close = blkverify_close, + .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_getlength = blkverify_getlength, .bdrv_refresh_filename = blkverify_refresh_filename, diff --git a/block/block-backend.c b/block/block-backend.c index 492e71e41f..daa7908d01 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -59,6 +59,9 @@ struct BlockBackend { bool iostatus_enabled; BlockDeviceIoStatus iostatus; + uint64_t perm; + uint64_t shared_perm; + bool allow_write_beyond_eof; NotifierList remove_bs_notifiers, insert_bs_notifiers; @@ -77,6 +80,7 @@ static const AIOCBInfo block_backend_aiocb_info = { static void drive_info_del(DriveInfo *dinfo); static BlockBackend *bdrv_first_blk(BlockDriverState *bs); +static char *blk_get_attached_dev_id(BlockBackend *blk); /* All BlockBackends */ static QTAILQ_HEAD(, BlockBackend) block_backends = @@ -99,6 +103,25 @@ static void blk_root_drained_end(BdrvChild *child); static void blk_root_change_media(BdrvChild *child, bool load); static void blk_root_resize(BdrvChild *child); +static char *blk_root_get_parent_desc(BdrvChild *child) +{ + BlockBackend *blk = child->opaque; + char *dev_id; + + if (blk->name) { + return g_strdup(blk->name); + } + + dev_id = blk_get_attached_dev_id(blk); + if (*dev_id) { + return dev_id; + } else { + /* TODO Callback into the BB owner for something more detailed */ + g_free(dev_id); + return g_strdup("a block device"); + } +} + static const char *blk_root_get_name(BdrvChild *child) { return blk_name(child->opaque); @@ -110,6 +133,7 @@ static const BdrvChildRole child_root = { .change_media = blk_root_change_media, .resize = blk_root_resize, .get_name = blk_root_get_name, + .get_parent_desc = blk_root_get_parent_desc, .drained_begin = blk_root_drained_begin, .drained_end = blk_root_drained_end, @@ -117,15 +141,23 @@ static const BdrvChildRole child_root = { /* * Create a new BlockBackend with a reference count of one. - * Store an error through @errp on failure, unless it's null. + * + * @perm is a bitmasks of BLK_PERM_* constants which describes the permissions + * to request for a block driver node that is attached to this BlockBackend. + * @shared_perm is a bitmask which describes which permissions may be granted + * to other users of the attached node. + * Both sets of permissions can be changed later using blk_set_perm(). + * * Return the new BlockBackend on success, null on failure. */ -BlockBackend *blk_new(void) +BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm) { BlockBackend *blk; blk = g_new0(BlockBackend, 1); blk->refcnt = 1; + blk->perm = perm; + blk->shared_perm = shared_perm; blk_set_enable_write_cache(blk, true); qemu_co_queue_init(&blk->public.throttled_reqs[0]); @@ -155,15 +187,33 @@ BlockBackend *blk_new_open(const char *filename, const char *reference, { BlockBackend *blk; BlockDriverState *bs; + uint64_t perm; + + /* blk_new_open() is mainly used in .bdrv_create implementations and the + * tools where sharing isn't a concern because the BDS stays private, so we + * just request permission according to the flags. + * + * The exceptions are xen_disk and blockdev_init(); in these cases, the + * caller of blk_new_open() doesn't make use of the permissions, but they + * shouldn't hurt either. We can still share everything here because the + * guest devices will add their own blockers if they can't share. */ + perm = BLK_PERM_CONSISTENT_READ; + if (flags & BDRV_O_RDWR) { + perm |= BLK_PERM_WRITE; + } + if (flags & BDRV_O_RESIZE) { + perm |= BLK_PERM_RESIZE; + } - blk = blk_new(); + blk = blk_new(perm, BLK_PERM_ALL); bs = bdrv_open(filename, reference, options, flags, errp); if (!bs) { blk_unref(blk); return NULL; } - blk->root = bdrv_root_attach_child(bs, "root", &child_root, blk); + blk->root = bdrv_root_attach_child(bs, "root", &child_root, + perm, BLK_PERM_ALL, blk, &error_abort); return blk; } @@ -495,16 +545,49 @@ void blk_remove_bs(BlockBackend *blk) /* * Associates a new BlockDriverState with @blk. */ -void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs) +int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp) { + blk->root = bdrv_root_attach_child(bs, "root", &child_root, + blk->perm, blk->shared_perm, blk, errp); + if (blk->root == NULL) { + return -EPERM; + } bdrv_ref(bs); - blk->root = bdrv_root_attach_child(bs, "root", &child_root, blk); notifier_list_notify(&blk->insert_bs_notifiers, blk); if (blk->public.throttle_state) { throttle_timers_attach_aio_context( &blk->public.throttle_timers, bdrv_get_aio_context(bs)); } + + return 0; +} + +/* + * Sets the permission bitmasks that the user of the BlockBackend needs. + */ +int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm, + Error **errp) +{ + int ret; + + if (blk->root) { + ret = bdrv_child_try_set_perm(blk->root, perm, shared_perm, errp); + if (ret < 0) { + return ret; + } + } + + blk->perm = perm; + blk->shared_perm = shared_perm; + + return 0; +} + +void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm) +{ + *perm = blk->perm; + *shared_perm = blk->shared_perm; } static int blk_do_attach_dev(BlockBackend *blk, void *dev) @@ -553,6 +636,7 @@ void blk_detach_dev(BlockBackend *blk, void *dev) blk->dev_ops = NULL; blk->dev_opaque = NULL; blk->guest_block_size = 512; + blk_set_perm(blk, 0, BLK_PERM_ALL, &error_abort); blk_unref(blk); } @@ -620,19 +704,29 @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, /* * Notify @blk's attached device model of media change. - * If @load is true, notify of media load. - * Else, notify of media eject. + * + * If @load is true, notify of media load. This action can fail, meaning that + * the medium cannot be loaded. @errp is set then. + * + * If @load is false, notify of media eject. This can never fail. + * * Also send DEVICE_TRAY_MOVED events as appropriate. */ -void blk_dev_change_media_cb(BlockBackend *blk, bool load) +void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp) { if (blk->dev_ops && blk->dev_ops->change_media_cb) { bool tray_was_open, tray_is_open; + Error *local_err = NULL; assert(!blk->legacy_dev); tray_was_open = blk_dev_is_tray_open(blk); - blk->dev_ops->change_media_cb(blk->dev_opaque, load); + blk->dev_ops->change_media_cb(blk->dev_opaque, load, &local_err); + if (local_err) { + assert(load == true); + error_propagate(errp, local_err); + return; + } tray_is_open = blk_dev_is_tray_open(blk); if (tray_was_open != tray_is_open) { @@ -646,7 +740,7 @@ void blk_dev_change_media_cb(BlockBackend *blk, bool load) static void blk_root_change_media(BdrvChild *child, bool load) { - blk_dev_change_media_cb(child->opaque, load); + blk_dev_change_media_cb(child->opaque, load, NULL); } /* diff --git a/block/bochs.c b/block/bochs.c index 7dd2ac4f51..516da56c3b 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -293,6 +293,7 @@ static BlockDriver bdrv_bochs = { .instance_size = sizeof(BDRVBochsState), .bdrv_probe = bochs_probe, .bdrv_open = bochs_open, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_refresh_limits = bochs_refresh_limits, .bdrv_co_preadv = bochs_co_preadv, .bdrv_close = bochs_close, diff --git a/block/cloop.c b/block/cloop.c index 877c9b0d1b..a6c7b9dbe6 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -290,6 +290,7 @@ static BlockDriver bdrv_cloop = { .instance_size = sizeof(BDRVCloopState), .bdrv_probe = cloop_probe, .bdrv_open = cloop_open, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_refresh_limits = cloop_refresh_limits, .bdrv_co_preadv = cloop_co_preadv, .bdrv_close = cloop_close, diff --git a/block/commit.c b/block/commit.c index c284e8535d..22a0a4db98 100644 --- a/block/commit.c +++ b/block/commit.c @@ -36,6 +36,7 @@ typedef struct CommitBlockJob { BlockJob common; RateLimit limit; BlockDriverState *active; + BlockDriverState *commit_top_bs; BlockBackend *top; BlockBackend *base; BlockdevOnError on_error; @@ -83,12 +84,23 @@ static void commit_complete(BlockJob *job, void *opaque) BlockDriverState *active = s->active; BlockDriverState *top = blk_bs(s->top); BlockDriverState *base = blk_bs(s->base); - BlockDriverState *overlay_bs = bdrv_find_overlay(active, top); + BlockDriverState *overlay_bs = bdrv_find_overlay(active, s->commit_top_bs); int ret = data->ret; + bool remove_commit_top_bs = false; + + /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before + * the normal backing chain can be restored. */ + blk_unref(s->base); if (!block_job_is_cancelled(&s->common) && ret == 0) { /* success */ - ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str); + ret = bdrv_drop_intermediate(active, s->commit_top_bs, base, + s->backing_file_str); + } else if (overlay_bs) { + /* XXX Can (or should) we somehow keep 'consistent read' blocked even + * after the failed/cancelled commit job is gone? If we already wrote + * something to base, the intermediate images aren't valid any more. */ + remove_commit_top_bs = true; } /* restore base open flags here if appropriate (e.g., change the base back @@ -102,9 +114,15 @@ static void commit_complete(BlockJob *job, void *opaque) } g_free(s->backing_file_str); blk_unref(s->top); - blk_unref(s->base); block_job_completed(&s->common, ret); g_free(data); + + /* If bdrv_drop_intermediate() didn't already do that, remove the commit + * filter driver from the backing chain. Do this as the final step so that + * the 'consistent read' permission can be granted. */ + if (remove_commit_top_bs) { + bdrv_set_backing_hd(overlay_bs, top, &error_abort); + } } static void coroutine_fn commit_run(void *opaque) @@ -208,10 +226,38 @@ static const BlockJobDriver commit_job_driver = { .start = commit_run, }; +static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags) +{ + return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags); +} + +static void bdrv_commit_top_close(BlockDriverState *bs) +{ +} + +static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + *nperm = 0; + *nshared = BLK_PERM_ALL; +} + +/* Dummy node that provides consistent read to its users without requiring it + * from its backing file and that allows writes on the backing file chain. */ +static BlockDriver bdrv_commit_top = { + .format_name = "commit_top", + .bdrv_co_preadv = bdrv_commit_top_preadv, + .bdrv_close = bdrv_commit_top_close, + .bdrv_child_perm = bdrv_commit_top_child_perm, +}; + void commit_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, BlockDriverState *top, int64_t speed, BlockdevOnError on_error, const char *backing_file_str, - Error **errp) + const char *filter_node_name, Error **errp) { CommitBlockJob *s; BlockReopenQueue *reopen_queue = NULL; @@ -219,7 +265,9 @@ void commit_start(const char *job_id, BlockDriverState *bs, int orig_base_flags; BlockDriverState *iter; BlockDriverState *overlay_bs; + BlockDriverState *commit_top_bs = NULL; Error *local_err = NULL; + int ret; assert(top != bs); if (top == base) { @@ -234,8 +282,8 @@ void commit_start(const char *job_id, BlockDriverState *bs, return; } - s = block_job_create(job_id, &commit_job_driver, bs, speed, - BLOCK_JOB_DEFAULT, NULL, NULL, errp); + s = block_job_create(job_id, &commit_job_driver, bs, 0, BLK_PERM_ALL, + speed, BLOCK_JOB_DEFAULT, NULL, NULL, errp); if (!s) { return; } @@ -256,30 +304,70 @@ void commit_start(const char *job_id, BlockDriverState *bs, bdrv_reopen_multiple(bdrv_get_aio_context(bs), reopen_queue, &local_err); if (local_err != NULL) { error_propagate(errp, local_err); - block_job_unref(&s->common); - return; + goto fail; } } + /* Insert commit_top block node above top, so we can block consistent read + * on the backing chain below it */ + commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, filter_node_name, 0, + errp); + if (commit_top_bs == NULL) { + goto fail; + } + + bdrv_set_backing_hd(commit_top_bs, top, &error_abort); + bdrv_set_backing_hd(overlay_bs, commit_top_bs, &error_abort); + + s->commit_top_bs = commit_top_bs; + bdrv_unref(commit_top_bs); /* Block all nodes between top and base, because they will * disappear from the chain after this operation. */ assert(bdrv_chain_contains(top, base)); - for (iter = top; iter != backing_bs(base); iter = backing_bs(iter)) { - block_job_add_bdrv(&s->common, iter); + for (iter = top; iter != base; iter = backing_bs(iter)) { + /* XXX BLK_PERM_WRITE needs to be allowed so we don't block ourselves + * at s->base (if writes are blocked for a node, they are also blocked + * for its backing file). The other options would be a second filter + * driver above s->base. */ + ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0, + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE, + errp); + if (ret < 0) { + goto fail; + } + } + + ret = block_job_add_bdrv(&s->common, "base", base, 0, BLK_PERM_ALL, errp); + if (ret < 0) { + goto fail; } + /* overlay_bs must be blocked because it needs to be modified to - * update the backing image string, but if it's the root node then - * don't block it again */ - if (bs != overlay_bs) { - block_job_add_bdrv(&s->common, overlay_bs); + * update the backing image string. */ + ret = block_job_add_bdrv(&s->common, "overlay of top", overlay_bs, + BLK_PERM_GRAPH_MOD, BLK_PERM_ALL, errp); + if (ret < 0) { + goto fail; } - s->base = blk_new(); - blk_insert_bs(s->base, base); + s->base = blk_new(BLK_PERM_CONSISTENT_READ + | BLK_PERM_WRITE + | BLK_PERM_RESIZE, + BLK_PERM_CONSISTENT_READ + | BLK_PERM_GRAPH_MOD + | BLK_PERM_WRITE_UNCHANGED); + ret = blk_insert_bs(s->base, base, errp); + if (ret < 0) { + goto fail; + } - s->top = blk_new(); - blk_insert_bs(s->top, top); + /* Required permissions are already taken with block_job_add_bdrv() */ + s->top = blk_new(0, BLK_PERM_ALL); + blk_insert_bs(s->top, top, errp); + if (ret < 0) { + goto fail; + } s->active = bs; @@ -292,6 +380,19 @@ void commit_start(const char *job_id, BlockDriverState *bs, trace_commit_start(bs, base, top, s); block_job_start(&s->common); + return; + +fail: + if (s->base) { + blk_unref(s->base); + } + if (s->top) { + blk_unref(s->top); + } + if (commit_top_bs) { + bdrv_set_backing_hd(overlay_bs, top, &error_abort); + } + block_job_unref(&s->common); } @@ -301,11 +402,14 @@ void commit_start(const char *job_id, BlockDriverState *bs, int bdrv_commit(BlockDriverState *bs) { BlockBackend *src, *backing; + BlockDriverState *backing_file_bs = NULL; + BlockDriverState *commit_top_bs = NULL; BlockDriver *drv = bs->drv; int64_t sector, total_sectors, length, backing_length; int n, ro, open_flags; int ret = 0; uint8_t *buf = NULL; + Error *local_err = NULL; if (!drv) return -ENOMEDIUM; @@ -328,11 +432,33 @@ int bdrv_commit(BlockDriverState *bs) } } - src = blk_new(); - blk_insert_bs(src, bs); + src = blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL); + backing = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE, BLK_PERM_ALL); - backing = blk_new(); - blk_insert_bs(backing, bs->backing->bs); + ret = blk_insert_bs(src, bs, &local_err); + if (ret < 0) { + error_report_err(local_err); + goto ro_cleanup; + } + + /* Insert commit_top block node above backing, so we can write to it */ + backing_file_bs = backing_bs(bs); + + commit_top_bs = bdrv_new_open_driver(&bdrv_commit_top, NULL, BDRV_O_RDWR, + &local_err); + if (commit_top_bs == NULL) { + error_report_err(local_err); + goto ro_cleanup; + } + + bdrv_set_backing_hd(commit_top_bs, backing_file_bs, &error_abort); + bdrv_set_backing_hd(bs, commit_top_bs, &error_abort); + + ret = blk_insert_bs(backing, backing_file_bs, &local_err); + if (ret < 0) { + error_report_err(local_err); + goto ro_cleanup; + } length = blk_getlength(src); if (length < 0) { @@ -404,8 +530,12 @@ int bdrv_commit(BlockDriverState *bs) ro_cleanup: qemu_vfree(buf); - blk_unref(src); blk_unref(backing); + if (backing_file_bs) { + bdrv_set_backing_hd(bs, backing_file_bs, &error_abort); + } + bdrv_unref(commit_top_bs); + blk_unref(src); if (ro) { /* ignoring error return here */ diff --git a/block/crypto.c b/block/crypto.c index 7cb2ff2946..4a2038888d 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -628,6 +628,7 @@ BlockDriver bdrv_crypto_luks = { .bdrv_probe = block_crypto_probe_luks, .bdrv_open = block_crypto_open_luks, .bdrv_close = block_crypto_close, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_create = block_crypto_create_luks, .bdrv_truncate = block_crypto_truncate, .create_opts = &block_crypto_create_opts_luks, diff --git a/block/dmg.c b/block/dmg.c index 8e387cdfe5..a7d25fc47b 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -697,6 +697,7 @@ static BlockDriver bdrv_dmg = { .bdrv_probe = dmg_probe, .bdrv_open = dmg_open, .bdrv_refresh_limits = dmg_refresh_limits, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_co_preadv = dmg_co_preadv, .bdrv_close = dmg_close, }; diff --git a/block/io.c b/block/io.c index d5c45447fd..8f38d46de0 100644 --- a/block/io.c +++ b/block/io.c @@ -925,9 +925,11 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset, return drv->bdrv_co_pwritev_compressed(bs, offset, bytes, qiov); } -static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs, +static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t offset, unsigned int bytes, QEMUIOVector *qiov) { + BlockDriverState *bs = child->bs; + /* Perform I/O through a temporary buffer so that users who scribble over * their read buffer while the operation is in progress do not end up * modifying the image file. This is critical for zero-copy guest I/O @@ -943,6 +945,8 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs, size_t skip_bytes; int ret; + assert(child->perm & (BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE)); + /* Cover entire cluster so no additional backing file I/O is required when * allocating cluster in the image file. */ @@ -1001,10 +1005,11 @@ err: * handles copy on read, zeroing after EOF, and fragmentation of large * reads; any other features must be implemented by the caller. */ -static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs, +static int coroutine_fn bdrv_aligned_preadv(BdrvChild *child, BdrvTrackedRequest *req, int64_t offset, unsigned int bytes, int64_t align, QEMUIOVector *qiov, int flags) { + BlockDriverState *bs = child->bs; int64_t total_bytes, max_bytes; int ret = 0; uint64_t bytes_remaining = bytes; @@ -1050,7 +1055,7 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs, } if (!ret || pnum != nb_sectors) { - ret = bdrv_co_do_copy_on_readv(bs, offset, bytes, qiov); + ret = bdrv_co_do_copy_on_readv(child, offset, bytes, qiov); goto out; } } @@ -1158,7 +1163,7 @@ int coroutine_fn bdrv_co_preadv(BdrvChild *child, } tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_READ); - ret = bdrv_aligned_preadv(bs, &req, offset, bytes, align, + ret = bdrv_aligned_preadv(child, &req, offset, bytes, align, use_local_qiov ? &local_qiov : qiov, flags); tracked_request_end(&req); @@ -1306,10 +1311,11 @@ fail: * Forwards an already correctly aligned write request to the BlockDriver, * after possibly fragmenting it. */ -static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, +static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child, BdrvTrackedRequest *req, int64_t offset, unsigned int bytes, int64_t align, QEMUIOVector *qiov, int flags) { + BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; bool waited; int ret; @@ -1332,6 +1338,8 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, assert(!waited || !req->serialising); assert(req->overlap_offset <= offset); assert(offset + bytes <= req->overlap_offset + req->overlap_bytes); + assert(child->perm & BLK_PERM_WRITE); + assert(end_sector <= bs->total_sectors || child->perm & BLK_PERM_RESIZE); ret = notifier_with_return_list_notify(&bs->before_write_notifiers, req); @@ -1397,12 +1405,13 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs, return ret; } -static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, +static int coroutine_fn bdrv_co_do_zero_pwritev(BdrvChild *child, int64_t offset, unsigned int bytes, BdrvRequestFlags flags, BdrvTrackedRequest *req) { + BlockDriverState *bs = child->bs; uint8_t *buf = NULL; QEMUIOVector local_qiov; struct iovec iov; @@ -1430,7 +1439,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, mark_request_serialising(req, align); wait_serialising_requests(req); bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD); - ret = bdrv_aligned_preadv(bs, req, offset & ~(align - 1), align, + ret = bdrv_aligned_preadv(child, req, offset & ~(align - 1), align, align, &local_qiov, 0); if (ret < 0) { goto fail; @@ -1438,7 +1447,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_HEAD); memset(buf + head_padding_bytes, 0, zero_bytes); - ret = bdrv_aligned_pwritev(bs, req, offset & ~(align - 1), align, + ret = bdrv_aligned_pwritev(child, req, offset & ~(align - 1), align, align, &local_qiov, flags & ~BDRV_REQ_ZERO_WRITE); if (ret < 0) { @@ -1452,7 +1461,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, if (bytes >= align) { /* Write the aligned part in the middle. */ uint64_t aligned_bytes = bytes & ~(align - 1); - ret = bdrv_aligned_pwritev(bs, req, offset, aligned_bytes, align, + ret = bdrv_aligned_pwritev(child, req, offset, aligned_bytes, align, NULL, flags); if (ret < 0) { goto fail; @@ -1468,7 +1477,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, mark_request_serialising(req, align); wait_serialising_requests(req); bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL); - ret = bdrv_aligned_preadv(bs, req, offset, align, + ret = bdrv_aligned_preadv(child, req, offset, align, align, &local_qiov, 0); if (ret < 0) { goto fail; @@ -1476,7 +1485,7 @@ static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_AFTER_TAIL); memset(buf, 0, bytes); - ret = bdrv_aligned_pwritev(bs, req, offset, align, align, + ret = bdrv_aligned_pwritev(child, req, offset, align, align, &local_qiov, flags & ~BDRV_REQ_ZERO_WRITE); } fail: @@ -1523,7 +1532,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_WRITE); if (!qiov) { - ret = bdrv_co_do_zero_pwritev(bs, offset, bytes, flags, &req); + ret = bdrv_co_do_zero_pwritev(child, offset, bytes, flags, &req); goto out; } @@ -1542,7 +1551,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, qemu_iovec_init_external(&head_qiov, &head_iov, 1); bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_HEAD); - ret = bdrv_aligned_preadv(bs, &req, offset & ~(align - 1), align, + ret = bdrv_aligned_preadv(child, &req, offset & ~(align - 1), align, align, &head_qiov, 0); if (ret < 0) { goto fail; @@ -1584,8 +1593,8 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, qemu_iovec_init_external(&tail_qiov, &tail_iov, 1); bdrv_debug_event(bs, BLKDBG_PWRITEV_RMW_TAIL); - ret = bdrv_aligned_preadv(bs, &req, (offset + bytes) & ~(align - 1), align, - align, &tail_qiov, 0); + ret = bdrv_aligned_preadv(child, &req, (offset + bytes) & ~(align - 1), + align, align, &tail_qiov, 0); if (ret < 0) { goto fail; } @@ -1603,7 +1612,7 @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child, bytes = ROUND_UP(bytes, align); } - ret = bdrv_aligned_pwritev(bs, &req, offset, bytes, align, + ret = bdrv_aligned_pwritev(child, &req, offset, bytes, align, use_local_qiov ? &local_qiov : qiov, flags); diff --git a/block/mirror.c b/block/mirror.c index 1b34b366d0..57f26c33a4 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -38,7 +38,10 @@ typedef struct MirrorBlockJob { BlockJob common; RateLimit limit; BlockBackend *target; + BlockDriverState *mirror_top_bs; + BlockDriverState *source; BlockDriverState *base; + /* The name of the graph node to replace */ char *replaces; /* The BDS to replace */ @@ -327,7 +330,7 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s, static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) { - BlockDriverState *source = blk_bs(s->common.blk); + BlockDriverState *source = s->source; int64_t sector_num, first_chunk; uint64_t delay_ns = 0; /* At least the first dirty chunk is mirrored in one iteration. */ @@ -497,12 +500,30 @@ static void mirror_exit(BlockJob *job, void *opaque) MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); MirrorExitData *data = opaque; AioContext *replace_aio_context = NULL; - BlockDriverState *src = blk_bs(s->common.blk); + BlockDriverState *src = s->source; BlockDriverState *target_bs = blk_bs(s->target); + BlockDriverState *mirror_top_bs = s->mirror_top_bs; + Error *local_err = NULL; /* Make sure that the source BDS doesn't go away before we called * block_job_completed(). */ bdrv_ref(src); + bdrv_ref(mirror_top_bs); + + /* We don't access the source any more. Dropping any WRITE/RESIZE is + * required before it could become a backing file of target_bs. */ + bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, + &error_abort); + if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { + BlockDriverState *backing = s->is_none_mode ? src : s->base; + if (backing_bs(target_bs) != backing) { + bdrv_set_backing_hd(target_bs, backing, &local_err); + if (local_err) { + error_report_err(local_err); + data->ret = -EPERM; + } + } + } if (s->to_replace) { replace_aio_context = bdrv_get_aio_context(s->to_replace); @@ -524,10 +545,6 @@ static void mirror_exit(BlockJob *job, void *opaque) bdrv_drained_begin(target_bs); bdrv_replace_in_backing_chain(to_replace, target_bs); bdrv_drained_end(target_bs); - - /* We just changed the BDS the job BB refers to */ - blk_remove_bs(job->blk); - blk_insert_bs(job->blk, src); } if (s->to_replace) { bdrv_op_unblock_all(s->to_replace, s->replace_blocker); @@ -540,9 +557,26 @@ static void mirror_exit(BlockJob *job, void *opaque) g_free(s->replaces); blk_unref(s->target); s->target = NULL; + + /* Remove the mirror filter driver from the graph. Before this, get rid of + * the blockers on the intermediate nodes so that the resulting state is + * valid. */ + block_job_remove_all_bdrv(job); + bdrv_replace_in_backing_chain(mirror_top_bs, backing_bs(mirror_top_bs)); + + /* We just changed the BDS the job BB refers to (with either or both of the + * bdrv_replace_in_backing_chain() calls), so switch the BB back so the + * cleanup does the right thing. We don't need any permissions any more + * now. */ + blk_remove_bs(job->blk); + blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); + blk_insert_bs(job->blk, mirror_top_bs, &error_abort); + block_job_completed(&s->common, data->ret); + g_free(data); bdrv_drained_end(src); + bdrv_unref(mirror_top_bs); bdrv_unref(src); } @@ -562,7 +596,7 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) { int64_t sector_num, end; BlockDriverState *base = s->base; - BlockDriverState *bs = blk_bs(s->common.blk); + BlockDriverState *bs = s->source; BlockDriverState *target_bs = blk_bs(s->target); int ret, n; @@ -644,7 +678,7 @@ static void coroutine_fn mirror_run(void *opaque) { MirrorBlockJob *s = opaque; MirrorExitData *data; - BlockDriverState *bs = blk_bs(s->common.blk); + BlockDriverState *bs = s->source; BlockDriverState *target_bs = blk_bs(s->target); bool need_drain = true; int64_t length; @@ -876,9 +910,8 @@ static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp) static void mirror_complete(BlockJob *job, Error **errp) { MirrorBlockJob *s = container_of(job, MirrorBlockJob, common); - BlockDriverState *src, *target; + BlockDriverState *target; - src = blk_bs(job->blk); target = blk_bs(s->target); if (!s->synced) { @@ -910,6 +943,10 @@ static void mirror_complete(BlockJob *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. */ error_setg(&s->replace_blocker, "block device is in use by block-job-complete"); bdrv_op_block_all(s->to_replace, s->replace_blocker); @@ -918,13 +955,6 @@ static void mirror_complete(BlockJob *job, Error **errp) aio_context_release(replace_aio_context); } - if (s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { - BlockDriverState *backing = s->is_none_mode ? src : s->base; - if (backing_bs(target) != backing) { - bdrv_set_backing_hd(target, backing); - } - } - s->should_complete = true; block_job_enter(&s->common); } @@ -980,6 +1010,77 @@ static const BlockJobDriver commit_active_job_driver = { .drain = mirror_drain, }; +static int coroutine_fn bdrv_mirror_top_preadv(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags) +{ + return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags); +} + +static int coroutine_fn bdrv_mirror_top_pwritev(BlockDriverState *bs, + uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags) +{ + return bdrv_co_pwritev(bs->backing, offset, bytes, qiov, flags); +} + +static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs) +{ + return bdrv_co_flush(bs->backing->bs); +} + +static int64_t coroutine_fn bdrv_mirror_top_get_block_status( + BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, + BlockDriverState **file) +{ + *pnum = nb_sectors; + *file = bs->backing->bs; + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA | + (sector_num << BDRV_SECTOR_BITS); +} + +static int coroutine_fn bdrv_mirror_top_pwrite_zeroes(BlockDriverState *bs, + int64_t offset, int count, BdrvRequestFlags flags) +{ + return bdrv_co_pwrite_zeroes(bs->backing, offset, count, flags); +} + +static int coroutine_fn bdrv_mirror_top_pdiscard(BlockDriverState *bs, + int64_t offset, int count) +{ + return bdrv_co_pdiscard(bs->backing->bs, offset, count); +} + +static void bdrv_mirror_top_close(BlockDriverState *bs) +{ +} + +static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + /* Must be able to forward guest writes to the real image */ + *nperm = 0; + if (perm & BLK_PERM_WRITE) { + *nperm |= BLK_PERM_WRITE; + } + + *nshared = BLK_PERM_ALL; +} + +/* Dummy node that provides consistent read to its users without requiring it + * from its backing file and that allows writes on the backing file chain. */ +static BlockDriver bdrv_mirror_top = { + .format_name = "mirror_top", + .bdrv_co_preadv = bdrv_mirror_top_preadv, + .bdrv_co_pwritev = bdrv_mirror_top_pwritev, + .bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes, + .bdrv_co_pdiscard = bdrv_mirror_top_pdiscard, + .bdrv_co_flush = bdrv_mirror_top_flush, + .bdrv_co_get_block_status = bdrv_mirror_top_get_block_status, + .bdrv_close = bdrv_mirror_top_close, + .bdrv_child_perm = bdrv_mirror_top_child_perm, +}; + static void mirror_start_job(const char *job_id, BlockDriverState *bs, int creation_flags, BlockDriverState *target, const char *replaces, int64_t speed, @@ -992,9 +1093,14 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, void *opaque, Error **errp, const BlockJobDriver *driver, bool is_none_mode, BlockDriverState *base, - bool auto_complete) + bool auto_complete, const char *filter_node_name) { MirrorBlockJob *s; + BlockDriverState *mirror_top_bs; + bool target_graph_mod; + bool target_is_backing; + Error *local_err = NULL; + int ret; if (granularity == 0) { granularity = bdrv_get_default_bitmap_granularity(target); @@ -1011,14 +1117,62 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, buf_size = DEFAULT_MIRROR_BUF_SIZE; } - s = block_job_create(job_id, driver, bs, speed, creation_flags, - cb, opaque, errp); - if (!s) { + /* In the case of active commit, add dummy driver to provide consistent + * reads on the top, while disabling it in the intermediate nodes, and make + * the backing chain writable. */ + mirror_top_bs = bdrv_new_open_driver(&bdrv_mirror_top, filter_node_name, + BDRV_O_RDWR, errp); + if (mirror_top_bs == NULL) { + return; + } + mirror_top_bs->total_sectors = bs->total_sectors; + + /* bdrv_append takes ownership of the mirror_top_bs reference, need to keep + * it alive until block_job_create() even if bs has no parent. */ + bdrv_ref(mirror_top_bs); + bdrv_drained_begin(bs); + bdrv_append(mirror_top_bs, bs, &local_err); + bdrv_drained_end(bs); + + if (local_err) { + bdrv_unref(mirror_top_bs); + error_propagate(errp, local_err); return; } - s->target = blk_new(); - blk_insert_bs(s->target, target); + /* Make sure that the source is not resized while the job is running */ + s = block_job_create(job_id, driver, mirror_top_bs, + BLK_PERM_CONSISTENT_READ, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | + BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed, + creation_flags, cb, opaque, errp); + bdrv_unref(mirror_top_bs); + if (!s) { + goto fail; + } + s->source = bs; + s->mirror_top_bs = mirror_top_bs; + + /* No resize for the target either; while the mirror is still running, a + * consistent read isn't necessarily possible. We could possibly allow + * writes and graph modifications, though it would likely defeat the + * purpose of a mirror, so leave them blocked for now. + * + * In the case of active commit, things look a bit different, though, + * because the target is an already populated backing file in active use. + * We can allow anything except resize there.*/ + target_is_backing = bdrv_chain_contains(bs, target); + target_graph_mod = (backing_mode != MIRROR_LEAVE_BACKING_CHAIN); + s->target = blk_new(BLK_PERM_WRITE | BLK_PERM_RESIZE | + (target_graph_mod ? BLK_PERM_GRAPH_MOD : 0), + BLK_PERM_WRITE_UNCHANGED | + (target_is_backing ? BLK_PERM_CONSISTENT_READ | + BLK_PERM_WRITE | + BLK_PERM_GRAPH_MOD : 0)); + ret = blk_insert_bs(s->target, target, errp); + if (ret < 0) { + goto fail; + } s->replaces = g_strdup(replaces); s->on_source_error = on_source_error; @@ -1041,18 +1195,40 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, return; } - block_job_add_bdrv(&s->common, target); + /* Required permissions are already taken with blk_new() */ + block_job_add_bdrv(&s->common, "target", target, 0, BLK_PERM_ALL, + &error_abort); + /* In commit_active_start() all intermediate nodes disappear, so * any jobs in them must be blocked */ - if (bdrv_chain_contains(bs, target)) { + if (target_is_backing) { BlockDriverState *iter; for (iter = backing_bs(bs); iter != target; iter = backing_bs(iter)) { - block_job_add_bdrv(&s->common, iter); + /* XXX BLK_PERM_WRITE needs to be allowed so we don't block + * ourselves at s->base (if writes are blocked for a node, they are + * also blocked for its backing file). The other options would be a + * second filter driver above s->base (== target). */ + ret = block_job_add_bdrv(&s->common, "intermediate node", iter, 0, + BLK_PERM_WRITE_UNCHANGED | BLK_PERM_WRITE, + errp); + if (ret < 0) { + goto fail; + } } } trace_mirror_start(bs, s, opaque); block_job_start(&s->common); + return; + +fail: + if (s) { + g_free(s->replaces); + blk_unref(s->target); + block_job_unref(&s->common); + } + + bdrv_replace_in_backing_chain(mirror_top_bs, backing_bs(mirror_top_bs)); } void mirror_start(const char *job_id, BlockDriverState *bs, @@ -1061,7 +1237,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs, MirrorSyncMode mode, BlockMirrorBackingMode backing_mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, - bool unmap, Error **errp) + bool unmap, const char *filter_node_name, Error **errp) { bool is_none_mode; BlockDriverState *base; @@ -1075,12 +1251,14 @@ void mirror_start(const char *job_id, BlockDriverState *bs, mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces, speed, granularity, buf_size, backing_mode, on_source_error, on_target_error, unmap, NULL, NULL, errp, - &mirror_job_driver, is_none_mode, base, false); + &mirror_job_driver, is_none_mode, base, false, + filter_node_name); } void commit_active_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, int creation_flags, int64_t speed, BlockdevOnError on_error, + const char *filter_node_name, BlockCompletionFunc *cb, void *opaque, Error **errp, bool auto_complete) { @@ -1096,7 +1274,8 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0, MIRROR_LEAVE_BACKING_CHAIN, on_error, on_error, true, cb, opaque, &local_err, - &commit_active_job_driver, false, base, auto_complete); + &commit_active_job_driver, false, base, auto_complete, + filter_node_name); if (local_err) { error_propagate(errp, local_err); goto error_restore_flags; diff --git a/block/parallels.c b/block/parallels.c index b2ec09f7e6..19935e29a9 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -488,7 +488,8 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp) } file = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (file == NULL) { error_propagate(errp, local_err); return -EIO; @@ -762,6 +763,7 @@ static BlockDriver bdrv_parallels = { .bdrv_probe = parallels_probe, .bdrv_open = parallels_open, .bdrv_close = parallels_close, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_co_get_block_status = parallels_co_get_block_status, .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_co_flush_to_os = parallels_co_flush_to_os, diff --git a/block/qcow.c b/block/qcow.c index 038b05ab1b..9d6ac83959 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -823,7 +823,8 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp) } qcow_blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (qcow_blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -1052,6 +1053,7 @@ static BlockDriver bdrv_qcow = { .bdrv_probe = qcow_probe, .bdrv_open = qcow_open, .bdrv_close = qcow_close, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_reopen_prepare = qcow_reopen_prepare, .bdrv_create = qcow_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, diff --git a/block/qcow2.c b/block/qcow2.c index 21e61427eb..6a92d2ef3f 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2202,7 +2202,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, } blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); return -EIO; @@ -2266,7 +2267,8 @@ static int qcow2_create2(const char *filename, int64_t total_size, options = qdict_new(); qdict_put(options, "driver", qstring_from_str("qcow2")); blk = blk_new_open(filename, NULL, options, - BDRV_O_RDWR | BDRV_O_NO_FLUSH, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_NO_FLUSH, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -3113,6 +3115,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, uint64_t cluster_size = s->cluster_size; bool encrypt; int refcount_bits = s->refcount_bits; + Error *local_err = NULL; int ret; QemuOptDesc *desc = opts->list->desc; Qcow2AmendHelperCBInfo helper_cb_info; @@ -3262,11 +3265,16 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, } if (new_size) { - BlockBackend *blk = blk_new(); - blk_insert_bs(blk, bs); + BlockBackend *blk = blk_new(BLK_PERM_RESIZE, BLK_PERM_ALL); + ret = blk_insert_bs(blk, bs, &local_err); + if (ret < 0) { + error_report_err(local_err); + blk_unref(blk); + return ret; + } + ret = blk_truncate(blk, new_size); blk_unref(blk); - if (ret < 0) { return ret; } @@ -3403,6 +3411,7 @@ BlockDriver bdrv_qcow2 = { .bdrv_reopen_commit = qcow2_reopen_commit, .bdrv_reopen_abort = qcow2_reopen_abort, .bdrv_join_options = qcow2_join_options, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_create = qcow2_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_co_get_block_status = qcow2_co_get_block_status, diff --git a/block/qed.c b/block/qed.c index 62a0a09326..5ec7fd83f2 100644 --- a/block/qed.c +++ b/block/qed.c @@ -625,7 +625,8 @@ static int qed_create(const char *filename, uint32_t cluster_size, } blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); return -EIO; @@ -1704,6 +1705,7 @@ static BlockDriver bdrv_qed = { .bdrv_open = bdrv_qed_open, .bdrv_close = bdrv_qed_close, .bdrv_reopen_prepare = bdrv_qed_reopen_prepare, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_create = bdrv_qed_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_co_get_block_status = bdrv_qed_co_get_block_status, diff --git a/block/quorum.c b/block/quorum.c index 86e2072dce..40205fb1b3 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1032,10 +1032,17 @@ static void quorum_add_child(BlockDriverState *bs, BlockDriverState *child_bs, /* We can safely add the child now */ bdrv_ref(child_bs); - child = bdrv_attach_child(bs, child_bs, indexstr, &child_format); + + child = bdrv_attach_child(bs, child_bs, indexstr, &child_format, errp); + if (child == NULL) { + s->next_child_index--; + bdrv_unref(child_bs); + goto out; + } s->children = g_renew(BdrvChild *, s->children, s->num_children + 1); s->children[s->num_children++] = child; +out: bdrv_drained_end(bs); } @@ -1126,6 +1133,8 @@ static BlockDriver bdrv_quorum = { .bdrv_add_child = quorum_add_child, .bdrv_del_child = quorum_del_child, + .bdrv_child_perm = bdrv_filter_default_perms, + .is_filter = true, .bdrv_recurse_is_first_non_filter = quorum_recurse_is_first_non_filter, }; diff --git a/block/raw-format.c b/block/raw-format.c index ce34d1b1cd..86fbc657eb 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -467,6 +467,7 @@ BlockDriver bdrv_raw = { .bdrv_reopen_abort = &raw_reopen_abort, .bdrv_open = &raw_open, .bdrv_close = &raw_close, + .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_create = &raw_create, .bdrv_co_preadv = &raw_co_preadv, .bdrv_co_pwritev = &raw_co_pwritev, diff --git a/block/replication.c b/block/replication.c index eff85c77ba..22f170fd33 100644 --- a/block/replication.c +++ b/block/replication.c @@ -644,7 +644,7 @@ static void replication_stop(ReplicationState *rs, bool failover, Error **errp) s->replication_state = BLOCK_REPLICATION_FAILOVER; commit_active_start(NULL, s->active_disk->bs, s->secondary_disk->bs, BLOCK_JOB_INTERNAL, 0, BLOCKDEV_ON_ERROR_REPORT, - replication_done, bs, errp, true); + NULL, replication_done, bs, errp, true); break; default: aio_context_release(aio_context); @@ -660,6 +660,7 @@ BlockDriver bdrv_replication = { .bdrv_open = replication_open, .bdrv_close = replication_close, + .bdrv_child_perm = bdrv_filter_default_perms, .bdrv_getlength = replication_getlength, .bdrv_co_readv = replication_co_readv, diff --git a/block/sheepdog.c b/block/sheepdog.c index 860ba61502..743471043e 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -1609,7 +1609,7 @@ static int sd_prealloc(const char *filename, Error **errp) int ret; blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, errp); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, errp); if (blk == NULL) { ret = -EIO; goto out_with_err_set; diff --git a/block/stream.c b/block/stream.c index 1523ba7dfb..0113710845 100644 --- a/block/stream.c +++ b/block/stream.c @@ -68,6 +68,7 @@ static void stream_complete(BlockJob *job, void *opaque) StreamCompleteData *data = opaque; BlockDriverState *bs = blk_bs(job->blk); BlockDriverState *base = s->base; + Error *local_err = NULL; if (!block_job_is_cancelled(&s->common) && data->reached_end && data->ret == 0) { @@ -79,11 +80,19 @@ static void stream_complete(BlockJob *job, void *opaque) } } data->ret = bdrv_change_backing_file(bs, base_id, base_fmt); - bdrv_set_backing_hd(bs, base); + bdrv_set_backing_hd(bs, base, &local_err); + if (local_err) { + error_report_err(local_err); + data->ret = -EPERM; + goto out; + } } +out: /* Reopen the image back in read-only mode if necessary */ if (s->bs_flags != bdrv_get_flags(bs)) { + /* Give up write permissions before making it read-only */ + blk_set_perm(job->blk, 0, BLK_PERM_ALL, &error_abort); bdrv_reopen(bs, s->bs_flags, NULL); } @@ -229,25 +238,35 @@ void stream_start(const char *job_id, BlockDriverState *bs, BlockDriverState *iter; int orig_bs_flags; - s = block_job_create(job_id, &stream_job_driver, bs, speed, - BLOCK_JOB_DEFAULT, NULL, NULL, errp); - if (!s) { - return; - } - /* Make sure that the image is opened in read-write mode */ orig_bs_flags = bdrv_get_flags(bs); if (!(orig_bs_flags & BDRV_O_RDWR)) { if (bdrv_reopen(bs, orig_bs_flags | BDRV_O_RDWR, errp) != 0) { - block_job_unref(&s->common); return; } } - /* Block all intermediate nodes between bs and base, because they - * will disappear from the chain after this operation */ + /* Prevent concurrent jobs trying to modify the graph structure here, we + * already have our own plans. Also don't allow resize as the image size is + * queried only at the job start and then cached. */ + s = block_job_create(job_id, &stream_job_driver, bs, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | + BLK_PERM_GRAPH_MOD, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | + BLK_PERM_WRITE, + speed, BLOCK_JOB_DEFAULT, NULL, NULL, errp); + if (!s) { + goto fail; + } + + /* Block all intermediate nodes between bs and base, because they will + * disappear from the chain after this operation. The streaming job reads + * every block only once, assuming that it doesn't change, so block writes + * and resizes. */ for (iter = backing_bs(bs); iter && iter != base; iter = backing_bs(iter)) { - block_job_add_bdrv(&s->common, iter); + block_job_add_bdrv(&s->common, "intermediate node", iter, 0, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED, + &error_abort); } s->base = base; @@ -257,4 +276,10 @@ void stream_start(const char *job_id, BlockDriverState *bs, s->on_error = on_error; trace_stream_start(bs, base, s); block_job_start(&s->common); + return; + +fail: + if (orig_bs_flags != bdrv_get_flags(bs)) { + bdrv_reopen(bs, s->bs_flags, NULL); + } } diff --git a/block/vdi.c b/block/vdi.c index 18b4773aac..9b4f70e977 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -763,7 +763,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp) } blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -891,6 +892,7 @@ static BlockDriver bdrv_vdi = { .bdrv_open = vdi_open, .bdrv_close = vdi_close, .bdrv_reopen_prepare = vdi_reopen_prepare, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_create = vdi_create, .bdrv_has_zero_init = bdrv_has_zero_init_1, .bdrv_co_get_block_status = vdi_co_get_block_status, diff --git a/block/vhdx.c b/block/vhdx.c index 9918ee98ff..052a753159 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1859,7 +1859,8 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp) } blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -1983,6 +1984,7 @@ static BlockDriver bdrv_vhdx = { .bdrv_open = vhdx_open, .bdrv_close = vhdx_close, .bdrv_reopen_prepare = vhdx_reopen_prepare, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_co_readv = vhdx_co_readv, .bdrv_co_writev = vhdx_co_writev, .bdrv_create = vhdx_create, diff --git a/block/vmdk.c b/block/vmdk.c index 9d68ec5a4e..a9bd22bf93 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1703,7 +1703,8 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, } blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -2071,7 +2072,8 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp) } new_blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (new_blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -2359,6 +2361,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_open = vmdk_open, .bdrv_check = vmdk_check, .bdrv_reopen_prepare = vmdk_reopen_prepare, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_co_preadv = vmdk_co_preadv, .bdrv_co_pwritev = vmdk_co_pwritev, .bdrv_co_pwritev_compressed = vmdk_co_pwritev_compressed, diff --git a/block/vpc.c b/block/vpc.c index d0df2a1c54..f591d4be38 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -915,7 +915,8 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp) } blk = blk_new_open(filename, NULL, NULL, - BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err); + BDRV_O_RDWR | BDRV_O_RESIZE | BDRV_O_PROTOCOL, + &local_err); if (blk == NULL) { error_propagate(errp, local_err); ret = -EIO; @@ -1067,6 +1068,7 @@ static BlockDriver bdrv_vpc = { .bdrv_open = vpc_open, .bdrv_close = vpc_close, .bdrv_reopen_prepare = vpc_reopen_prepare, + .bdrv_child_perm = bdrv_format_default_perms, .bdrv_create = vpc_create, .bdrv_co_preadv = vpc_co_preadv, diff --git a/block/vvfat.c b/block/vvfat.c index 7f230be006..aa61c329e7 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3041,7 +3041,7 @@ static int enable_write_target(BlockDriverState *bs, Error **errp) &error_abort); *(void**) backing->opaque = s; - bdrv_set_backing_hd(s->bs, backing); + bdrv_set_backing_hd(s->bs, backing, &error_abort); bdrv_unref(backing); return 0; @@ -3052,6 +3052,27 @@ err: return ret; } +static void vvfat_child_perm(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared) +{ + BDRVVVFATState *s = bs->opaque; + + assert(c == s->qcow || role == &child_backing); + + if (c == s->qcow) { + /* This is a private node, nobody should try to attach to it */ + *nperm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE; + *nshared = BLK_PERM_WRITE_UNCHANGED; + } else { + /* The backing file is there so 'commit' can use it. vvfat doesn't + * access it in any way. */ + *nperm = 0; + *nshared = BLK_PERM_ALL; + } +} + static void vvfat_close(BlockDriverState *bs) { BDRVVVFATState *s = bs->opaque; @@ -3077,6 +3098,7 @@ static BlockDriver bdrv_vvfat = { .bdrv_file_open = vvfat_open, .bdrv_refresh_limits = vvfat_refresh_limits, .bdrv_close = vvfat_close, + .bdrv_child_perm = vvfat_child_perm, .bdrv_co_preadv = vvfat_co_preadv, .bdrv_co_pwritev = vvfat_co_pwritev, diff --git a/blockdev.c b/blockdev.c index 8682bd81d8..8eb4e84fe0 100644 --- a/blockdev.c +++ b/blockdev.c @@ -558,7 +558,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts, if ((!file || !*file) && !qdict_size(bs_opts)) { BlockBackendRootState *blk_rs; - blk = blk_new(); + blk = blk_new(0, BLK_PERM_ALL); blk_rs = blk_get_root_state(blk); blk_rs->open_flags = bdrv_flags; blk_rs->read_only = read_only; @@ -1768,6 +1768,17 @@ static void external_snapshot_prepare(BlkActionState *common, if (!state->new_bs->drv->supports_backing) { error_setg(errp, "The snapshot does not support backing images"); + return; + } + + /* This removes our old bs and adds the new bs. This is an operation that + * can fail, so we need to do it in .prepare; undoing it for abort is + * always possible. */ + bdrv_ref(state->new_bs); + bdrv_append(state->new_bs, state->old_bs, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; } } @@ -1778,8 +1789,6 @@ static void external_snapshot_commit(BlkActionState *common) bdrv_set_aio_context(state->new_bs, state->aio_context); - /* This removes our old bs and adds the new bs */ - bdrv_append(state->new_bs, state->old_bs); /* We don't need (or want) to use the transactional * bdrv_reopen_multiple() across all the entries at once, because we * don't want to abort all of them if one of them fails the reopen */ @@ -1794,7 +1803,9 @@ static void external_snapshot_abort(BlkActionState *common) ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); if (state->new_bs) { - bdrv_unref(state->new_bs); + if (state->new_bs->backing) { + bdrv_replace_in_backing_chain(state->new_bs, state->old_bs); + } } } @@ -1805,6 +1816,7 @@ static void external_snapshot_clean(BlkActionState *common) if (state->aio_context) { bdrv_drained_end(state->old_bs); aio_context_release(state->aio_context); + bdrv_unref(state->new_bs); } } @@ -2311,7 +2323,7 @@ static int do_open_tray(const char *blk_name, const char *qdev_id, } if (!locked || force) { - blk_dev_change_media_cb(blk, false); + blk_dev_change_media_cb(blk, false, &error_abort); } if (locked && !force) { @@ -2349,6 +2361,7 @@ void qmp_blockdev_close_tray(bool has_device, const char *device, Error **errp) { BlockBackend *blk; + Error *local_err = NULL; device = has_device ? device : NULL; id = has_id ? id : NULL; @@ -2372,7 +2385,11 @@ void qmp_blockdev_close_tray(bool has_device, const char *device, return; } - blk_dev_change_media_cb(blk, true); + blk_dev_change_media_cb(blk, true, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } } void qmp_x_blockdev_remove_medium(bool has_device, const char *device, @@ -2425,7 +2442,7 @@ void qmp_x_blockdev_remove_medium(bool has_device, const char *device, * called at all); therefore, the medium needs to be ejected here. * Do it after blk_remove_bs() so blk_is_inserted(blk) returns the @load * value passed here (i.e. false). */ - blk_dev_change_media_cb(blk, false); + blk_dev_change_media_cb(blk, false, &error_abort); } out: @@ -2435,7 +2452,9 @@ out: static void qmp_blockdev_insert_anon_medium(BlockBackend *blk, BlockDriverState *bs, Error **errp) { + Error *local_err = NULL; bool has_device; + int ret; /* For BBs without a device, we can exchange the BDS tree at will */ has_device = blk_get_attached_dev(blk); @@ -2455,7 +2474,10 @@ static void qmp_blockdev_insert_anon_medium(BlockBackend *blk, return; } - blk_insert_bs(blk, bs); + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + return; + } if (!blk_dev_has_tray(blk)) { /* For tray-less devices, blockdev-close-tray is a no-op (or may not be @@ -2463,7 +2485,12 @@ static void qmp_blockdev_insert_anon_medium(BlockBackend *blk, * slot here. * Do it after blk_insert_bs() so blk_is_inserted(blk) returns the @load * value passed here (i.e. true). */ - blk_dev_change_media_cb(blk, true); + blk_dev_change_media_cb(blk, true, &local_err); + if (local_err) { + error_propagate(errp, local_err); + blk_remove_bs(blk); + return; + } } } @@ -2890,8 +2917,11 @@ void qmp_block_resize(bool has_device, const char *device, goto out; } - blk = blk_new(); - blk_insert_bs(blk, bs); + blk = blk_new(BLK_PERM_RESIZE, BLK_PERM_ALL); + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto out; + } /* complete all in-flight operations before resizing the device */ bdrv_drain_all(); @@ -3014,6 +3044,7 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, bool has_top, const char *top, bool has_backing_file, const char *backing_file, bool has_speed, int64_t speed, + bool has_filter_node_name, const char *filter_node_name, Error **errp) { BlockDriverState *bs; @@ -3029,6 +3060,9 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, if (!has_speed) { speed = 0; } + if (!has_filter_node_name) { + filter_node_name = NULL; + } /* Important Note: * libvirt relies on the DeviceNotFound error class in order to probe for @@ -3103,8 +3137,8 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, goto out; } commit_active_start(has_job_id ? job_id : NULL, bs, base_bs, - BLOCK_JOB_DEFAULT, speed, on_error, NULL, NULL, - &local_err, false); + BLOCK_JOB_DEFAULT, speed, on_error, + filter_node_name, NULL, NULL, &local_err, false); } else { BlockDriverState *overlay_bs = bdrv_find_overlay(bs, top_bs); if (bdrv_op_is_blocked(overlay_bs, BLOCK_OP_TYPE_COMMIT_TARGET, errp)) { @@ -3112,7 +3146,7 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device, } commit_start(has_job_id ? job_id : NULL, bs, base_bs, top_bs, speed, on_error, has_backing_file ? backing_file : NULL, - &local_err); + filter_node_name, &local_err); } if (local_err != NULL) { error_propagate(errp, local_err); @@ -3348,6 +3382,8 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, bool has_on_target_error, BlockdevOnError on_target_error, bool has_unmap, bool unmap, + bool has_filter_node_name, + const char *filter_node_name, Error **errp) { @@ -3369,6 +3405,9 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, if (!has_unmap) { unmap = true; } + if (!has_filter_node_name) { + filter_node_name = NULL; + } if (granularity != 0 && (granularity < 512 || granularity > 1048576 * 64)) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "granularity", @@ -3398,7 +3437,8 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs, mirror_start(job_id, bs, target, has_replaces ? replaces : NULL, speed, granularity, buf_size, sync, backing_mode, - on_source_error, on_target_error, unmap, errp); + on_source_error, on_target_error, unmap, filter_node_name, + errp); } void qmp_drive_mirror(DriveMirror *arg, Error **errp) @@ -3536,6 +3576,7 @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp) arg->has_on_source_error, arg->on_source_error, arg->has_on_target_error, arg->on_target_error, arg->has_unmap, arg->unmap, + false, NULL, &local_err); bdrv_unref(target_bs); error_propagate(errp, local_err); @@ -3554,6 +3595,8 @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id, BlockdevOnError on_source_error, bool has_on_target_error, BlockdevOnError on_target_error, + bool has_filter_node_name, + const char *filter_node_name, Error **errp) { BlockDriverState *bs; @@ -3585,6 +3628,7 @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id, has_on_source_error, on_source_error, has_on_target_error, on_target_error, true, true, + has_filter_node_name, filter_node_name, &local_err); error_propagate(errp, local_err); diff --git a/blockjob.c b/blockjob.c index abee11bb08..69126af97f 100644 --- a/blockjob.c +++ b/blockjob.c @@ -55,6 +55,19 @@ struct BlockJobTxn { static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs); +static char *child_job_get_parent_desc(BdrvChild *c) +{ + BlockJob *job = c->opaque; + return g_strdup_printf("%s job '%s'", + BlockJobType_lookup[job->driver->job_type], + job->id); +} + +static const BdrvChildRole child_job = { + .get_parent_desc = child_job_get_parent_desc, + .stay_at_node = true, +}; + BlockJob *block_job_next(BlockJob *job) { if (!job) { @@ -115,19 +128,44 @@ static void block_job_detach_aio_context(void *opaque) block_job_unref(job); } -void block_job_add_bdrv(BlockJob *job, BlockDriverState *bs) +void block_job_remove_all_bdrv(BlockJob *job) +{ + GSList *l; + for (l = job->nodes; l; l = l->next) { + BdrvChild *c = l->data; + bdrv_op_unblock_all(c->bs, job->blocker); + bdrv_root_unref_child(c); + } + g_slist_free(job->nodes); + job->nodes = NULL; +} + +int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, + uint64_t perm, uint64_t shared_perm, Error **errp) { - job->nodes = g_slist_prepend(job->nodes, bs); + BdrvChild *c; + + c = bdrv_root_attach_child(bs, name, &child_job, perm, shared_perm, + job, errp); + if (c == NULL) { + return -EPERM; + } + + job->nodes = g_slist_prepend(job->nodes, c); bdrv_ref(bs); bdrv_op_block_all(bs, job->blocker); + + return 0; } void *block_job_create(const char *job_id, const BlockJobDriver *driver, - BlockDriverState *bs, int64_t speed, int flags, + BlockDriverState *bs, uint64_t perm, + uint64_t shared_perm, int64_t speed, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp) { BlockBackend *blk; BlockJob *job; + int ret; if (bs->job) { error_setg(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(bs)); @@ -159,13 +197,17 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, } } - blk = blk_new(); - blk_insert_bs(blk, bs); + blk = blk_new(perm, shared_perm); + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + blk_unref(blk); + return NULL; + } job = g_malloc0(driver->instance_size); error_setg(&job->blocker, "block device is in use by block job: %s", BlockJobType_lookup[driver->job_type]); - block_job_add_bdrv(job, bs); + block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); job->driver = driver; @@ -228,15 +270,9 @@ void block_job_ref(BlockJob *job) void block_job_unref(BlockJob *job) { if (--job->refcnt == 0) { - GSList *l; BlockDriverState *bs = blk_bs(job->blk); bs->job = NULL; - for (l = job->nodes; l; l = l->next) { - bs = l->data; - bdrv_op_unblock_all(bs, job->blocker); - bdrv_unref(bs); - } - g_slist_free(job->nodes); + block_job_remove_all_bdrv(job); blk_remove_aio_context_notifier(job->blk, block_job_attached_aio_context, block_job_detach_aio_context, job); @@ -2045,13 +2045,17 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) const char* device = qdict_get_str(qdict, "device"); const char* command = qdict_get_str(qdict, "command"); Error *err = NULL; + int ret; blk = blk_by_name(device); if (!blk) { BlockDriverState *bs = bdrv_lookup_bs(NULL, device, &err); if (bs) { - blk = local_blk = blk_new(); - blk_insert_bs(blk, bs); + blk = local_blk = blk_new(0, BLK_PERM_ALL); + ret = blk_insert_bs(blk, bs, &err); + if (ret < 0) { + goto fail; + } } else { goto fail; } @@ -2060,6 +2064,31 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict) aio_context = blk_get_aio_context(blk); aio_context_acquire(aio_context); + /* + * Notably absent: Proper permission management. This is sad, but it seems + * almost impossible to achieve without changing the semantics and thereby + * limiting the use cases of the qemu-io HMP command. + * + * In an ideal world we would unconditionally create a new BlockBackend for + * qemuio_command(), but we have commands like 'reopen' and want them to + * take effect on the exact BlockBackend whose name the user passed instead + * of just on a temporary copy of it. + * + * Another problem is that deleting the temporary BlockBackend involves + * draining all requests on it first, but some qemu-iotests cases want to + * issue multiple aio_read/write requests and expect them to complete in + * the background while the monitor has already returned. + * + * This is also what prevents us from saving the original permissions and + * restoring them later: We can't revoke permissions until all requests + * have completed, and we don't know when that is nor can we really let + * anything else run before we have revoken them to avoid race conditions. + * + * What happens now is that command() in qemu-io-cmds.c can extend the + * permissions if necessary for the qemu-io command. And they simply stay + * extended, possibly resulting in a read-only guest device keeping write + * permissions. Ugly, but it appears to be the lesser evil. + */ qemuio_command(blk, command); aio_context_release(aio_context); diff --git a/hw/block/block.c b/hw/block/block.c index 8dc9d84a39..27878d0087 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -51,11 +51,33 @@ void blkconf_blocksizes(BlockConf *conf) } } -void blkconf_apply_backend_options(BlockConf *conf) +void blkconf_apply_backend_options(BlockConf *conf, bool readonly, + bool resizable, Error **errp) { BlockBackend *blk = conf->blk; BlockdevOnError rerror, werror; + uint64_t perm, shared_perm; bool wce; + int ret; + + perm = BLK_PERM_CONSISTENT_READ; + if (!readonly) { + perm |= BLK_PERM_WRITE; + } + + shared_perm = BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | + BLK_PERM_GRAPH_MOD; + if (resizable) { + shared_perm |= BLK_PERM_RESIZE; + } + if (conf->share_rw) { + shared_perm |= BLK_PERM_WRITE; + } + + ret = blk_set_perm(blk, perm, shared_perm, errp); + if (ret < 0) { + return; + } switch (conf->wce) { case ON_OFF_AUTO_ON: wce = true; break; diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 17d29e7bc5..a328693d15 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -186,6 +186,7 @@ typedef enum FDiskFlags { struct FDrive { FDCtrl *fdctrl; BlockBackend *blk; + BlockConf *conf; /* Drive status */ FloppyDriveType drive; /* CMOS drive type */ uint8_t perpendicular; /* 2.88 MB access mode */ @@ -469,9 +470,22 @@ static void fd_revalidate(FDrive *drv) } } -static void fd_change_cb(void *opaque, bool load) +static void fd_change_cb(void *opaque, bool load, Error **errp) { FDrive *drive = opaque; + Error *local_err = NULL; + + if (!load) { + blk_set_perm(drive->blk, 0, BLK_PERM_ALL, &error_abort); + } else { + blkconf_apply_backend_options(drive->conf, + blk_is_read_only(drive->blk), false, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } drive->media_changed = 1; drive->media_validated = false; @@ -508,6 +522,7 @@ static int floppy_drive_init(DeviceState *qdev) FloppyDrive *dev = FLOPPY_DRIVE(qdev); FloppyBus *bus = FLOPPY_BUS(qdev->parent_bus); FDrive *drive; + Error *local_err = NULL; int ret; if (dev->unit == -1) { @@ -533,7 +548,7 @@ static int floppy_drive_init(DeviceState *qdev) if (!dev->conf.blk) { /* Anonymous BlockBackend for an empty drive */ - dev->conf.blk = blk_new(); + dev->conf.blk = blk_new(0, BLK_PERM_ALL); ret = blk_attach_dev(dev->conf.blk, qdev); assert(ret == 0); } @@ -551,7 +566,13 @@ static int floppy_drive_init(DeviceState *qdev) * blkconf_apply_backend_options(). */ dev->conf.rerror = BLOCKDEV_ON_ERROR_AUTO; dev->conf.werror = BLOCKDEV_ON_ERROR_AUTO; - blkconf_apply_backend_options(&dev->conf); + + blkconf_apply_backend_options(&dev->conf, blk_is_read_only(dev->conf.blk), + false, &local_err); + if (local_err) { + error_report_err(local_err); + return -1; + } /* 'enospc' is the default for -drive, 'report' is what blk_new() gives us * for empty drives. */ @@ -565,6 +586,7 @@ static int floppy_drive_init(DeviceState *qdev) return -1; } + drive->conf = &dev->conf; drive->blk = dev->conf.blk; drive->fdctrl = bus->fdc; diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 2d6eb46a04..190573cefa 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -1215,6 +1215,7 @@ static void m25p80_realize(SSISlave *ss, Error **errp) { Flash *s = M25P80(ss); M25P80Class *mc = M25P80_GET_CLASS(s); + int ret; s->pi = mc->pi; @@ -1222,6 +1223,13 @@ static void m25p80_realize(SSISlave *ss, Error **errp) s->dirty_page = -1; if (s->blk) { + uint64_t perm = BLK_PERM_CONSISTENT_READ | + (blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE); + ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp); + if (ret < 0) { + return; + } + DB_PRINT_L(0, "Binding to IF_MTD drive\n"); s->storage = blk_blockalign(s->blk, s->size); diff --git a/hw/block/nand.c b/hw/block/nand.c index c69e6755d9..0d33ac281f 100644 --- a/hw/block/nand.c +++ b/hw/block/nand.c @@ -373,6 +373,8 @@ static void nand_realize(DeviceState *dev, Error **errp) { int pagesize; NANDFlashState *s = NAND(dev); + int ret; + s->buswidth = nand_flash_ids[s->chip_id].width >> 3; s->size = nand_flash_ids[s->chip_id].size << 20; @@ -407,6 +409,11 @@ static void nand_realize(DeviceState *dev, Error **errp) error_setg(errp, "Can't use a read-only drive"); return; } + ret = blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, + BLK_PERM_ALL, errp); + if (ret < 0) { + return; + } if (blk_getlength(s->blk) >= (s->pages << s->page_shift) + (s->pages << s->oob_shift)) { pagesize = 0; diff --git a/hw/block/nvme.c b/hw/block/nvme.c index ae91a18f17..ae303d44e5 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -835,6 +835,7 @@ static int nvme_init(PCIDevice *pci_dev) int i; int64_t bs_size; uint8_t *pci_conf; + Error *local_err = NULL; if (!n->conf.blk) { return -1; @@ -850,7 +851,12 @@ static int nvme_init(PCIDevice *pci_dev) return -1; } blkconf_blocksizes(&n->conf); - blkconf_apply_backend_options(&n->conf); + blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk), + false, &local_err); + if (local_err) { + error_report_err(local_err); + return -1; + } pci_conf = pci_dev->config; pci_conf[PCI_INTERRUPT_PIN] = 1; diff --git a/hw/block/onenand.c b/hw/block/onenand.c index 8d8422739e..ddf5492426 100644 --- a/hw/block/onenand.c +++ b/hw/block/onenand.c @@ -778,6 +778,7 @@ static int onenand_initfn(SysBusDevice *sbd) OneNANDState *s = ONE_NAND(dev); uint32_t size = 1 << (24 + ((s->id.dev >> 4) & 7)); void *ram; + Error *local_err = NULL; s->base = (hwaddr)-1; s->rdy = NULL; @@ -796,6 +797,12 @@ static int onenand_initfn(SysBusDevice *sbd) error_report("Can't use a read-only drive"); return -1; } + blk_set_perm(s->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, + BLK_PERM_ALL, &local_err); + if (local_err) { + error_report_err(local_err); + return -1; + } s->blk_cur = s->blk; } s->otp = memset(g_malloc((64 + 2) << PAGE_SHIFT), diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 71b98a3eef..594d4cf6fe 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -758,6 +758,18 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem); if (pfl->blk) { + uint64_t perm; + pfl->ro = blk_is_read_only(pfl->blk); + perm = BLK_PERM_CONSISTENT_READ | (pfl->ro ? 0 : BLK_PERM_WRITE); + ret = blk_set_perm(pfl->blk, perm, BLK_PERM_ALL, errp); + if (ret < 0) { + return; + } + } else { + pfl->ro = 0; + } + + if (pfl->blk) { /* read the initial flash content */ ret = blk_pread(pfl->blk, 0, pfl->storage, total_len); @@ -768,12 +780,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error **errp) } } - if (pfl->blk) { - pfl->ro = blk_is_read_only(pfl->blk); - } else { - pfl->ro = 0; - } - /* Default to devices being used at their maximum device width. This was * assumed before the device_width support was added. */ diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index ef71322759..e6c5c6c25d 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -632,6 +632,19 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp) vmstate_register_ram(&pfl->orig_mem, DEVICE(pfl)); pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem); pfl->chip_len = chip_len; + + if (pfl->blk) { + uint64_t perm; + pfl->ro = blk_is_read_only(pfl->blk); + perm = BLK_PERM_CONSISTENT_READ | (pfl->ro ? 0 : BLK_PERM_WRITE); + ret = blk_set_perm(pfl->blk, perm, BLK_PERM_ALL, errp); + if (ret < 0) { + return; + } + } else { + pfl->ro = 0; + } + if (pfl->blk) { /* read the initial flash content */ ret = blk_pread(pfl->blk, 0, pfl->storage, chip_len); @@ -646,12 +659,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error **errp) pfl->rom_mode = 1; sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem); - if (pfl->blk) { - pfl->ro = blk_is_read_only(pfl->blk); - } else { - pfl->ro = 0; - } - pfl->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pflash_timer, pfl); pfl->wcycle = 0; pfl->cmd = 0; diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 843bd2fa73..98c16a7a9a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -928,7 +928,13 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp) } blkconf_serial(&conf->conf, &conf->serial); - blkconf_apply_backend_options(&conf->conf); + blkconf_apply_backend_options(&conf->conf, + blk_is_read_only(conf->conf.blk), true, + &err); + if (err) { + error_propagate(errp, err); + return; + } s->original_wce = blk_enable_write_cache(conf->conf.blk); blkconf_geometry(&conf->conf, NULL, 65535, 255, 255, &err); if (err) { diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 94f4d8bde4..c34be1c1ba 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -73,14 +73,19 @@ static void parse_drive(DeviceState *dev, const char *str, void **ptr, { BlockBackend *blk; bool blk_created = false; + int ret; blk = blk_by_name(str); if (!blk) { BlockDriverState *bs = bdrv_lookup_bs(NULL, str, NULL); if (bs) { - blk = blk_new(); - blk_insert_bs(blk, bs); + blk = blk_new(0, BLK_PERM_ALL); blk_created = true; + + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto fail; + } } } if (!blk) { diff --git a/hw/ide/core.c b/hw/ide/core.c index cfa5de6ebf..db509b3e15 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -1120,7 +1120,7 @@ static void ide_cfata_metadata_write(IDEState *s) } /* called when the inserted state of the media has changed */ -static void ide_cd_change_cb(void *opaque, bool load) +static void ide_cd_change_cb(void *opaque, bool load, Error **errp) { IDEState *s = opaque; uint64_t nb_sectors; diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c index dbaa75cf59..4383cd111d 100644 --- a/hw/ide/qdev.c +++ b/hw/ide/qdev.c @@ -170,7 +170,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) return -1; } else { /* Anonymous BlockBackend for an empty drive */ - dev->conf.blk = blk_new(); + dev->conf.blk = blk_new(0, BLK_PERM_ALL); } } @@ -196,7 +196,12 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind) return -1; } } - blkconf_apply_backend_options(&dev->conf); + blkconf_apply_backend_options(&dev->conf, kind == IDE_CD, kind != IDE_CD, + &err); + if (err) { + error_report_err(err); + return -1; + } if (ide_init_drive(s, dev->conf.blk, kind, dev->version, dev->serial, dev->model, dev->wwn, diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c index 65ba188555..aa5d2c1f5f 100644 --- a/hw/nvram/spapr_nvram.c +++ b/hw/nvram/spapr_nvram.c @@ -141,9 +141,17 @@ static void rtas_nvram_store(PowerPCCPU *cpu, sPAPRMachineState *spapr, static void spapr_nvram_realize(VIOsPAPRDevice *dev, Error **errp) { sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(dev); + int ret; if (nvram->blk) { nvram->size = blk_getlength(nvram->blk); + + ret = blk_set_perm(nvram->blk, + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, + BLK_PERM_ALL, errp); + if (ret < 0) { + return; + } } else { nvram->size = DEFAULT_NVRAM_SIZE; } diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index bbfb5dc289..a53f058621 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -2240,7 +2240,7 @@ static void scsi_disk_resize_cb(void *opaque) } } -static void scsi_cd_change_media_cb(void *opaque, bool load) +static void scsi_cd_change_media_cb(void *opaque, bool load, Error **errp) { SCSIDiskState *s = opaque; @@ -2328,7 +2328,13 @@ static void scsi_realize(SCSIDevice *dev, Error **errp) return; } } - blkconf_apply_backend_options(&dev->conf); + blkconf_apply_backend_options(&dev->conf, + blk_is_read_only(s->qdev.conf.blk), + dev->type == TYPE_DISK, &err); + if (err) { + error_propagate(errp, err); + return; + } if (s->qdev.conf.discard_granularity == -1) { s->qdev.conf.discard_granularity = @@ -2380,7 +2386,7 @@ static void scsi_cd_realize(SCSIDevice *dev, Error **errp) SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev); if (!dev->conf.blk) { - dev->conf.blk = blk_new(); + dev->conf.blk = blk_new(0, BLK_PERM_ALL); } s->qdev.blocksize = 2048; diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 8e88e8311a..ba47bff4db 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -458,7 +458,7 @@ static bool sd_get_readonly(SDState *sd) return sd->wp_switch; } -static void sd_cardchange(void *opaque, bool load) +static void sd_cardchange(void *opaque, bool load, Error **errp) { SDState *sd = opaque; DeviceState *dev = DEVICE(sd); @@ -1887,6 +1887,7 @@ static void sd_instance_finalize(Object *obj) static void sd_realize(DeviceState *dev, Error **errp) { SDState *sd = SD_CARD(dev); + int ret; if (sd->blk && blk_is_read_only(sd->blk)) { error_setg(errp, "Cannot use read-only drive as SD card"); @@ -1894,6 +1895,11 @@ static void sd_realize(DeviceState *dev, Error **errp) } if (sd->blk) { + ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE, + BLK_PERM_ALL, errp); + if (ret < 0) { + return; + } blk_set_dev_ops(sd->blk, &sd_block_ops, sd); } } diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index c607f7606d..a71b354fa6 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -603,7 +603,11 @@ static void usb_msd_realize_storage(USBDevice *dev, Error **errp) blkconf_serial(&s->conf, &dev->serial); blkconf_blocksizes(&s->conf); - blkconf_apply_backend_options(&s->conf); + blkconf_apply_backend_options(&s->conf, blk_is_read_only(blk), true, &err); + if (err) { + error_propagate(errp, err); + return; + } /* * Hack alert: this pretends to be a block device, but it's really diff --git a/include/block/block.h b/include/block/block.h index bde5ebda18..c7c4a3ac3a 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -82,6 +82,7 @@ typedef struct HDGeometry { } HDGeometry; #define BDRV_O_RDWR 0x0002 +#define BDRV_O_RESIZE 0x0004 /* request permission for resizing the node */ #define BDRV_O_SNAPSHOT 0x0008 /* open the file read only and save writes in a snapshot */ #define BDRV_O_TEMPORARY 0x0010 /* delete the file after use */ #define BDRV_O_NOCACHE 0x0020 /* do not use the host page cache */ @@ -187,6 +188,42 @@ typedef enum BlockOpType { BLOCK_OP_TYPE_MAX, } BlockOpType; +/* Block node permission constants */ +enum { + /** + * A user that has the "permission" of consistent reads is guaranteed that + * their view of the contents of the block device is complete and + * self-consistent, representing the contents of a disk at a specific + * point. + * + * For most block devices (including their backing files) this is true, but + * the property cannot be maintained in a few situations like for + * intermediate nodes of a commit block job. + */ + BLK_PERM_CONSISTENT_READ = 0x01, + + /** This permission is required to change the visible disk contents. */ + BLK_PERM_WRITE = 0x02, + + /** + * This permission (which is weaker than BLK_PERM_WRITE) is both enough and + * required for writes to the block node when the caller promises that + * the visible disk content doesn't change. + */ + BLK_PERM_WRITE_UNCHANGED = 0x04, + + /** This permission is required to change the size of a block node. */ + BLK_PERM_RESIZE = 0x08, + + /** + * This permission is required to change the node that this BdrvChild + * points to. + */ + BLK_PERM_GRAPH_MOD = 0x10, + + BLK_PERM_ALL = 0x1f, +}; + /* disk I/O throttling */ void bdrv_init(void); void bdrv_init_with_whitelist(void); @@ -199,7 +236,8 @@ int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts, Error **errp); int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp); BlockDriverState *bdrv_new(void); -void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top); +void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, + Error **errp); void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new); @@ -210,7 +248,8 @@ BdrvChild *bdrv_open_child(const char *filename, BlockDriverState* parent, const BdrvChildRole *child_role, bool allow_none, Error **errp); -void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd); +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd, + Error **errp); int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options, const char *bdref_key, Error **errp); BlockDriverState *bdrv_open(const char *filename, const char *reference, @@ -484,7 +523,8 @@ void bdrv_unref_child(BlockDriverState *parent, BdrvChild *child); BdrvChild *bdrv_attach_child(BlockDriverState *parent_bs, BlockDriverState *child_bs, const char *child_name, - const BdrvChildRole *child_role); + const BdrvChildRole *child_role, + Error **errp); bool bdrv_op_is_blocked(BlockDriverState *bs, BlockOpType op, Error **errp); void bdrv_op_block(BlockDriverState *bs, BlockOpType op, Error *reason); diff --git a/include/block/block_int.h b/include/block/block_int.h index 1670941da9..a57c0bfb55 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -320,6 +320,59 @@ struct BlockDriver { void (*bdrv_del_child)(BlockDriverState *parent, BdrvChild *child, Error **errp); + /** + * Informs the block driver that a permission change is intended. The + * driver checks whether the change is permissible and may take other + * preparations for the change (e.g. get file system locks). This operation + * is always followed either by a call to either .bdrv_set_perm or + * .bdrv_abort_perm_update. + * + * Checks whether the requested set of cumulative permissions in @perm + * can be granted for accessing @bs and whether no other users are using + * permissions other than those given in @shared (both arguments take + * BLK_PERM_* bitmasks). + * + * If both conditions are met, 0 is returned. Otherwise, -errno is returned + * and errp is set to an error describing the conflict. + */ + int (*bdrv_check_perm)(BlockDriverState *bs, uint64_t perm, + uint64_t shared, Error **errp); + + /** + * Called to inform the driver that the set of cumulative set of used + * permissions for @bs has changed to @perm, and the set of sharable + * permission to @shared. The driver can use this to propagate changes to + * its children (i.e. request permissions only if a parent actually needs + * them). + * + * This function is only invoked after bdrv_check_perm(), so block drivers + * may rely on preparations made in their .bdrv_check_perm implementation. + */ + void (*bdrv_set_perm)(BlockDriverState *bs, uint64_t perm, uint64_t shared); + + /* + * Called to inform the driver that after a previous bdrv_check_perm() + * call, the permission update is not performed and any preparations made + * for it (e.g. taken file locks) need to be undone. + * + * This function can be called even for nodes that never saw a + * bdrv_check_perm() call. It is a no-op then. + */ + void (*bdrv_abort_perm_update)(BlockDriverState *bs); + + /** + * Returns in @nperm and @nshared the permissions that the driver for @bs + * needs on its child @c, based on the cumulative permissions requested by + * the parents in @parent_perm and @parent_shared. + * + * If @c is NULL, return the permissions for attaching a new child for the + * given @role. + */ + void (*bdrv_child_perm)(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t parent_perm, uint64_t parent_shared, + uint64_t *nperm, uint64_t *nshared); + QLIST_ENTRY(BlockDriver) list; }; @@ -388,6 +441,10 @@ typedef struct BdrvAioNotifier { } BdrvAioNotifier; struct BdrvChildRole { + /* If true, bdrv_replace_in_backing_chain() doesn't change the node this + * BdrvChild points to. */ + bool stay_at_node; + void (*inherit_options)(int *child_flags, QDict *child_options, int parent_flags, QDict *parent_options); @@ -399,6 +456,12 @@ struct BdrvChildRole { * name), or NULL if the parent can't provide a better name. */ const char* (*get_name)(BdrvChild *child); + /* Returns a malloced string that describes the parent of the child for a + * human reader. This could be a node-name, BlockBackend name, qdev ID or + * QOM path of the device owning the BlockBackend, job type and ID etc. The + * caller is responsible for freeing the memory. */ + char* (*get_parent_desc)(BdrvChild *child); + /* * If this pair of functions is implemented, the parent doesn't issue new * requests after returning from .drained_begin() until .drained_end() is @@ -409,16 +472,32 @@ struct BdrvChildRole { */ void (*drained_begin)(BdrvChild *child); void (*drained_end)(BdrvChild *child); + + void (*attach)(BdrvChild *child); + void (*detach)(BdrvChild *child); }; extern const BdrvChildRole child_file; extern const BdrvChildRole child_format; +extern const BdrvChildRole child_backing; struct BdrvChild { BlockDriverState *bs; char *name; const BdrvChildRole *role; void *opaque; + + /** + * Granted permissions for operating on this BdrvChild (BLK_PERM_* bitmask) + */ + uint64_t perm; + + /** + * Permissions that can still be granted to other users of @bs while this + * BdrvChild is still attached to it. (BLK_PERM_* bitmask) + */ + uint64_t shared_perm; + QLIST_ENTRY(BdrvChild) next; QLIST_ENTRY(BdrvChild) next_parent; }; @@ -701,13 +780,16 @@ void stream_start(const char *job_id, BlockDriverState *bs, * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @on_error: The action to take upon error. * @backing_file_str: String to use as the backing file in @top's overlay + * @filter_node_name: The node name that should be assigned to the filter + * driver that the commit job inserts into the graph above @top. NULL means + * that a node name should be autogenerated. * @errp: Error object. * */ void commit_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, BlockDriverState *top, int64_t speed, BlockdevOnError on_error, const char *backing_file_str, - Error **errp); + const char *filter_node_name, Error **errp); /** * commit_active_start: * @job_id: The id of the newly-created job, or %NULL to use the @@ -718,6 +800,9 @@ void commit_start(const char *job_id, BlockDriverState *bs, * See @BlockJobCreateFlags * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @on_error: The action to take upon error. + * @filter_node_name: The node name that should be assigned to the filter + * driver that the commit job inserts into the graph above @bs. NULL means that + * a node name should be autogenerated. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. * @errp: Error object. @@ -727,8 +812,9 @@ void commit_start(const char *job_id, BlockDriverState *bs, void commit_active_start(const char *job_id, BlockDriverState *bs, BlockDriverState *base, int creation_flags, int64_t speed, BlockdevOnError on_error, - BlockCompletionFunc *cb, - void *opaque, Error **errp, bool auto_complete); + const char *filter_node_name, + BlockCompletionFunc *cb, void *opaque, Error **errp, + bool auto_complete); /* * mirror_start: * @job_id: The id of the newly-created job, or %NULL to use the @@ -745,6 +831,9 @@ void commit_active_start(const char *job_id, BlockDriverState *bs, * @on_source_error: The action to take upon error reading from the source. * @on_target_error: The action to take upon error writing to the target. * @unmap: Whether to unmap target where source sectors only contain zeroes. + * @filter_node_name: The node name that should be assigned to the filter + * driver that the mirror job inserts into the graph above @bs. NULL means that + * a node name should be autogenerated. * @errp: Error object. * * Start a mirroring operation on @bs. Clusters that are allocated @@ -758,7 +847,7 @@ void mirror_start(const char *job_id, BlockDriverState *bs, MirrorSyncMode mode, BlockMirrorBackingMode backing_mode, BlockdevOnError on_source_error, BlockdevOnError on_target_error, - bool unmap, Error **errp); + bool unmap, const char *filter_node_name, Error **errp); /* * backup_job_create: @@ -796,11 +885,36 @@ void hmp_drive_add_node(Monitor *mon, const char *optstr); BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, const char *child_name, const BdrvChildRole *child_role, - void *opaque); + uint64_t perm, uint64_t shared_perm, + void *opaque, Error **errp); void bdrv_root_unref_child(BdrvChild *child); +int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared, + Error **errp); +void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared); +void bdrv_child_abort_perm_update(BdrvChild *c); +int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, + Error **errp); + +/* Default implementation for BlockDriver.bdrv_child_perm() that can be used by + * block filters: Forward CONSISTENT_READ, WRITE, WRITE_UNCHANGED and RESIZE to + * all children */ +void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared); + +/* Default implementation for BlockDriver.bdrv_child_perm() that can be used by + * (non-raw) image formats: Like above for bs->backing, but for bs->file it + * requires WRITE | RESIZE for read-write images, always requires + * CONSISTENT_READ and doesn't share WRITE. */ +void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, + const BdrvChildRole *role, + uint64_t perm, uint64_t shared, + uint64_t *nperm, uint64_t *nshared); + const char *bdrv_get_parent_name(const BlockDriverState *bs); -void blk_dev_change_media_cb(BlockBackend *blk, bool load); +void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); bool blk_dev_has_removable_media(BlockBackend *blk); bool blk_dev_has_tray(BlockBackend *blk); void blk_dev_eject_request(BlockBackend *blk, bool force); diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 1acb256223..9e906f7d7e 100644 --- a/include/block/blockjob.h +++ b/include/block/blockjob.h @@ -169,13 +169,25 @@ BlockJob *block_job_get(const char *id); /** * block_job_add_bdrv: * @job: A block job + * @name: The name to assign to the new BdrvChild * @bs: A BlockDriverState that is involved in @job + * @perm, @shared_perm: Permissions to request on the node * * Add @bs to the list of BlockDriverState that are involved in * @job. This means that all operations will be blocked on @bs while * @job exists. */ -void block_job_add_bdrv(BlockJob *job, BlockDriverState *bs); +int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, + uint64_t perm, uint64_t shared_perm, Error **errp); + +/** + * block_job_remove_all_bdrv: + * @job: The block job + * + * Remove all BlockDriverStates from the list of nodes that are involved in the + * job. This removes the blockers added with block_job_add_bdrv(). + */ +void block_job_remove_all_bdrv(BlockJob *job); /** * block_job_set_speed: diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 82238229c6..3f86cc5acc 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -119,6 +119,7 @@ struct BlockJobDriver { * generated automatically. * @job_type: The class object for the newly-created job. * @bs: The block + * @perm, @shared_perm: Permissions to request for @bs * @speed: The maximum speed, in bytes per second, or 0 for unlimited. * @cb: Completion function for the job. * @opaque: Opaque pointer value passed to @cb. @@ -134,7 +135,8 @@ struct BlockJobDriver { * called from a wrapper that is specific to the job type. */ void *block_job_create(const char *job_id, const BlockJobDriver *driver, - BlockDriverState *bs, int64_t speed, int flags, + BlockDriverState *bs, uint64_t perm, + uint64_t shared_perm, int64_t speed, int flags, BlockCompletionFunc *cb, void *opaque, Error **errp); /** diff --git a/include/hw/block/block.h b/include/hw/block/block.h index df9d207d81..f3f6e8ef02 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -26,6 +26,7 @@ typedef struct BlockConf { /* geometry, not all devices use this */ uint32_t cyls, heads, secs; OnOffAuto wce; + bool share_rw; BlockdevOnError rerror; BlockdevOnError werror; } BlockConf; @@ -53,7 +54,9 @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) DEFINE_PROP_UINT32("opt_io_size", _state, _conf.opt_io_size, 0), \ DEFINE_PROP_UINT32("discard_granularity", _state, \ _conf.discard_granularity, -1), \ - DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, ON_OFF_AUTO_AUTO) + DEFINE_PROP_ON_OFF_AUTO("write-cache", _state, _conf.wce, \ + ON_OFF_AUTO_AUTO), \ + DEFINE_PROP_BOOL("share-rw", _state, _conf.share_rw, false) #define DEFINE_BLOCK_CHS_PROPERTIES(_state, _conf) \ DEFINE_PROP_UINT32("cyls", _state, _conf.cyls, 0), \ @@ -73,7 +76,8 @@ void blkconf_geometry(BlockConf *conf, int *trans, unsigned cyls_max, unsigned heads_max, unsigned secs_max, Error **errp); void blkconf_blocksizes(BlockConf *conf); -void blkconf_apply_backend_options(BlockConf *conf); +void blkconf_apply_backend_options(BlockConf *conf, bool readonly, + bool resizable, Error **errp); /* Hard disk geometry */ diff --git a/include/qemu-io.h b/include/qemu-io.h index 4d402b9b01..196fde0f3a 100644 --- a/include/qemu-io.h +++ b/include/qemu-io.h @@ -36,6 +36,7 @@ typedef struct cmdinfo { const char *args; const char *oneline; helpfunc_t help; + uint64_t perm; } cmdinfo_t; extern bool qemuio_misalign; diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index f365a51acf..096c17fce0 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -34,7 +34,7 @@ typedef struct BlockDevOps { * changes. Sure would be useful if it did. * Device models with removable media must implement this callback. */ - void (*change_media_cb)(void *opaque, bool load); + void (*change_media_cb)(void *opaque, bool load, Error **errp); /* * Runs when an eject request is issued from the monitor, the tray * is closed, and the medium is locked. @@ -84,7 +84,7 @@ typedef struct BlockBackendPublic { QLIST_ENTRY(BlockBackendPublic) round_robin; } BlockBackendPublic; -BlockBackend *blk_new(void); +BlockBackend *blk_new(uint64_t perm, uint64_t shared_perm); BlockBackend *blk_new_open(const char *filename, const char *reference, QDict *options, int flags, Error **errp); int blk_get_refcnt(BlockBackend *blk); @@ -102,9 +102,12 @@ BlockBackend *blk_by_public(BlockBackendPublic *public); BlockDriverState *blk_bs(BlockBackend *blk); void blk_remove_bs(BlockBackend *blk); -void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs); +int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp); bool bdrv_has_blk(BlockDriverState *bs); bool bdrv_is_root_node(BlockDriverState *bs); +int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm, + Error **errp); +void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm); void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow); void blk_iostatus_enable(BlockBackend *blk); diff --git a/migration/block.c b/migration/block.c index ebc10e628d..1941bc2402 100644 --- a/migration/block.c +++ b/migration/block.c @@ -379,7 +379,7 @@ static void unset_dirty_tracking(void) } } -static void init_blk_migration(QEMUFile *f) +static int init_blk_migration(QEMUFile *f) { BlockDriverState *bs; BlkMigDevState *bmds; @@ -390,6 +390,8 @@ static void init_blk_migration(QEMUFile *f) BlkMigDevState *bmds; BlockDriverState *bs; } *bmds_bs; + Error *local_err = NULL; + int ret; block_mig_state.submitted = 0; block_mig_state.read_done = 0; @@ -411,11 +413,12 @@ static void init_blk_migration(QEMUFile *f) sectors = bdrv_nb_sectors(bs); if (sectors <= 0) { + ret = sectors; goto out; } bmds = g_new0(BlkMigDevState, 1); - bmds->blk = blk_new(); + bmds->blk = blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL); bmds->blk_name = g_strdup(bdrv_get_device_name(bs)); bmds->bulk_completed = 0; bmds->total_sectors = sectors; @@ -445,7 +448,11 @@ static void init_blk_migration(QEMUFile *f) BlockDriverState *bs = bmds_bs[i].bs; if (bmds) { - blk_insert_bs(bmds->blk, bs); + ret = blk_insert_bs(bmds->blk, bs, &local_err); + if (ret < 0) { + error_report_err(local_err); + goto out; + } alloc_aio_bitmap(bmds); error_setg(&bmds->blocker, "block device is in use by migration"); @@ -453,8 +460,10 @@ static void init_blk_migration(QEMUFile *f) } } + ret = 0; out: g_free(bmds_bs); + return ret; } /* Called with no lock taken. */ @@ -705,7 +714,11 @@ static int block_save_setup(QEMUFile *f, void *opaque) block_mig_state.submitted, block_mig_state.transferred); qemu_mutex_lock_iothread(); - init_blk_migration(f); + ret = init_blk_migration(f); + if (ret < 0) { + qemu_mutex_unlock_iothread(); + return ret; + } /* start track dirty blocks */ ret = set_dirty_tracking(); diff --git a/nbd/server.c b/nbd/server.c index ac92fa0727..924a1fe2db 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -891,9 +891,21 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size, { BlockBackend *blk; NBDExport *exp = g_malloc0(sizeof(NBDExport)); + uint64_t perm; + int ret; - blk = blk_new(); - blk_insert_bs(blk, bs); + /* Don't allow resize while the NBD server is running, otherwise we don't + * care what happens with the node. */ + perm = BLK_PERM_CONSISTENT_READ; + if ((nbdflags & NBD_FLAG_READ_ONLY) == 0) { + perm |= BLK_PERM_WRITE; + } + blk = blk_new(perm, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED | + BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD); + ret = blk_insert_bs(blk, bs, errp); + if (ret < 0) { + goto fail; + } blk_set_enable_write_cache(blk, !writethrough); exp->refcount = 1; diff --git a/qapi/block-core.json b/qapi/block-core.json index cf24c04242..5cc992fb8f 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1304,6 +1304,11 @@ # # @speed: #optional the maximum speed, in bytes per second # +# @filter-node-name: #optional the node name that should be assigned to the +# filter driver that the commit job inserts into the graph +# above @top. If this option is not given, a node name is +# autogenerated. (Since: 2.9) +# # Returns: Nothing on success # If commit or stream is already active on this device, DeviceInUse # If @device does not exist, DeviceNotFound @@ -1323,7 +1328,8 @@ ## { 'command': 'block-commit', 'data': { '*job-id': 'str', 'device': 'str', '*base': 'str', '*top': 'str', - '*backing-file': 'str', '*speed': 'int' } } + '*backing-file': 'str', '*speed': 'int', + '*filter-node-name': 'str' } } ## # @drive-backup: @@ -1671,6 +1677,11 @@ # default 'report' (no limitations, since this applies to # a different block device than @device). # +# @filter-node-name: #optional the node name that should be assigned to the +# filter driver that the mirror job inserts into the graph +# above @device. If this option is not given, a node name is +# autogenerated. (Since: 2.9) +# # Returns: nothing on success. # # Since: 2.6 @@ -1690,7 +1701,8 @@ 'sync': 'MirrorSyncMode', '*speed': 'int', '*granularity': 'uint32', '*buf-size': 'int', '*on-source-error': 'BlockdevOnError', - '*on-target-error': 'BlockdevOnError' } } + '*on-target-error': 'BlockdevOnError', + '*filter-node-name': 'str' } } ## # @block_set_io_throttle: diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx index f054599a91..9c9702cc62 100644 --- a/qemu-img-cmds.hx +++ b/qemu-img-cmds.hx @@ -40,9 +40,9 @@ STEXI ETEXI DEF("convert", img_convert, - "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename") + "convert [--object objectdef] [--image-opts] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename") STEXI -@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} +@item convert [--object @var{objectdef}] [--image-opts] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename} ETEXI DEF("dd", img_dd, diff --git a/qemu-img.c b/qemu-img.c index df3aefd35a..98b836b030 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -156,6 +156,11 @@ static void QEMU_NORETURN help(void) " kinds of errors, with a higher risk of choosing the wrong fix or\n" " hiding corruption that has already occurred.\n" "\n" + "Parameters to convert subcommand:\n" + " '-m' specifies how many coroutines work in parallel during the convert\n" + " process (defaults to 8)\n" + " '-W' allow to write to the target out of order rather than sequential\n" + "\n" "Parameters to snapshot subcommand:\n" " 'snapshot' is the name of the snapshot to create, apply or delete\n" " '-a' applies a snapshot (revert disk to saved state)\n" @@ -809,6 +814,8 @@ static void run_block_job(BlockJob *job, Error **errp) { AioContext *aio_context = blk_get_aio_context(job->blk); + /* FIXME In error cases, the job simply goes away and we access a dangling + * pointer below. */ aio_context_acquire(aio_context); do { aio_poll(aio_context, true); @@ -830,6 +837,7 @@ static int img_commit(int argc, char **argv) const char *filename, *fmt, *cache, *base; BlockBackend *blk; BlockDriverState *bs, *base_bs; + BlockJob *job; bool progress = false, quiet = false, drop = false; bool writethrough; Error *local_err = NULL; @@ -950,8 +958,8 @@ static int img_commit(int argc, char **argv) aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); commit_active_start("commit", bs, base_bs, BLOCK_JOB_DEFAULT, 0, - BLOCKDEV_ON_ERROR_REPORT, common_block_job_cb, &cbi, - &local_err, false); + BLOCKDEV_ON_ERROR_REPORT, NULL, common_block_job_cb, + &cbi, &local_err, false); aio_context_release(aio_context); if (local_err) { goto done; @@ -965,7 +973,8 @@ static int img_commit(int argc, char **argv) bdrv_ref(bs); } - run_block_job(bs->job, &local_err); + job = block_job_get("commit"); + run_block_job(job, &local_err); if (local_err) { goto unref_backing; } @@ -1462,48 +1471,61 @@ enum ImgConvertBlockStatus { BLK_BACKING_FILE, }; +#define MAX_COROUTINES 16 + typedef struct ImgConvertState { BlockBackend **src; int64_t *src_sectors; - int src_cur, src_num; - int64_t src_cur_offset; + int src_num; int64_t total_sectors; int64_t allocated_sectors; + int64_t allocated_done; + int64_t sector_num; + int64_t wr_offs; enum ImgConvertBlockStatus status; int64_t sector_next_status; BlockBackend *target; bool has_zero_init; bool compressed; bool target_has_backing; + bool wr_in_order; int min_sparse; size_t cluster_sectors; size_t buf_sectors; + int num_coroutines; + int running_coroutines; + Coroutine *co[MAX_COROUTINES]; + int64_t wait_sector_num[MAX_COROUTINES]; + CoMutex lock; + int ret; } ImgConvertState; -static void convert_select_part(ImgConvertState *s, int64_t sector_num) +static void convert_select_part(ImgConvertState *s, int64_t sector_num, + int *src_cur, int64_t *src_cur_offset) { - assert(sector_num >= s->src_cur_offset); - while (sector_num - s->src_cur_offset >= s->src_sectors[s->src_cur]) { - s->src_cur_offset += s->src_sectors[s->src_cur]; - s->src_cur++; - assert(s->src_cur < s->src_num); + *src_cur = 0; + *src_cur_offset = 0; + while (sector_num - *src_cur_offset >= s->src_sectors[*src_cur]) { + *src_cur_offset += s->src_sectors[*src_cur]; + (*src_cur)++; + assert(*src_cur < s->src_num); } } static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) { - int64_t ret; - int n; + int64_t ret, src_cur_offset; + int n, src_cur; - convert_select_part(s, sector_num); + convert_select_part(s, sector_num, &src_cur, &src_cur_offset); assert(s->total_sectors > sector_num); n = MIN(s->total_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS); if (s->sector_next_status <= sector_num) { BlockDriverState *file; - ret = bdrv_get_block_status(blk_bs(s->src[s->src_cur]), - sector_num - s->src_cur_offset, + ret = bdrv_get_block_status(blk_bs(s->src[src_cur]), + sector_num - src_cur_offset, n, &n, &file); if (ret < 0) { return ret; @@ -1519,8 +1541,8 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) /* Check block status of the backing file chain to avoid * needlessly reading zeroes and limiting the iteration to the * buffer size */ - ret = bdrv_get_block_status_above(blk_bs(s->src[s->src_cur]), NULL, - sector_num - s->src_cur_offset, + ret = bdrv_get_block_status_above(blk_bs(s->src[src_cur]), NULL, + sector_num - src_cur_offset, n, &n, &file); if (ret < 0) { return ret; @@ -1558,28 +1580,34 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) return n; } -static int convert_read(ImgConvertState *s, int64_t sector_num, int nb_sectors, - uint8_t *buf) +static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num, + int nb_sectors, uint8_t *buf) { - int n; - int ret; + int n, ret; + QEMUIOVector qiov; + struct iovec iov; assert(nb_sectors <= s->buf_sectors); while (nb_sectors > 0) { BlockBackend *blk; - int64_t bs_sectors; + int src_cur; + int64_t bs_sectors, src_cur_offset; /* In the case of compression with multiple source files, we can get a * nb_sectors that spreads into the next part. So we must be able to * read across multiple BDSes for one convert_read() call. */ - convert_select_part(s, sector_num); - blk = s->src[s->src_cur]; - bs_sectors = s->src_sectors[s->src_cur]; - - n = MIN(nb_sectors, bs_sectors - (sector_num - s->src_cur_offset)); - ret = blk_pread(blk, - (sector_num - s->src_cur_offset) << BDRV_SECTOR_BITS, - buf, n << BDRV_SECTOR_BITS); + convert_select_part(s, sector_num, &src_cur, &src_cur_offset); + blk = s->src[src_cur]; + bs_sectors = s->src_sectors[src_cur]; + + n = MIN(nb_sectors, bs_sectors - (sector_num - src_cur_offset)); + iov.iov_base = buf; + iov.iov_len = n << BDRV_SECTOR_BITS; + qemu_iovec_init_external(&qiov, &iov, 1); + + ret = blk_co_preadv( + blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS, + n << BDRV_SECTOR_BITS, &qiov, 0); if (ret < 0) { return ret; } @@ -1592,15 +1620,18 @@ static int convert_read(ImgConvertState *s, int64_t sector_num, int nb_sectors, return 0; } -static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors, - const uint8_t *buf) + +static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, + int nb_sectors, uint8_t *buf, + enum ImgConvertBlockStatus status) { int ret; + QEMUIOVector qiov; + struct iovec iov; while (nb_sectors > 0) { int n = nb_sectors; - - switch (s->status) { + switch (status) { case BLK_BACKING_FILE: /* If we have a backing file, leave clusters unallocated that are * unallocated in the source image, so that the backing file is @@ -1621,9 +1652,13 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors, break; } - ret = blk_pwrite_compressed(s->target, - sector_num << BDRV_SECTOR_BITS, - buf, n << BDRV_SECTOR_BITS); + iov.iov_base = buf; + iov.iov_len = n << BDRV_SECTOR_BITS; + qemu_iovec_init_external(&qiov, &iov, 1); + + ret = blk_co_pwritev(s->target, sector_num << BDRV_SECTOR_BITS, + n << BDRV_SECTOR_BITS, &qiov, + BDRV_REQ_WRITE_COMPRESSED); if (ret < 0) { return ret; } @@ -1636,8 +1671,12 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors, if (!s->min_sparse || is_allocated_sectors_min(buf, n, &n, s->min_sparse)) { - ret = blk_pwrite(s->target, sector_num << BDRV_SECTOR_BITS, - buf, n << BDRV_SECTOR_BITS, 0); + iov.iov_base = buf; + iov.iov_len = n << BDRV_SECTOR_BITS; + qemu_iovec_init_external(&qiov, &iov, 1); + + ret = blk_co_pwritev(s->target, sector_num << BDRV_SECTOR_BITS, + n << BDRV_SECTOR_BITS, &qiov, 0); if (ret < 0) { return ret; } @@ -1649,8 +1688,9 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors, if (s->has_zero_init) { break; } - ret = blk_pwrite_zeroes(s->target, sector_num << BDRV_SECTOR_BITS, - n << BDRV_SECTOR_BITS, 0); + ret = blk_co_pwrite_zeroes(s->target, + sector_num << BDRV_SECTOR_BITS, + n << BDRV_SECTOR_BITS, 0); if (ret < 0) { return ret; } @@ -1665,12 +1705,122 @@ static int convert_write(ImgConvertState *s, int64_t sector_num, int nb_sectors, return 0; } -static int convert_do_copy(ImgConvertState *s) +static void coroutine_fn convert_co_do_copy(void *opaque) { + ImgConvertState *s = opaque; uint8_t *buf = NULL; - int64_t sector_num, allocated_done; - int ret; - int n; + int ret, i; + int index = -1; + + for (i = 0; i < s->num_coroutines; i++) { + if (s->co[i] == qemu_coroutine_self()) { + index = i; + break; + } + } + assert(index >= 0); + + s->running_coroutines++; + buf = blk_blockalign(s->target, s->buf_sectors * BDRV_SECTOR_SIZE); + + while (1) { + int n; + int64_t sector_num; + enum ImgConvertBlockStatus status; + + qemu_co_mutex_lock(&s->lock); + if (s->ret != -EINPROGRESS || s->sector_num >= s->total_sectors) { + qemu_co_mutex_unlock(&s->lock); + goto out; + } + n = convert_iteration_sectors(s, s->sector_num); + if (n < 0) { + qemu_co_mutex_unlock(&s->lock); + s->ret = n; + goto out; + } + /* save current sector and allocation status to local variables */ + sector_num = s->sector_num; + status = s->status; + if (!s->min_sparse && s->status == BLK_ZERO) { + n = MIN(n, s->buf_sectors); + } + /* increment global sector counter so that other coroutines can + * already continue reading beyond this request */ + s->sector_num += n; + qemu_co_mutex_unlock(&s->lock); + + if (status == BLK_DATA || (!s->min_sparse && status == BLK_ZERO)) { + s->allocated_done += n; + qemu_progress_print(100.0 * s->allocated_done / + s->allocated_sectors, 0); + } + + if (status == BLK_DATA) { + ret = convert_co_read(s, sector_num, n, buf); + if (ret < 0) { + error_report("error while reading sector %" PRId64 + ": %s", sector_num, strerror(-ret)); + s->ret = ret; + goto out; + } + } else if (!s->min_sparse && status == BLK_ZERO) { + status = BLK_DATA; + memset(buf, 0x00, n * BDRV_SECTOR_SIZE); + } + + if (s->wr_in_order) { + /* keep writes in order */ + while (s->wr_offs != sector_num) { + if (s->ret != -EINPROGRESS) { + goto out; + } + s->wait_sector_num[index] = sector_num; + qemu_coroutine_yield(); + } + s->wait_sector_num[index] = -1; + } + + ret = convert_co_write(s, sector_num, n, buf, status); + if (ret < 0) { + error_report("error while writing sector %" PRId64 + ": %s", sector_num, strerror(-ret)); + s->ret = ret; + goto out; + } + + if (s->wr_in_order) { + /* reenter the coroutine that might have waited + * for this write to complete */ + s->wr_offs = sector_num + n; + for (i = 0; i < s->num_coroutines; i++) { + if (s->co[i] && s->wait_sector_num[i] == s->wr_offs) { + /* + * A -> B -> A cannot occur because A has + * s->wait_sector_num[i] == -1 during A -> B. Therefore + * B will never enter A during this time window. + */ + qemu_coroutine_enter(s->co[i]); + break; + } + } + } + } + +out: + qemu_vfree(buf); + s->co[index] = NULL; + s->running_coroutines--; + if (!s->running_coroutines && s->ret == -EINPROGRESS) { + /* the convert job finished successfully */ + s->ret = 0; + } +} + +static int convert_do_copy(ImgConvertState *s) +{ + int ret, i, n; + int64_t sector_num = 0; /* Check whether we have zero initialisation or can get it efficiently */ s->has_zero_init = s->min_sparse && !s->target_has_backing @@ -1691,21 +1841,15 @@ static int convert_do_copy(ImgConvertState *s) if (s->compressed) { if (s->cluster_sectors <= 0 || s->cluster_sectors > s->buf_sectors) { error_report("invalid cluster size"); - ret = -EINVAL; - goto fail; + return -EINVAL; } s->buf_sectors = s->cluster_sectors; } - buf = blk_blockalign(s->target, s->buf_sectors * BDRV_SECTOR_SIZE); - /* Calculate allocated sectors for progress */ - s->allocated_sectors = 0; - sector_num = 0; while (sector_num < s->total_sectors) { n = convert_iteration_sectors(s, sector_num); if (n < 0) { - ret = n; - goto fail; + return n; } if (s->status == BLK_DATA || (!s->min_sparse && s->status == BLK_ZERO)) { @@ -1715,61 +1859,29 @@ static int convert_do_copy(ImgConvertState *s) } /* Do the copy */ - s->src_cur = 0; - s->src_cur_offset = 0; s->sector_next_status = 0; + s->ret = -EINPROGRESS; - sector_num = 0; - allocated_done = 0; - - while (sector_num < s->total_sectors) { - n = convert_iteration_sectors(s, sector_num); - if (n < 0) { - ret = n; - goto fail; - } - if (s->status == BLK_DATA || (!s->min_sparse && s->status == BLK_ZERO)) - { - allocated_done += n; - qemu_progress_print(100.0 * allocated_done / s->allocated_sectors, - 0); - } - - if (s->status == BLK_DATA) { - ret = convert_read(s, sector_num, n, buf); - if (ret < 0) { - error_report("error while reading sector %" PRId64 - ": %s", sector_num, strerror(-ret)); - goto fail; - } - } else if (!s->min_sparse && s->status == BLK_ZERO) { - n = MIN(n, s->buf_sectors); - memset(buf, 0, n * BDRV_SECTOR_SIZE); - s->status = BLK_DATA; - } - - ret = convert_write(s, sector_num, n, buf); - if (ret < 0) { - error_report("error while writing sector %" PRId64 - ": %s", sector_num, strerror(-ret)); - goto fail; - } + qemu_co_mutex_init(&s->lock); + for (i = 0; i < s->num_coroutines; i++) { + s->co[i] = qemu_coroutine_create(convert_co_do_copy, s); + s->wait_sector_num[i] = -1; + qemu_coroutine_enter(s->co[i]); + } - sector_num += n; + while (s->ret == -EINPROGRESS) { + main_loop_wait(false); } - if (s->compressed) { + if (s->compressed && !s->ret) { /* signal EOF to align */ ret = blk_pwrite_compressed(s->target, 0, NULL, 0); if (ret < 0) { - goto fail; + return ret; } } - ret = 0; -fail: - qemu_vfree(buf); - return ret; + return s->ret; } static int img_convert(int argc, char **argv) @@ -1797,6 +1909,8 @@ static int img_convert(int argc, char **argv) QemuOpts *sn_opts = NULL; ImgConvertState state; bool image_opts = false; + bool wr_in_order = true; + long num_coroutines = 8; fmt = NULL; out_fmt = "raw"; @@ -1812,7 +1926,7 @@ static int img_convert(int argc, char **argv) {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, {0, 0, 0, 0} }; - c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qn", + c = getopt_long(argc, argv, "hf:O:B:ce6o:s:l:S:pt:T:qnm:W", long_options, NULL); if (c == -1) { break; @@ -1904,6 +2018,18 @@ static int img_convert(int argc, char **argv) case 'n': skip_create = 1; break; + case 'm': + if (qemu_strtol(optarg, NULL, 0, &num_coroutines) || + num_coroutines < 1 || num_coroutines > MAX_COROUTINES) { + error_report("Invalid number of coroutines. Allowed number of" + " coroutines is between 1 and %d", MAX_COROUTINES); + ret = -1; + goto fail_getopt; + } + break; + case 'W': + wr_in_order = false; + break; case OPTION_OBJECT: opts = qemu_opts_parse_noisily(&qemu_object_opts, optarg, true); @@ -1923,6 +2049,12 @@ static int img_convert(int argc, char **argv) goto fail_getopt; } + if (!wr_in_order && compress) { + error_report("Out of order write and compress are mutually exclusive"); + ret = -1; + goto fail_getopt; + } + /* Initialize before goto out */ if (quiet) { progress = 0; @@ -2163,6 +2295,8 @@ static int img_convert(int argc, char **argv) .min_sparse = min_sparse, .cluster_sectors = cluster_sectors, .buf_sectors = bufsectors, + .wr_in_order = wr_in_order, + .num_coroutines = num_coroutines, }; ret = convert_do_copy(&state); @@ -3289,7 +3423,7 @@ static int img_resize(int argc, char **argv) qemu_opts_del(param); blk = img_open(image_opts, filename, fmt, - BDRV_O_RDWR, false, quiet); + BDRV_O_RDWR | BDRV_O_RESIZE, false, quiet); if (!blk) { ret = -1; goto out; diff --git a/qemu-img.texi b/qemu-img.texi index 174aae38b7..c81db3e81c 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -137,6 +137,12 @@ Parameters to convert subcommand: @item -n Skip the creation of the target volume +@item -m +Number of parallel coroutines for the convert process +@item -W +Allow out-of-order writes to the destination. This option improves performance, +but is only recommended for preallocated devices like host devices or other +raw block devices. @end table Parameters to dd subcommand: @@ -296,7 +302,7 @@ Error on reading data @end table -@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} +@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-m @var{num_coroutines}] [-W] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename} Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated) to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c} @@ -326,6 +332,14 @@ skipped. This is useful for formats such as @code{rbd} if the target volume has already been created with site specific options that cannot be supplied through qemu-img. +Out of order writes can be enabled with @code{-W} to improve performance. +This is only recommended for preallocated devices like host devices or other +raw block devices. Out of order write does not work in combination with +creating compressed images. + +@var{num_coroutines} specifies how many coroutines work in parallel during +the convert process (defaults to 8). + @item dd [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output} Dd copies from @var{input} file to @var{output} file converting it from diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 7ac1576d4c..2c48f9ce1a 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -83,6 +83,29 @@ static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc, } return 0; } + + /* Request additional permissions if necessary for this command. The caller + * is responsible for restoring the original permissions afterwards if this + * is what it wants. */ + if (ct->perm && blk_is_available(blk)) { + uint64_t orig_perm, orig_shared_perm; + blk_get_perm(blk, &orig_perm, &orig_shared_perm); + + if (ct->perm & ~orig_perm) { + uint64_t new_perm; + Error *local_err = NULL; + int ret; + + new_perm = orig_perm | ct->perm; + + ret = blk_set_perm(blk, new_perm, orig_shared_perm, &local_err); + if (ret < 0) { + error_report_err(local_err); + return 0; + } + } + } + optind = 0; return ct->cfunc(blk, argc, argv); } @@ -918,6 +941,7 @@ static const cmdinfo_t write_cmd = { .name = "write", .altname = "w", .cfunc = write_f, + .perm = BLK_PERM_WRITE, .argmin = 2, .argmax = -1, .args = "[-bcCfquz] [-P pattern] off len", @@ -1093,6 +1117,7 @@ static int writev_f(BlockBackend *blk, int argc, char **argv); static const cmdinfo_t writev_cmd = { .name = "writev", .cfunc = writev_f, + .perm = BLK_PERM_WRITE, .argmin = 2, .argmax = -1, .args = "[-Cfq] [-P pattern] off len [len..]", @@ -1392,6 +1417,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv); static const cmdinfo_t aio_write_cmd = { .name = "aio_write", .cfunc = aio_write_f, + .perm = BLK_PERM_WRITE, .argmin = 2, .argmax = -1, .args = "[-Cfiquz] [-P pattern] off len [len..]", @@ -1556,6 +1582,7 @@ static const cmdinfo_t truncate_cmd = { .name = "truncate", .altname = "t", .cfunc = truncate_f, + .perm = BLK_PERM_WRITE | BLK_PERM_RESIZE, .argmin = 1, .argmax = 1, .args = "off", @@ -1653,6 +1680,7 @@ static const cmdinfo_t discard_cmd = { .name = "discard", .altname = "d", .cfunc = discard_f, + .perm = BLK_PERM_WRITE, .argmin = 2, .argmax = -1, .args = "[-Cq] off len", diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out index 4673b67f37..34e66db691 100644 --- a/tests/qemu-iotests/049.out +++ b/tests/qemu-iotests/049.out @@ -95,14 +95,14 @@ qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1024 qemu-img: Image size must be less than 8 EiB! qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2 -qemu-img: Parameter 'size' expects a non-negative number below 2^64 +qemu-img: Value '-1024' is out of range for parameter 'size' qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2' qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k qemu-img: Image size must be less than 8 EiB! qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2 -qemu-img: Parameter 'size' expects a non-negative number below 2^64 +qemu-img: Value '-1k' is out of range for parameter 'size' qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2' qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte @@ -110,15 +110,19 @@ qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. qemu-img create -f qcow2 -o size=1kilobyte TEST_DIR/t.qcow2 -Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +qemu-img: Parameter 'size' expects a non-negative number below 2^64 +Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta- +and exabytes, respectively. +qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2' qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- foobar qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for qemu-img: kilobytes, megabytes, gigabytes, terabytes, petabytes and exabytes. qemu-img create -f qcow2 -o size=foobar TEST_DIR/t.qcow2 -qemu-img: Parameter 'size' expects a size -You may use k, M, G or T suffixes for kilobytes, megabytes, gigabytes and terabytes. +qemu-img: Parameter 'size' expects a non-negative number below 2^64 +Optional suffix k, M, G, T, P or E means kilo-, mega-, giga-, tera-, peta- +and exabytes, respectively. qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2' == Check correct interpretation of suffixes for cluster size == diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out index e206ad6c29..c6f4eef215 100644 --- a/tests/qemu-iotests/051.pc.out +++ b/tests/qemu-iotests/051.pc.out @@ -179,7 +179,7 @@ q[K[Dqu[K[D[Dqui[K[D[D[Dquit[K Testing: -drive file=TEST_DIR/t.qcow2,if=ide,readonly=on QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: Can't use a read-only drive +(qemu) QEMU_PROG: Block node is read-only QEMU_PROG: Initialization of device ide-hd failed: Device initialization failed. Testing: -drive file=TEST_DIR/t.qcow2,if=scsi,readonly=on @@ -201,12 +201,12 @@ QEMU X.Y.Z monitor - type 'help' for more information Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-drive,drive=disk QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: -device ide-drive,drive=disk: Can't use a read-only drive +(qemu) QEMU_PROG: -device ide-drive,drive=disk: Block node is read-only QEMU_PROG: -device ide-drive,drive=disk: Device initialization failed. Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device ide-hd,drive=disk QEMU X.Y.Z monitor - type 'help' for more information -(qemu) QEMU_PROG: -device ide-hd,drive=disk: Can't use a read-only drive +(qemu) QEMU_PROG: -device ide-hd,drive=disk: Block node is read-only QEMU_PROG: -device ide-hd,drive=disk: Device initialization failed. Testing: -drive file=TEST_DIR/t.qcow2,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index 1d3fd04b65..aafcd249f6 100755 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -48,7 +48,8 @@ class TestSingleDrive(iotests.QMPTestCase): def setUp(self): qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len)) - self.vm = iotests.VM().add_drive(test_img).add_drive(blockdev_target_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.add_drive(blockdev_target_img, interface="none") if iotests.qemu_default_machine == 'pc': self.vm.add_drive(None, 'media=cdrom', 'ide') self.vm.launch() @@ -164,7 +165,8 @@ class TestSetSpeed(iotests.QMPTestCase): def setUp(self): qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len)) - self.vm = iotests.VM().add_drive(test_img).add_drive(blockdev_target_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.add_drive(blockdev_target_img, interface="none") self.vm.launch() def tearDown(self): @@ -247,7 +249,8 @@ class TestSingleTransaction(iotests.QMPTestCase): def setUp(self): qemu_img('create', '-f', iotests.imgfmt, blockdev_target_img, str(image_len)) - self.vm = iotests.VM().add_drive(test_img).add_drive(blockdev_target_img) + self.vm = iotests.VM().add_drive(test_img) + self.vm.add_drive(blockdev_target_img, interface="none") if iotests.qemu_default_machine == 'pc': self.vm.add_drive(None, 'media=cdrom', 'ide') self.vm.launch() @@ -460,7 +463,7 @@ class TestDriveCompression(iotests.QMPTestCase): qemu_img('create', '-f', fmt, blockdev_target_img, str(TestDriveCompression.image_len), *args) - self.vm.add_drive(blockdev_target_img, format=fmt) + self.vm.add_drive(blockdev_target_img, format=fmt, interface="none") self.vm.launch() diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out index 08e4bb7218..182acb42cf 100644 --- a/tests/qemu-iotests/085.out +++ b/tests/qemu-iotests/085.out @@ -74,7 +74,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/ === Invalid command - snapshot node used as backing hd === -{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'virtio0'"}} +{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'snap_12'"}} === Invalid command - snapshot node has a backing image === diff --git a/tests/qemu-iotests/141 b/tests/qemu-iotests/141 index 3ba79f027a..6d8f0a1a84 100755 --- a/tests/qemu-iotests/141 +++ b/tests/qemu-iotests/141 @@ -67,7 +67,7 @@ test_blockjob() _send_qemu_cmd $QEMU_HANDLE \ "{'execute': 'x-blockdev-del', 'arguments': {'node-name': 'drv0'}}" \ - 'error' + 'error' | _filter_generated_node_ids _send_qemu_cmd $QEMU_HANDLE \ "{'execute': 'block-job-cancel', diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out index 195ca1a604..82e763b68d 100644 --- a/tests/qemu-iotests/141.out +++ b/tests/qemu-iotests/141.out @@ -20,7 +20,7 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t. Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}} {"return": {}} -{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "mirror"}} {"return": {}} @@ -30,7 +30,7 @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t. {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} {"return": {}} -{"error": {"class": "GenericError", "desc": "Node drv0 is in use"}} +{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}} {"return": {}} diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out index 6b7edaf28f..54b53293d7 100644 --- a/tests/qemu-iotests/172.out +++ b/tests/qemu-iotests/172.out @@ -28,6 +28,7 @@ Testing: opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "288" @@ -57,6 +58,7 @@ Testing: -fda TEST_DIR/t.qcow2 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fdb TEST_DIR/t.qcow2 @@ -83,6 +85,7 @@ Testing: -fdb TEST_DIR/t.qcow2 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -93,6 +96,7 @@ Testing: -fdb TEST_DIR/t.qcow2 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "288" Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2 @@ -119,6 +123,7 @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -129,6 +134,7 @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" @@ -158,6 +164,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1 @@ -184,6 +191,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -194,6 +202,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "288" Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t.qcow2,index=1 @@ -220,6 +229,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -230,6 +240,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" @@ -259,6 +270,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 @@ -285,6 +297,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -global isa-fdc.driveB=none1 @@ -311,6 +324,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -321,6 +335,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" @@ -350,6 +365,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1 @@ -376,6 +392,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0 -device floppy,drive=none1,unit=1 @@ -402,6 +419,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -412,6 +430,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" @@ -441,6 +460,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa- opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -451,6 +471,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa- opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 @@ -477,6 +498,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa- opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -487,6 +509,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa- opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 @@ -513,6 +536,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa- opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 @@ -539,6 +563,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa- opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" @@ -568,6 +593,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -578,6 +604,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1 @@ -604,6 +631,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -614,6 +642,7 @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0 @@ -640,6 +669,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 1 (0x1) @@ -650,6 +680,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=0 @@ -676,6 +707,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 1 (0x1) @@ -686,6 +718,7 @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device flop opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=0 @@ -723,6 +756,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -733,6 +767,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1 @@ -759,6 +794,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -769,6 +805,7 @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=0 @@ -802,6 +839,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -812,6 +850,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=1 @@ -838,6 +877,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 0 (0x0) @@ -848,6 +888,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 -device floppy,drive=none1 @@ -874,6 +915,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 1 (0x1) @@ -884,6 +926,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveB=none0 -device floppy,drive=none1,unit=0 @@ -910,6 +953,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" dev: floppy, id "" unit = 1 (0x1) @@ -920,6 +964,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2 -global isa-fdc.driveA=none0 -device floppy,drive=none1,unit=0 @@ -964,6 +1009,7 @@ Testing: -device floppy opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "288" Testing: -device floppy,drive-type=120 @@ -990,6 +1036,7 @@ Testing: -device floppy,drive-type=120 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "120" Testing: -device floppy,drive-type=144 @@ -1016,6 +1063,7 @@ Testing: -device floppy,drive-type=144 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -device floppy,drive-type=288 @@ -1042,6 +1090,7 @@ Testing: -device floppy,drive-type=288 opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "288" @@ -1071,6 +1120,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "120" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-type=288 @@ -1097,6 +1147,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "288" @@ -1126,6 +1177,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physical_block_size=512 @@ -1152,6 +1204,7 @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica opt_io_size = 0 (0x0) discard_granularity = 4294967295 (0xffffffff) write-cache = "auto" + share-rw = false drive-type = "144" Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical_block_size=4096 diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c index f6dfd08746..4ccbda14af 100644 --- a/tests/test-blockjob-txn.c +++ b/tests/test-blockjob-txn.c @@ -101,9 +101,9 @@ static BlockJob *test_block_job_start(unsigned int iterations, g_assert_nonnull(bs); snprintf(job_id, sizeof(job_id), "job%u", counter++); - s = block_job_create(job_id, &test_block_job_driver, bs, 0, - BLOCK_JOB_DEFAULT, test_block_job_cb, - data, &error_abort); + s = block_job_create(job_id, &test_block_job_driver, bs, + 0, BLK_PERM_ALL, 0, BLOCK_JOB_DEFAULT, + test_block_job_cb, data, &error_abort); s->iterations = iterations; s->use_timer = use_timer; s->rc = rc; diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c index 068c9e419b..740e740398 100644 --- a/tests/test-blockjob.c +++ b/tests/test-blockjob.c @@ -30,8 +30,9 @@ static BlockJob *do_test_id(BlockBackend *blk, const char *id, BlockJob *job; Error *errp = NULL; - job = block_job_create(id, &test_block_job_driver, blk_bs(blk), 0, - BLOCK_JOB_DEFAULT, block_job_cb, NULL, &errp); + job = block_job_create(id, &test_block_job_driver, blk_bs(blk), + 0, BLK_PERM_ALL, 0, BLOCK_JOB_DEFAULT, block_job_cb, + NULL, &errp); if (should_succeed) { g_assert_null(errp); g_assert_nonnull(job); @@ -53,13 +54,14 @@ static BlockJob *do_test_id(BlockBackend *blk, const char *id, * BlockDriverState inserted. */ static BlockBackend *create_blk(const char *name) { - BlockBackend *blk = blk_new(); + /* No I/O is performed on this device */ + BlockBackend *blk = blk_new(0, BLK_PERM_ALL); BlockDriverState *bs; bs = bdrv_open("null-co://", NULL, NULL, 0, &error_abort); g_assert_nonnull(bs); - blk_insert_bs(blk, bs); + blk_insert_bs(blk, bs, &error_abort); bdrv_unref(bs); if (name) { diff --git a/tests/test-throttle.c b/tests/test-throttle.c index 363b59a38f..bd7c501b2e 100644 --- a/tests/test-throttle.c +++ b/tests/test-throttle.c @@ -593,9 +593,10 @@ static void test_groups(void) BlockBackend *blk1, *blk2, *blk3; BlockBackendPublic *blkp1, *blkp2, *blkp3; - blk1 = blk_new(); - blk2 = blk_new(); - blk3 = blk_new(); + /* No actual I/O is performed on these devices */ + blk1 = blk_new(0, BLK_PERM_ALL); + blk2 = blk_new(0, BLK_PERM_ALL); + blk3 = blk_new(0, BLK_PERM_ALL); blkp1 = blk_get_public(blk1); blkp2 = blk_get_public(blk2); diff --git a/util/qemu-option.c b/util/qemu-option.c index 419f2528b8..5ce1b5c246 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -179,7 +179,7 @@ void parse_option_size(const char *name, const char *value, err = qemu_strtosz(value, NULL, &size); if (err == -ERANGE) { - error_setg(errp, "Value '%s' is too large for parameter '%s'", + error_setg(errp, "Value '%s' is out of range for parameter '%s'", value, name); return; } |