diff options
| -rw-r--r-- | block.c | 182 | ||||
| -rw-r--r-- | block/block-backend.c | 7 | ||||
| -rw-r--r-- | block/commit.c | 18 | ||||
| -rw-r--r-- | block/gluster.c | 127 | ||||
| -rw-r--r-- | block/mirror.c | 35 | ||||
| -rw-r--r-- | block/sheepdog.c | 453 | ||||
| -rw-r--r-- | blockdev.c | 6 | ||||
| -rw-r--r-- | include/block/block.h | 4 | ||||
| -rw-r--r-- | include/block/block_int.h | 6 | ||||
| -rw-r--r-- | qapi-schema.json | 38 | ||||
| -rw-r--r-- | qapi/block-core.json | 73 |
11 files changed, 623 insertions, 326 deletions
diff --git a/block.c b/block.c index fe7bddbe5c..b404ef251a 100644 --- a/block.c +++ b/block.c @@ -1403,7 +1403,8 @@ static int bdrv_fill_options(QDict **options, const char *filename, * or bdrv_abort_perm_update(). */ static int bdrv_check_perm(BlockDriverState *bs, uint64_t cumulative_perms, - uint64_t cumulative_shared_perms, Error **errp) + uint64_t cumulative_shared_perms, + GSList *ignore_children, Error **errp) { BlockDriver *drv = bs->drv; BdrvChild *c; @@ -1439,7 +1440,8 @@ static int bdrv_check_perm(BlockDriverState *bs, uint64_t cumulative_perms, 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); + ret = bdrv_child_check_perm(c, cur_perm, cur_shared, ignore_children, + errp); if (ret < 0) { return ret; } @@ -1559,15 +1561,15 @@ static char *bdrv_perm_names(uint64_t perm) /* * 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. + * @new_used_perm/@new_shared_perm as its permissions. If @ignore_children is + * set, the BdrvChild objects in this list are 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) + GSList *ignore_children, Error **errp) { BdrvChild *c; uint64_t cumulative_perms = new_used_perm; @@ -1577,7 +1579,7 @@ static int bdrv_check_update_perm(BlockDriverState *bs, uint64_t new_used_perm, assert(new_shared_perm & BLK_PERM_WRITE_UNCHANGED); QLIST_FOREACH(c, &bs->parents, next_parent) { - if (c == ignore_child) { + if (g_slist_find(ignore_children, c)) { continue; } @@ -1607,15 +1609,22 @@ static int bdrv_check_update_perm(BlockDriverState *bs, uint64_t new_used_perm, cumulative_shared_perms &= c->shared_perm; } - return bdrv_check_perm(bs, cumulative_perms, cumulative_shared_perms, errp); + return bdrv_check_perm(bs, cumulative_perms, cumulative_shared_perms, + ignore_children, 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) + GSList *ignore_children, Error **errp) { - return bdrv_check_update_perm(c->bs, perm, shared, c, errp); + int ret; + + ignore_children = g_slist_prepend(g_slist_copy(ignore_children), c); + ret = bdrv_check_update_perm(c->bs, perm, shared, ignore_children, errp); + g_slist_free(ignore_children); + + return ret; } void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared) @@ -1640,7 +1649,7 @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, { int ret; - ret = bdrv_child_check_perm(c, perm, shared, errp); + ret = bdrv_child_check_perm(c, perm, shared, NULL, errp); if (ret < 0) { bdrv_child_abort_perm_update(c); return ret; @@ -1718,11 +1727,10 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c, *nshared = shared; } -static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs, - bool check_new_perm) +static void bdrv_replace_child_noperm(BdrvChild *child, + BlockDriverState *new_bs) { BlockDriverState *old_bs = child->bs; - uint64_t perm, shared_perm; if (old_bs) { if (old_bs->quiesce_counter && child->role->drained_end) { @@ -1732,13 +1740,6 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs, 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; @@ -1749,15 +1750,35 @@ static void bdrv_replace_child(BdrvChild *child, BlockDriverState *new_bs, child->role->drained_begin(child); } + if (child->role->attach) { + child->role->attach(child); + } + } +} + +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) { + /* 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, NULL, &error_abort); + bdrv_set_perm(old_bs, perm, shared_perm); + } + + bdrv_replace_child_noperm(child, new_bs); + + if (new_bs) { bdrv_get_cumulative_perm(new_bs, &perm, &shared_perm); if (check_new_perm) { - bdrv_check_perm(new_bs, perm, shared_perm, &error_abort); + bdrv_check_perm(new_bs, perm, shared_perm, NULL, &error_abort); } bdrv_set_perm(new_bs, perm, shared_perm); - - if (child->role->attach) { - child->role->attach(child); - } } } @@ -2891,35 +2912,82 @@ void bdrv_close_all(void) assert(QTAILQ_EMPTY(&all_bdrv_states)); } -static void change_parent_backing_link(BlockDriverState *from, - BlockDriverState *to) +static bool should_update_child(BdrvChild *c, BlockDriverState *to) +{ + BdrvChild *to_c; + + if (c->role->stay_at_node) { + return false; + } + + if (c->role == &child_backing) { + /* 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; + } + } + if (to_c) { + return false; + } + } + + return true; +} + +void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, + Error **errp) { - BdrvChild *c, *next, *to_c; + BdrvChild *c, *next; + GSList *list = NULL, *p; + uint64_t old_perm, old_shared; + uint64_t perm = 0, shared = BLK_PERM_ALL; + int ret; + + assert(!atomic_read(&from->in_flight)); + assert(!atomic_read(&to->in_flight)); + + /* Make sure that @from doesn't go away until we have successfully attached + * all of its parents to @to. */ + bdrv_ref(from); + /* Put all parents into @list and calculate their cumulative permissions */ QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) { - if (c->role->stay_at_node) { + if (!should_update_child(c, to)) { continue; } - if (c->role == &child_backing) { - /* 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; - } - } - if (to_c) { - continue; - } - } + list = g_slist_prepend(list, c); + perm |= c->perm; + shared &= c->shared_perm; + } + + /* Check whether the required permissions can be granted on @to, ignoring + * all BdrvChild in @list so that they can't block themselves. */ + ret = bdrv_check_update_perm(to, perm, shared, list, errp); + if (ret < 0) { + bdrv_abort_perm_update(to); + goto out; + } + + /* Now actually perform the change. We performed the permission check for + * all elements of @list at once, so set the permissions all at once at the + * very end. */ + for (p = list; p != NULL; p = p->next) { + c = p->data; bdrv_ref(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_replace_child_noperm(c, to); bdrv_unref(from); } + + bdrv_get_cumulative_perm(to, &old_perm, &old_shared); + bdrv_set_perm(to, old_perm | perm, old_shared | shared); + +out: + g_slist_free(list); + bdrv_unref(from); } /* @@ -2943,16 +3011,18 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, { Error *local_err = NULL; - 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_replace_node(bs_top, bs_new, &local_err); + if (local_err) { + error_propagate(errp, local_err); + bdrv_set_backing_hd(bs_new, NULL, &error_abort); + goto out; + } /* bs_new is now referenced by its new parents, we don't need the * additional reference any more. */ @@ -2960,18 +3030,6 @@ out: bdrv_unref(bs_new); } -void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new) -{ - assert(!bdrv_requests_pending(old)); - assert(!bdrv_requests_pending(new)); - - bdrv_ref(old); - - change_parent_backing_link(old, new); - - bdrv_unref(old); -} - static void bdrv_delete(BlockDriverState *bs) { assert(!bs->job); diff --git a/block/block-backend.c b/block/block-backend.c index daa7908d01..5742c09c2c 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -213,7 +213,12 @@ BlockBackend *blk_new_open(const char *filename, const char *reference, } blk->root = bdrv_root_attach_child(bs, "root", &child_root, - perm, BLK_PERM_ALL, blk, &error_abort); + perm, BLK_PERM_ALL, blk, errp); + if (!blk->root) { + bdrv_unref(bs); + blk_unref(blk); + return NULL; + } return blk; } diff --git a/block/commit.c b/block/commit.c index 22a0a4db98..9c4198837f 100644 --- a/block/commit.c +++ b/block/commit.c @@ -316,8 +316,20 @@ void commit_start(const char *job_id, BlockDriverState *bs, goto fail; } - bdrv_set_backing_hd(commit_top_bs, top, &error_abort); - bdrv_set_backing_hd(overlay_bs, commit_top_bs, &error_abort); + bdrv_set_backing_hd(commit_top_bs, top, &local_err); + if (local_err) { + bdrv_unref(commit_top_bs); + commit_top_bs = NULL; + error_propagate(errp, local_err); + goto fail; + } + bdrv_set_backing_hd(overlay_bs, commit_top_bs, &local_err); + if (local_err) { + bdrv_unref(commit_top_bs); + commit_top_bs = NULL; + error_propagate(errp, local_err); + goto fail; + } s->commit_top_bs = commit_top_bs; bdrv_unref(commit_top_bs); @@ -364,7 +376,7 @@ void commit_start(const char *job_id, BlockDriverState *bs, /* 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); + ret = blk_insert_bs(s->top, top, errp); if (ret < 0) { goto fail; } diff --git a/block/gluster.c b/block/gluster.c index 56b4abe3a7..a577daef10 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -12,6 +12,7 @@ #include "block/block_int.h" #include "qapi/error.h" #include "qapi/qmp/qerror.h" +#include "qapi/util.h" #include "qemu/uri.h" #include "qemu/error-report.h" #include "qemu/cutils.h" @@ -151,7 +152,7 @@ static QemuOptsList runtime_type_opts = { { .name = GLUSTER_OPT_TYPE, .type = QEMU_OPT_STRING, - .help = "tcp|unix", + .help = "inet|unix", }, { /* end of list */ } }, @@ -170,14 +171,14 @@ static QemuOptsList runtime_unix_opts = { }, }; -static QemuOptsList runtime_tcp_opts = { - .name = "gluster_tcp", - .head = QTAILQ_HEAD_INITIALIZER(runtime_tcp_opts.head), +static QemuOptsList runtime_inet_opts = { + .name = "gluster_inet", + .head = QTAILQ_HEAD_INITIALIZER(runtime_inet_opts.head), .desc = { { .name = GLUSTER_OPT_TYPE, .type = QEMU_OPT_STRING, - .help = "tcp|unix", + .help = "inet|unix", }, { .name = GLUSTER_OPT_HOST, @@ -320,7 +321,7 @@ static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path) static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf, const char *filename) { - GlusterServer *gsconf; + SocketAddressFlat *gsconf; URI *uri; QueryParams *qp = NULL; bool is_unix = false; @@ -331,19 +332,19 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf, return -EINVAL; } - gconf->server = g_new0(GlusterServerList, 1); - gconf->server->value = gsconf = g_new0(GlusterServer, 1); + gconf->server = g_new0(SocketAddressFlatList, 1); + gconf->server->value = gsconf = g_new0(SocketAddressFlat, 1); /* transport */ if (!uri->scheme || !strcmp(uri->scheme, "gluster")) { - gsconf->type = GLUSTER_TRANSPORT_TCP; + gsconf->type = SOCKET_ADDRESS_FLAT_TYPE_INET; } else if (!strcmp(uri->scheme, "gluster+tcp")) { - gsconf->type = GLUSTER_TRANSPORT_TCP; + gsconf->type = SOCKET_ADDRESS_FLAT_TYPE_INET; } else if (!strcmp(uri->scheme, "gluster+unix")) { - gsconf->type = GLUSTER_TRANSPORT_UNIX; + gsconf->type = SOCKET_ADDRESS_FLAT_TYPE_UNIX; is_unix = true; } else if (!strcmp(uri->scheme, "gluster+rdma")) { - gsconf->type = GLUSTER_TRANSPORT_TCP; + gsconf->type = SOCKET_ADDRESS_FLAT_TYPE_INET; error_report("Warning: rdma feature is not supported, falling " "back to tcp"); } else { @@ -373,11 +374,11 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf, } gsconf->u.q_unix.path = g_strdup(qp->p[0].value); } else { - gsconf->u.tcp.host = g_strdup(uri->server ? uri->server : "localhost"); + gsconf->u.inet.host = g_strdup(uri->server ? uri->server : "localhost"); if (uri->port) { - gsconf->u.tcp.port = g_strdup_printf("%d", uri->port); + gsconf->u.inet.port = g_strdup_printf("%d", uri->port); } else { - gsconf->u.tcp.port = g_strdup_printf("%d", GLUSTER_DEFAULT_PORT); + gsconf->u.inet.port = g_strdup_printf("%d", GLUSTER_DEFAULT_PORT); } } @@ -395,7 +396,7 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf, struct glfs *glfs; int ret; int old_errno; - GlusterServerList *server; + SocketAddressFlatList *server; unsigned long long port; glfs = glfs_find_preopened(gconf->volume); @@ -411,21 +412,19 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf, glfs_set_preopened(gconf->volume, glfs); for (server = gconf->server; server; server = server->next) { - if (server->value->type == GLUSTER_TRANSPORT_UNIX) { - ret = glfs_set_volfile_server(glfs, - GlusterTransport_lookup[server->value->type], + if (server->value->type == SOCKET_ADDRESS_FLAT_TYPE_UNIX) { + ret = glfs_set_volfile_server(glfs, "unix", server->value->u.q_unix.path, 0); } else { - if (parse_uint_full(server->value->u.tcp.port, &port, 10) < 0 || + if (parse_uint_full(server->value->u.inet.port, &port, 10) < 0 || port > 65535) { error_setg(errp, "'%s' is not a valid port number", - server->value->u.tcp.port); + server->value->u.inet.port); errno = EINVAL; goto out; } - ret = glfs_set_volfile_server(glfs, - GlusterTransport_lookup[server->value->type], - server->value->u.tcp.host, + ret = glfs_set_volfile_server(glfs, "tcp", + server->value->u.inet.host, (int)port); } @@ -444,13 +443,13 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf, error_setg(errp, "Gluster connection for volume %s, path %s failed" " to connect", gconf->volume, gconf->path); for (server = gconf->server; server; server = server->next) { - if (server->value->type == GLUSTER_TRANSPORT_UNIX) { + if (server->value->type == SOCKET_ADDRESS_FLAT_TYPE_UNIX) { error_append_hint(errp, "hint: failed on socket %s ", server->value->u.q_unix.path); } else { error_append_hint(errp, "hint: failed on host %s and port %s ", - server->value->u.tcp.host, - server->value->u.tcp.port); + server->value->u.inet.host, + server->value->u.inet.port); } } @@ -474,23 +473,6 @@ out: return NULL; } -static int qapi_enum_parse(const char *opt) -{ - int i; - - if (!opt) { - return GLUSTER_TRANSPORT__MAX; - } - - for (i = 0; i < GLUSTER_TRANSPORT__MAX; i++) { - if (!strcmp(opt, GlusterTransport_lookup[i])) { - return i; - } - } - - return i; -} - /* * Convert the json formatted command line into qapi. */ @@ -498,8 +480,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, QDict *options, Error **errp) { QemuOpts *opts; - GlusterServer *gsconf; - GlusterServerList *curr = NULL; + SocketAddressFlat *gsconf = NULL; + SocketAddressFlatList *curr = NULL; QDict *backing_options = NULL; Error *local_err = NULL; char *str = NULL; @@ -547,25 +529,31 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, } ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE); - gsconf = g_new0(GlusterServer, 1); - gsconf->type = qapi_enum_parse(ptr); if (!ptr) { error_setg(&local_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE); error_append_hint(&local_err, GERR_INDEX_HINT, i); goto out; } - if (gsconf->type == GLUSTER_TRANSPORT__MAX) { - error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, - GLUSTER_OPT_TYPE, "tcp or unix"); + gsconf = g_new0(SocketAddressFlat, 1); + if (!strcmp(ptr, "tcp")) { + ptr = "inet"; /* accept legacy "tcp" */ + } + gsconf->type = qapi_enum_parse(SocketAddressFlatType_lookup, ptr, + SOCKET_ADDRESS_FLAT_TYPE__MAX, -1, + &local_err); + if (local_err) { + error_append_hint(&local_err, + "Parameter '%s' may be 'inet' or 'unix'\n", + GLUSTER_OPT_TYPE); error_append_hint(&local_err, GERR_INDEX_HINT, i); goto out; } qemu_opts_del(opts); - if (gsconf->type == GLUSTER_TRANSPORT_TCP) { - /* create opts info from runtime_tcp_opts list */ - opts = qemu_opts_create(&runtime_tcp_opts, NULL, 0, &error_abort); + if (gsconf->type == SOCKET_ADDRESS_FLAT_TYPE_INET) { + /* create opts info from runtime_inet_opts list */ + opts = qemu_opts_create(&runtime_inet_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, backing_options, &local_err); if (local_err) { goto out; @@ -578,7 +566,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, error_append_hint(&local_err, GERR_INDEX_HINT, i); goto out; } - gsconf->u.tcp.host = g_strdup(ptr); + gsconf->u.inet.host = g_strdup(ptr); ptr = qemu_opt_get(opts, GLUSTER_OPT_PORT); if (!ptr) { error_setg(&local_err, QERR_MISSING_PARAMETER, @@ -586,28 +574,28 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, error_append_hint(&local_err, GERR_INDEX_HINT, i); goto out; } - gsconf->u.tcp.port = g_strdup(ptr); + gsconf->u.inet.port = g_strdup(ptr); /* defend for unsupported fields in InetSocketAddress, * i.e. @ipv4, @ipv6 and @to */ ptr = qemu_opt_get(opts, GLUSTER_OPT_TO); if (ptr) { - gsconf->u.tcp.has_to = true; + gsconf->u.inet.has_to = true; } ptr = qemu_opt_get(opts, GLUSTER_OPT_IPV4); if (ptr) { - gsconf->u.tcp.has_ipv4 = true; + gsconf->u.inet.has_ipv4 = true; } ptr = qemu_opt_get(opts, GLUSTER_OPT_IPV6); if (ptr) { - gsconf->u.tcp.has_ipv6 = true; + gsconf->u.inet.has_ipv6 = true; } - if (gsconf->u.tcp.has_to) { + if (gsconf->u.inet.has_to) { error_setg(&local_err, "Parameter 'to' not supported"); goto out; } - if (gsconf->u.tcp.has_ipv4 || gsconf->u.tcp.has_ipv6) { + if (gsconf->u.inet.has_ipv4 || gsconf->u.inet.has_ipv6) { error_setg(&local_err, "Parameters 'ipv4/ipv6' not supported"); goto out; } @@ -632,16 +620,18 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, } if (gconf->server == NULL) { - gconf->server = g_new0(GlusterServerList, 1); + gconf->server = g_new0(SocketAddressFlatList, 1); gconf->server->value = gsconf; curr = gconf->server; } else { - curr->next = g_new0(GlusterServerList, 1); + curr->next = g_new0(SocketAddressFlatList, 1); curr->next->value = gsconf; curr = curr->next; } + gsconf = NULL; - qdict_del(backing_options, str); + QDECREF(backing_options); + backing_options = NULL; g_free(str); str = NULL; } @@ -650,11 +640,10 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf, out: error_propagate(errp, local_err); + qapi_free_SocketAddressFlat(gsconf); qemu_opts_del(opts); - if (str) { - qdict_del(backing_options, str); - g_free(str); - } + g_free(str); + QDECREF(backing_options); errno = EINVAL; return -errno; } @@ -683,7 +672,7 @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster *gconf, "file.volume=testvol,file.path=/path/a.qcow2" "[,file.debug=9]" "[,file.logfile=/path/filename.log]," - "file.server.0.type=tcp," + "file.server.0.type=inet," "file.server.0.host=1.2.3.4," "file.server.0.port=24007," "file.server.1.transport=unix," diff --git a/block/mirror.c b/block/mirror.c index 57f26c33a4..a5d30ee575 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -509,6 +509,13 @@ static void mirror_exit(BlockJob *job, void *opaque) * block_job_completed(). */ bdrv_ref(src); bdrv_ref(mirror_top_bs); + bdrv_ref(target_bs); + + /* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before + * inserting target_bs at s->to_replace, where we might not be able to get + * these permissions. */ + blk_unref(s->target); + s->target = NULL; /* We don't access the source any more. Dropping any WRITE/RESIZE is * required before it could become a backing file of target_bs. */ @@ -543,8 +550,12 @@ static void mirror_exit(BlockJob *job, void *opaque) /* The mirror job has no requests in flight any more, but we need to * drain potential other users of the BDS before changing the graph. */ bdrv_drained_begin(target_bs); - bdrv_replace_in_backing_chain(to_replace, target_bs); + bdrv_replace_node(to_replace, target_bs, &local_err); bdrv_drained_end(target_bs); + if (local_err) { + error_report_err(local_err); + data->ret = -EPERM; + } } if (s->to_replace) { bdrv_op_unblock_all(s->to_replace, s->replace_blocker); @@ -555,19 +566,19 @@ static void mirror_exit(BlockJob *job, void *opaque) aio_context_release(replace_aio_context); } g_free(s->replaces); - blk_unref(s->target); - s->target = NULL; + bdrv_unref(target_bs); /* 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. */ + * valid. Also give up permissions on mirror_top_bs->backing, which might + * block the removal. */ block_job_remove_all_bdrv(job); - bdrv_replace_in_backing_chain(mirror_top_bs, backing_bs(mirror_top_bs)); + bdrv_child_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL); + bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); /* 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. */ + * bdrv_replace_node() 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); @@ -1189,10 +1200,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs, s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp); if (!s->dirty_bitmap) { - g_free(s->replaces); - blk_unref(s->target); - block_job_unref(&s->common); - return; + goto fail; } /* Required permissions are already taken with blk_new() */ @@ -1228,7 +1236,8 @@ fail: block_job_unref(&s->common); } - bdrv_replace_in_backing_chain(mirror_top_bs, backing_bs(mirror_top_bs)); + bdrv_child_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL); + bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); } void mirror_start(const char *job_id, BlockDriverState *bs, diff --git a/block/sheepdog.c b/block/sheepdog.c index 743471043e..89e98edab6 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -14,6 +14,8 @@ #include "qemu/osdep.h" #include "qapi/error.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp/qint.h" #include "qemu/uri.h" #include "qemu/error-report.h" #include "qemu/sockets.h" @@ -374,8 +376,7 @@ struct BDRVSheepdogState { uint32_t cache_flags; bool discard_supported; - char *host_spec; - bool is_unix; + SocketAddress *addr; int fd; CoMutex lock; @@ -527,21 +528,36 @@ static void sd_aio_setup(SheepdogAIOCB *acb, BDRVSheepdogState *s, QLIST_INSERT_HEAD(&s->inflight_aiocb_head, acb, aiocb_siblings); } +static SocketAddress *sd_socket_address(const char *path, + const char *host, const char *port) +{ + SocketAddress *addr = g_new0(SocketAddress, 1); + + if (path) { + addr->type = SOCKET_ADDRESS_KIND_UNIX; + addr->u.q_unix.data = g_new0(UnixSocketAddress, 1); + addr->u.q_unix.data->path = g_strdup(path); + } else { + addr->type = SOCKET_ADDRESS_KIND_INET; + addr->u.inet.data = g_new0(InetSocketAddress, 1); + addr->u.inet.data->host = g_strdup(host ?: SD_DEFAULT_ADDR); + addr->u.inet.data->port = g_strdup(port ?: stringify(SD_DEFAULT_PORT)); + } + + return addr; +} + /* Return -EIO in case of error, file descriptor on success */ static int connect_to_sdog(BDRVSheepdogState *s, Error **errp) { int fd; - if (s->is_unix) { - fd = unix_connect(s->host_spec, errp); - } else { - fd = inet_connect(s->host_spec, errp); + fd = socket_connect(s->addr, errp, NULL, NULL); - if (fd >= 0) { - int ret = socket_set_nodelay(fd); - if (ret < 0) { - error_report("%s", strerror(errno)); - } + if (s->addr->type == SOCKET_ADDRESS_KIND_INET && fd >= 0) { + int ret = socket_set_nodelay(fd); + if (ret < 0) { + error_report("%s", strerror(errno)); } } @@ -820,8 +836,7 @@ static void coroutine_fn aio_read_response(void *opaque) case AIOCB_DISCARD_OBJ: switch (rsp.result) { case SD_RES_INVALID_PARMS: - error_report("sheep(%s) doesn't support discard command", - s->host_spec); + error_report("server doesn't support discard command"); rsp.result = SD_RES_SUCCESS; s->discard_supported = false; break; @@ -914,71 +929,155 @@ static int get_sheep_fd(BDRVSheepdogState *s, Error **errp) return fd; } -static int sd_parse_uri(BDRVSheepdogState *s, const char *filename, - char *vdi, uint32_t *snapid, char *tag) +/* + * Parse numeric snapshot ID in @str + * If @str can't be parsed as number, return false. + * Else, if the number is zero or too large, set *@snapid to zero and + * return true. + * Else, set *@snapid to the number and return true. + */ +static bool sd_parse_snapid(const char *str, uint32_t *snapid) +{ + unsigned long ul; + int ret; + + ret = qemu_strtoul(str, NULL, 10, &ul); + if (ret == -ERANGE) { + ul = ret = 0; + } + if (ret) { + return false; + } + if (ul > UINT32_MAX) { + ul = 0; + } + + *snapid = ul; + return true; +} + +static bool sd_parse_snapid_or_tag(const char *str, + uint32_t *snapid, char tag[]) { + if (!sd_parse_snapid(str, snapid)) { + *snapid = 0; + if (g_strlcpy(tag, str, SD_MAX_VDI_TAG_LEN) >= SD_MAX_VDI_TAG_LEN) { + return false; + } + } else if (!*snapid) { + return false; + } else { + tag[0] = 0; + } + return true; +} + +typedef struct { + const char *path; /* non-null iff transport is tcp */ + const char *host; /* valid when transport is tcp */ + int port; /* valid when transport is tcp */ + char vdi[SD_MAX_VDI_LEN]; + char tag[SD_MAX_VDI_TAG_LEN]; + uint32_t snap_id; + /* Remainder is only for sd_config_done() */ URI *uri; + QueryParams *qp; +} SheepdogConfig; + +static void sd_config_done(SheepdogConfig *cfg) +{ + if (cfg->qp) { + query_params_free(cfg->qp); + } + uri_free(cfg->uri); +} + +static void sd_parse_uri(SheepdogConfig *cfg, const char *filename, + Error **errp) +{ + Error *err = NULL; QueryParams *qp = NULL; - int ret = 0; + bool is_unix; + URI *uri; + + memset(cfg, 0, sizeof(*cfg)); - uri = uri_parse(filename); + cfg->uri = uri = uri_parse(filename); if (!uri) { - return -EINVAL; + error_setg(&err, "invalid URI"); + goto out; } /* transport */ if (!strcmp(uri->scheme, "sheepdog")) { - s->is_unix = false; + is_unix = false; } else if (!strcmp(uri->scheme, "sheepdog+tcp")) { - s->is_unix = false; + is_unix = false; } else if (!strcmp(uri->scheme, "sheepdog+unix")) { - s->is_unix = true; + is_unix = true; } else { - ret = -EINVAL; + error_setg(&err, "URI scheme must be 'sheepdog', 'sheepdog+tcp'," + " or 'sheepdog+unix'"); goto out; } if (uri->path == NULL || !strcmp(uri->path, "/")) { - ret = -EINVAL; + error_setg(&err, "missing file path in URI"); goto out; } - pstrcpy(vdi, SD_MAX_VDI_LEN, uri->path + 1); - - qp = query_params_parse(uri->query); - if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) { - ret = -EINVAL; + if (g_strlcpy(cfg->vdi, uri->path + 1, SD_MAX_VDI_LEN) + >= SD_MAX_VDI_LEN) { + error_setg(&err, "VDI name is too long"); goto out; } - if (s->is_unix) { + cfg->qp = qp = query_params_parse(uri->query); + + if (is_unix) { /* sheepdog+unix:///vdiname?socket=path */ - if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) { - ret = -EINVAL; + if (uri->server || uri->port) { + error_setg(&err, "URI scheme %s doesn't accept a server address", + uri->scheme); + goto out; + } + if (!qp->n) { + error_setg(&err, + "URI scheme %s requires query parameter 'socket'", + uri->scheme); + goto out; + } + if (qp->n != 1 || strcmp(qp->p[0].name, "socket")) { + error_setg(&err, "unexpected query parameters"); goto out; } - s->host_spec = g_strdup(qp->p[0].value); + cfg->path = qp->p[0].value; } else { /* sheepdog[+tcp]://[host:port]/vdiname */ - s->host_spec = g_strdup_printf("%s:%d", uri->server ?: SD_DEFAULT_ADDR, - uri->port ?: SD_DEFAULT_PORT); + if (qp->n) { + error_setg(&err, "unexpected query parameters"); + goto out; + } + cfg->host = uri->server; + cfg->port = uri->port; } /* snapshot tag */ if (uri->fragment) { - *snapid = strtoul(uri->fragment, NULL, 10); - if (*snapid == 0) { - pstrcpy(tag, SD_MAX_VDI_TAG_LEN, uri->fragment); + if (!sd_parse_snapid_or_tag(uri->fragment, + &cfg->snap_id, cfg->tag)) { + error_setg(&err, "'%s' is not a valid snapshot ID", + uri->fragment); + goto out; } } else { - *snapid = CURRENT_VDI_ID; /* search current vdi */ + cfg->snap_id = CURRENT_VDI_ID; /* search current vdi */ } out: - if (qp) { - query_params_free(qp); + if (err) { + error_propagate(errp, err); + sd_config_done(cfg); } - uri_free(uri); - return ret; } /* @@ -998,12 +1097,13 @@ out: * You can run VMs outside the Sheepdog cluster by specifying * `hostname' and `port' (experimental). */ -static int parse_vdiname(BDRVSheepdogState *s, const char *filename, - char *vdi, uint32_t *snapid, char *tag) +static void parse_vdiname(SheepdogConfig *cfg, const char *filename, + Error **errp) { + Error *err = NULL; char *p, *q, *uri; const char *host_spec, *vdi_spec; - int nr_sep, ret; + int nr_sep; strstart(filename, "sheepdog:", &filename); p = q = g_strdup(filename); @@ -1038,12 +1138,60 @@ static int parse_vdiname(BDRVSheepdogState *s, const char *filename, uri = g_strdup_printf("sheepdog://%s/%s", host_spec, vdi_spec); - ret = sd_parse_uri(s, uri, vdi, snapid, tag); + /* + * FIXME We to escape URI meta-characters, e.g. "x?y=z" + * produces "sheepdog://x?y=z". Because of that ... + */ + sd_parse_uri(cfg, uri, &err); + if (err) { + /* + * ... this can fail, but the error message is misleading. + * Replace it by the traditional useless one until the + * escaping is fixed. + */ + error_free(err); + error_setg(errp, "Can't parse filename"); + } g_free(q); g_free(uri); +} - return ret; +static void sd_parse_filename(const char *filename, QDict *options, + Error **errp) +{ + Error *err = NULL; + SheepdogConfig cfg; + char buf[32]; + + if (strstr(filename, "://")) { + sd_parse_uri(&cfg, filename, &err); + } else { + parse_vdiname(&cfg, filename, &err); + } + if (err) { + error_propagate(errp, err); + return; + } + + if (cfg.host) { + qdict_set_default_str(options, "host", cfg.host); + } + if (cfg.port) { + snprintf(buf, sizeof(buf), "%d", cfg.port); + qdict_set_default_str(options, "port", buf); + } + if (cfg.path) { + qdict_set_default_str(options, "path", cfg.path); + } + qdict_set_default_str(options, "vdi", cfg.vdi); + qdict_set_default_str(options, "tag", cfg.tag); + if (cfg.snap_id) { + snprintf(buf, sizeof(buf), "%d", cfg.snap_id); + qdict_set_default_str(options, "snap-id", buf); + } + + sd_config_done(&cfg); } static int find_vdi_name(BDRVSheepdogState *s, const char *filename, @@ -1357,15 +1505,33 @@ static void sd_attach_aio_context(BlockDriverState *bs, co_read_response, NULL, NULL, s); } -/* TODO Convert to fine grained options */ static QemuOptsList runtime_opts = { .name = "sheepdog", .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), .desc = { { - .name = "filename", + .name = "host", + .type = QEMU_OPT_STRING, + }, + { + .name = "port", + .type = QEMU_OPT_STRING, + }, + { + .name = "path", + .type = QEMU_OPT_STRING, + }, + { + .name = "vdi", + .type = QEMU_OPT_STRING, + }, + { + .name = "snap-id", + .type = QEMU_OPT_NUMBER, + }, + { + .name = "tag", .type = QEMU_OPT_STRING, - .help = "URL to the sheepdog image", }, { /* end of list */ } }, @@ -1377,12 +1543,11 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, int ret, fd; uint32_t vid = 0; BDRVSheepdogState *s = bs->opaque; - char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN]; - uint32_t snapid; + const char *host, *port, *path, *vdi, *snap_id_str, *tag; + uint64_t snap_id; char *buf = NULL; QemuOpts *opts; Error *local_err = NULL; - const char *filename; s->bs = bs; s->aio_context = bdrv_get_aio_context(bs); @@ -1392,37 +1557,68 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, if (local_err) { error_propagate(errp, local_err); ret = -EINVAL; - goto out; + goto err_no_fd; } - filename = qemu_opt_get(opts, "filename"); + host = qemu_opt_get(opts, "host"); + port = qemu_opt_get(opts, "port"); + path = qemu_opt_get(opts, "path"); + vdi = qemu_opt_get(opts, "vdi"); + snap_id_str = qemu_opt_get(opts, "snap-id"); + snap_id = qemu_opt_get_number(opts, "snap-id", CURRENT_VDI_ID); + tag = qemu_opt_get(opts, "tag"); - QLIST_INIT(&s->inflight_aio_head); - QLIST_INIT(&s->failed_aio_head); - QLIST_INIT(&s->inflight_aiocb_head); - s->fd = -1; + if ((host || port) && path) { + error_setg(errp, "can't use 'path' together with 'host' or 'port'"); + ret = -EINVAL; + goto err_no_fd; + } - memset(vdi, 0, sizeof(vdi)); - memset(tag, 0, sizeof(tag)); + if (!vdi) { + error_setg(errp, "parameter 'vdi' is missing"); + ret = -EINVAL; + goto err_no_fd; + } + if (strlen(vdi) >= SD_MAX_VDI_LEN) { + error_setg(errp, "value of parameter 'vdi' is too long"); + ret = -EINVAL; + goto err_no_fd; + } - if (strstr(filename, "://")) { - ret = sd_parse_uri(s, filename, vdi, &snapid, tag); - } else { - ret = parse_vdiname(s, filename, vdi, &snapid, tag); + if (snap_id > UINT32_MAX) { + snap_id = 0; } - if (ret < 0) { - error_setg(errp, "Can't parse filename"); - goto out; + if (snap_id_str && !snap_id) { + error_setg(errp, "'snap-id=%s' is not a valid snapshot ID", + snap_id_str); + ret = -EINVAL; + goto err_no_fd; + } + + if (!tag) { + tag = ""; } + if (tag && strlen(tag) >= SD_MAX_VDI_TAG_LEN) { + error_setg(errp, "value of parameter 'tag' is too long"); + ret = -EINVAL; + goto err_no_fd; + } + + s->addr = sd_socket_address(path, host, port); + + QLIST_INIT(&s->inflight_aio_head); + QLIST_INIT(&s->failed_aio_head); + QLIST_INIT(&s->inflight_aiocb_head); + s->fd = get_sheep_fd(s, errp); if (s->fd < 0) { ret = s->fd; - goto out; + goto err_no_fd; } - ret = find_vdi_name(s, vdi, snapid, tag, &vid, true, errp); + ret = find_vdi_name(s, vdi, (uint32_t)snap_id, tag, &vid, true, errp); if (ret) { - goto out; + goto err; } /* @@ -1435,7 +1631,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, } s->discard_supported = true; - if (snapid || tag[0] != '\0') { + if (snap_id || tag[0]) { DPRINTF("%" PRIx32 " snapshot inode was open.\n", vid); s->is_snapshot = true; } @@ -1443,7 +1639,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, fd = connect_to_sdog(s, errp); if (fd < 0) { ret = fd; - goto out; + goto err; } buf = g_malloc(SD_INODE_SIZE); @@ -1454,7 +1650,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, if (ret) { error_setg(errp, "Can't read snapshot inode"); - goto out; + goto err; } memcpy(&s->inode, buf, sizeof(s->inode)); @@ -1466,12 +1662,12 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, qemu_opts_del(opts); g_free(buf); return 0; -out: + +err: aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd, false, NULL, NULL, NULL, NULL); - if (s->fd >= 0) { - closesocket(s->fd); - } + closesocket(s->fd); +err_no_fd: qemu_opts_del(opts); g_free(buf); return ret; @@ -1685,6 +1881,7 @@ static int parse_redundancy(BDRVSheepdogState *s, const char *opt) } copy = strtol(n1, NULL, 10); + /* FIXME fix error checking by switching to qemu_strtol() */ if (copy > SD_MAX_COPIES || copy < 1) { return -EINVAL; } @@ -1699,6 +1896,7 @@ static int parse_redundancy(BDRVSheepdogState *s, const char *opt) } parity = strtol(n2, NULL, 10); + /* FIXME fix error checking by switching to qemu_strtol() */ if (parity >= SD_EC_MAX_STRIP || parity < 1) { return -EINVAL; } @@ -1737,29 +1935,34 @@ static int parse_block_size_shift(BDRVSheepdogState *s, QemuOpts *opt) static int sd_create(const char *filename, QemuOpts *opts, Error **errp) { + Error *err = NULL; int ret = 0; uint32_t vid = 0; char *backing_file = NULL; char *buf = NULL; BDRVSheepdogState *s; - char tag[SD_MAX_VDI_TAG_LEN]; - uint32_t snapid; + SheepdogConfig cfg; uint64_t max_vdi_size; bool prealloc = false; s = g_new0(BDRVSheepdogState, 1); - memset(tag, 0, sizeof(tag)); if (strstr(filename, "://")) { - ret = sd_parse_uri(s, filename, s->name, &snapid, tag); + sd_parse_uri(&cfg, filename, &err); } else { - ret = parse_vdiname(s, filename, s->name, &snapid, tag); + parse_vdiname(&cfg, filename, &err); } - if (ret < 0) { - error_setg(errp, "Can't parse filename"); + if (err) { + error_propagate(errp, err); goto out; } + buf = cfg.port ? g_strdup_printf("%d", cfg.port) : NULL; + s->addr = sd_socket_address(cfg.path, cfg.host, buf); + g_free(buf); + strcpy(s->name, cfg.vdi); + sd_config_done(&cfg); + s->inode.vdi_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0), BDRV_SECTOR_SIZE); backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE); @@ -1829,14 +2032,12 @@ static int sd_create(const char *filename, QemuOpts *opts, if (s->inode.block_size_shift == 0) { SheepdogVdiReq hdr; SheepdogClusterRsp *rsp = (SheepdogClusterRsp *)&hdr; - Error *local_err = NULL; int fd; unsigned int wlen = 0, rlen = 0; - fd = connect_to_sdog(s, &local_err); + fd = connect_to_sdog(s, errp); if (fd < 0) { - error_report_err(local_err); - ret = -EIO; + ret = fd; goto out; } @@ -1922,7 +2123,7 @@ static void sd_close(BlockDriverState *bs) aio_set_fd_handler(bdrv_get_aio_context(bs), s->fd, false, NULL, NULL, NULL, NULL); closesocket(s->fd); - g_free(s->host_spec); + qapi_free_SocketAddress(s->addr); } static int64_t sd_getlength(BlockDriverState *bs) @@ -2367,19 +2568,16 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) BDRVSheepdogState *old_s; char tag[SD_MAX_VDI_TAG_LEN]; uint32_t snapid = 0; - int ret = 0; + int ret; + + if (!sd_parse_snapid_or_tag(snapshot_id, &snapid, tag)) { + return -EINVAL; + } old_s = g_new(BDRVSheepdogState, 1); memcpy(old_s, s, sizeof(BDRVSheepdogState)); - snapid = strtoul(snapshot_id, NULL, 10); - if (snapid) { - tag[0] = 0; - } else { - pstrcpy(tag, sizeof(tag), snapshot_id); - } - ret = reload_inode(s, snapid, tag); if (ret) { goto out; @@ -2405,18 +2603,15 @@ out: #define NR_BATCHED_DISCARD 128 -static bool remove_objects(BDRVSheepdogState *s) +static int remove_objects(BDRVSheepdogState *s, Error **errp) { int fd, i = 0, nr_objs = 0; - Error *local_err = NULL; - int ret = 0; - bool result = true; + int ret; SheepdogInode *inode = &s->inode; - fd = connect_to_sdog(s, &local_err); + fd = connect_to_sdog(s, errp); if (fd < 0) { - error_report_err(local_err); - return false; + return fd; } nr_objs = count_data_objs(inode); @@ -2446,15 +2641,15 @@ static bool remove_objects(BDRVSheepdogState *s) data_vdi_id[start_idx]), false, s->cache_flags); if (ret < 0) { - error_report("failed to discard snapshot inode."); - result = false; + error_setg(errp, "Failed to discard snapshot inode"); goto out; } } + ret = 0; out: closesocket(fd); - return result; + return ret; } static int sd_snapshot_delete(BlockDriverState *bs, @@ -2462,9 +2657,12 @@ static int sd_snapshot_delete(BlockDriverState *bs, const char *name, Error **errp) { + /* + * FIXME should delete the snapshot matching both @snapshot_id and + * @name, but @name not used here + */ unsigned long snap_id = 0; char snap_tag[SD_MAX_VDI_TAG_LEN]; - Error *local_err = NULL; int fd, ret; char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN]; BDRVSheepdogState *s = bs->opaque; @@ -2477,15 +2675,22 @@ static int sd_snapshot_delete(BlockDriverState *bs, }; SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr; - if (!remove_objects(s)) { - return -1; + ret = remove_objects(s, errp); + if (ret) { + return ret; } memset(buf, 0, sizeof(buf)); memset(snap_tag, 0, sizeof(snap_tag)); pstrcpy(buf, SD_MAX_VDI_LEN, s->name); + /* TODO Use sd_parse_snapid() once this mess is cleaned up */ ret = qemu_strtoul(snapshot_id, NULL, 10, &snap_id); if (ret || snap_id > UINT32_MAX) { + /* + * FIXME Since qemu_strtoul() returns -EINVAL when + * @snapshot_id is null, @snapshot_id is mandatory. Correct + * would be to require at least one of @snapshot_id and @name. + */ error_setg(errp, "Invalid snapshot ID: %s", snapshot_id ? snapshot_id : "<null>"); return -EINVAL; @@ -2494,40 +2699,42 @@ static int sd_snapshot_delete(BlockDriverState *bs, if (snap_id) { hdr.snapid = (uint32_t) snap_id; } else { + /* FIXME I suspect we should use @name here */ + /* FIXME don't truncate silently */ pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id); pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag); } - ret = find_vdi_name(s, s->name, snap_id, snap_tag, &vid, true, - &local_err); + ret = find_vdi_name(s, s->name, snap_id, snap_tag, &vid, true, errp); if (ret) { return ret; } - fd = connect_to_sdog(s, &local_err); + fd = connect_to_sdog(s, errp); if (fd < 0) { - error_report_err(local_err); - return -1; + return fd; } ret = do_req(fd, s->bs, (SheepdogReq *)&hdr, buf, &wlen, &rlen); closesocket(fd); if (ret) { + error_setg_errno(errp, -ret, "Couldn't send request to server"); return ret; } switch (rsp->result) { case SD_RES_NO_VDI: - error_report("%s was already deleted", s->name); + error_setg(errp, "Can't find the snapshot"); + return -ENOENT; case SD_RES_SUCCESS: break; default: - error_report("%s, %s", sd_strerror(rsp->result), s->name); - return -1; + error_setg(errp, "%s", sd_strerror(rsp->result)); + return -EIO; } - return ret; + return 0; } static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) @@ -2832,7 +3039,7 @@ static BlockDriver bdrv_sheepdog = { .format_name = "sheepdog", .protocol_name = "sheepdog", .instance_size = sizeof(BDRVSheepdogState), - .bdrv_needs_filename = true, + .bdrv_parse_filename = sd_parse_filename, .bdrv_file_open = sd_open, .bdrv_reopen_prepare = sd_reopen_prepare, .bdrv_reopen_commit = sd_reopen_commit, @@ -2868,7 +3075,7 @@ static BlockDriver bdrv_sheepdog_tcp = { .format_name = "sheepdog", .protocol_name = "sheepdog+tcp", .instance_size = sizeof(BDRVSheepdogState), - .bdrv_needs_filename = true, + .bdrv_parse_filename = sd_parse_filename, .bdrv_file_open = sd_open, .bdrv_reopen_prepare = sd_reopen_prepare, .bdrv_reopen_commit = sd_reopen_commit, @@ -2904,7 +3111,7 @@ static BlockDriver bdrv_sheepdog_unix = { .format_name = "sheepdog", .protocol_name = "sheepdog+unix", .instance_size = sizeof(BDRVSheepdogState), - .bdrv_needs_filename = true, + .bdrv_parse_filename = sd_parse_filename, .bdrv_file_open = sd_open, .bdrv_reopen_prepare = sd_reopen_prepare, .bdrv_reopen_commit = sd_reopen_commit, diff --git a/blockdev.c b/blockdev.c index 8eb4e84fe0..f1f49bd3ca 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1614,6 +1614,7 @@ typedef struct ExternalSnapshotState { BlockDriverState *old_bs; BlockDriverState *new_bs; AioContext *aio_context; + bool overlay_appended; } ExternalSnapshotState; static void external_snapshot_prepare(BlkActionState *common, @@ -1780,6 +1781,7 @@ static void external_snapshot_prepare(BlkActionState *common, error_propagate(errp, local_err); return; } + state->overlay_appended = true; } static void external_snapshot_commit(BlkActionState *common) @@ -1803,8 +1805,8 @@ static void external_snapshot_abort(BlkActionState *common) ExternalSnapshotState *state = DO_UPCAST(ExternalSnapshotState, common, common); if (state->new_bs) { - if (state->new_bs->backing) { - bdrv_replace_in_backing_chain(state->new_bs, state->old_bs); + if (state->overlay_appended) { + bdrv_replace_node(state->new_bs, state->old_bs, &error_abort); } } } diff --git a/include/block/block.h b/include/block/block.h index c7c4a3ac3a..5149260827 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -238,8 +238,8 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp); BlockDriverState *bdrv_new(void); void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top, Error **errp); -void bdrv_replace_in_backing_chain(BlockDriverState *old, - BlockDriverState *new); +void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, + Error **errp); int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough); int bdrv_parse_discard_flags(const char *mode, int *flags); diff --git a/include/block/block_int.h b/include/block/block_int.h index a57c0bfb55..6c699ac9c3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -441,8 +441,8 @@ typedef struct BdrvAioNotifier { } BdrvAioNotifier; struct BdrvChildRole { - /* If true, bdrv_replace_in_backing_chain() doesn't change the node this - * BdrvChild points to. */ + /* If true, bdrv_replace_node() doesn't change the node this BdrvChild + * points to. */ bool stay_at_node; void (*inherit_options)(int *child_flags, QDict *child_options, @@ -890,7 +890,7 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, void bdrv_root_unref_child(BdrvChild *child); int bdrv_child_check_perm(BdrvChild *c, uint64_t perm, uint64_t shared, - Error **errp); + GSList *ignore_children, 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, diff --git a/qapi-schema.json b/qapi-schema.json index 6febfa7b90..32b4a4b782 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4101,6 +4101,44 @@ 'fd': 'String' } } ## +# @SocketAddressFlatType: +# +# Available SocketAddressFlat types +# +# @inet: Internet address +# +# @unix: Unix domain socket +# +# Since: 2.9 +## +{ 'enum': 'SocketAddressFlatType', + 'data': [ 'unix', 'inet' ] } + +## +# @SocketAddressFlat: +# +# Captures the address of a socket +# +# @type: Transport type +# +# This is similar to SocketAddress, only distinction: +# +# 1. SocketAddressFlat is a flat union, SocketAddress is a simple union. +# A flat union is nicer than simple because it avoids nesting +# (i.e. more {}) on the wire. +# +# 2. SocketAddressFlat supports only types 'unix' and 'inet', because +# that's what its current users need. +# +# Since: 2.9 +## +{ 'union': 'SocketAddressFlat', + 'base': { 'type': 'SocketAddressFlatType' }, + 'discriminator': 'type', + 'data': { 'unix': 'UnixSocketAddress', + 'inet': 'InetSocketAddress' } } + +## # @getfd: # # Receive a file descriptor via SCM rights and assign it a name diff --git a/qapi/block-core.json b/qapi/block-core.json index bc0ccd615c..9bb7f4a17b 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2124,6 +2124,7 @@ # @ssh: Since 2.8 # @iscsi: Since 2.9 # @rbd: Since 2.9 +# @sheepdog: Since 2.9 # # Since: 2.0 ## @@ -2132,8 +2133,8 @@ 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', - 'quorum', 'raw', 'rbd', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', - 'vpc', 'vvfat' ] } + 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', + 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } ## # @BlockdevOptionsFile: @@ -2546,50 +2547,6 @@ '*read-pattern': 'QuorumReadPattern' } } ## -# @GlusterTransport: -# -# An enumeration of Gluster transport types -# -# @tcp: TCP - Transmission Control Protocol -# -# @unix: UNIX - Unix domain socket -# -# Since: 2.7 -## -{ 'enum': 'GlusterTransport', - 'data': [ 'unix', 'tcp' ] } - - -## -# @GlusterServer: -# -# Captures the address of a socket -# -# Details for connecting to a gluster server -# -# @type: Transport type used for gluster connection -# -# This is similar to SocketAddress, only distinction: -# -# 1. GlusterServer is a flat union, SocketAddress is a simple union. -# A flat union is nicer than simple because it avoids nesting -# (i.e. more {}) on the wire. -# -# 2. GlusterServer lacks case 'fd', since gluster doesn't let you -# pass in a file descriptor. -# -# GlusterServer is actually not Gluster-specific, its a -# compatibility evolved into an alternate for SocketAddress. -# -# Since: 2.7 -## -{ 'union': 'GlusterServer', - 'base': { 'type': 'GlusterTransport' }, - 'discriminator': 'type', - 'data': { 'unix': 'UnixSocketAddress', - 'tcp': 'InetSocketAddress' } } - -## # @BlockdevOptionsGluster: # # Driver specific block device options for Gluster @@ -2610,7 +2567,7 @@ { 'struct': 'BlockdevOptionsGluster', 'data': { 'volume': 'str', 'path': 'str', - 'server': ['GlusterServer'], + 'server': ['SocketAddressFlat'], '*debug': 'int', '*logfile': 'str' } } @@ -2736,6 +2693,26 @@ '*password-secret': 'str' } } ## +# @BlockdevOptionsSheepdog: +# +# Driver specific block device options for sheepdog +# +# @vdi: Virtual disk image name +# @addr: The Sheepdog server to connect to +# @snap-id: Snapshot ID +# @tag: Snapshot tag name +# +# Only one of @snap-id and @tag may be present. +# +# Since: 2.9 +## +{ 'struct': 'BlockdevOptionsSheepdog', + 'data': { 'addr': 'SocketAddressFlat', + 'vdi': 'str', + '*snap-id': 'uint32', + '*tag': 'str' } } + +## # @ReplicationMode: # # An enumeration of replication modes. @@ -2935,7 +2912,7 @@ 'raw': 'BlockdevOptionsRaw', 'rbd': 'BlockdevOptionsRbd', 'replication':'BlockdevOptionsReplication', -# TODO sheepdog: Wait for structured options + 'sheepdog': 'BlockdevOptionsSheepdog', 'ssh': 'BlockdevOptionsSsh', 'vdi': 'BlockdevOptionsGenericFormat', 'vhdx': 'BlockdevOptionsGenericFormat', |