diff options
| -rw-r--r-- | block/nbd.c | 71 | ||||
| -rw-r--r-- | nbd/server.c | 217 | ||||
| -rw-r--r-- | qemu-nbd.c | 15 |
3 files changed, 155 insertions, 148 deletions
diff --git a/block/nbd.c b/block/nbd.c index 9daf003bea..4548046cd7 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -122,6 +122,8 @@ typedef struct BDRVNBDState { Error *connect_err; bool wait_in_flight; + QEMUTimer *reconnect_delay_timer; + NBDClientRequest requests[MAX_NBD_REQUESTS]; NBDReply reply; BlockDriverState *bs; @@ -188,10 +190,49 @@ static void nbd_recv_coroutines_wake_all(BDRVNBDState *s) } } +static void reconnect_delay_timer_del(BDRVNBDState *s) +{ + if (s->reconnect_delay_timer) { + timer_del(s->reconnect_delay_timer); + timer_free(s->reconnect_delay_timer); + s->reconnect_delay_timer = NULL; + } +} + +static void reconnect_delay_timer_cb(void *opaque) +{ + BDRVNBDState *s = opaque; + + if (s->state == NBD_CLIENT_CONNECTING_WAIT) { + s->state = NBD_CLIENT_CONNECTING_NOWAIT; + while (qemu_co_enter_next(&s->free_sema, NULL)) { + /* Resume all queued requests */ + } + } + + reconnect_delay_timer_del(s); +} + +static void reconnect_delay_timer_init(BDRVNBDState *s, uint64_t expire_time_ns) +{ + if (s->state != NBD_CLIENT_CONNECTING_WAIT) { + return; + } + + assert(!s->reconnect_delay_timer); + s->reconnect_delay_timer = aio_timer_new(bdrv_get_aio_context(s->bs), + QEMU_CLOCK_REALTIME, + SCALE_NS, + reconnect_delay_timer_cb, s); + timer_mod(s->reconnect_delay_timer, expire_time_ns); +} + static void nbd_client_detach_aio_context(BlockDriverState *bs) { BDRVNBDState *s = (BDRVNBDState *)bs->opaque; + /* Timer is deleted in nbd_client_co_drain_begin() */ + assert(!s->reconnect_delay_timer); qio_channel_detach_aio_context(QIO_CHANNEL(s->ioc)); } @@ -242,6 +283,13 @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) } nbd_co_establish_connection_cancel(bs, false); + + reconnect_delay_timer_del(s); + + if (s->state == NBD_CLIENT_CONNECTING_WAIT) { + s->state = NBD_CLIENT_CONNECTING_NOWAIT; + qemu_co_queue_restart_all(&s->free_sema); + } } static void coroutine_fn nbd_client_co_drain_end(BlockDriverState *bs) @@ -544,7 +592,7 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s) /* Finalize previous connection if any */ if (s->ioc) { - nbd_client_detach_aio_context(s->bs); + qio_channel_detach_aio_context(QIO_CHANNEL(s->ioc)); object_unref(OBJECT(s->sioc)); s->sioc = NULL; object_unref(OBJECT(s->ioc)); @@ -588,21 +636,17 @@ out: static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s) { - uint64_t start_time_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME); - uint64_t delay_ns = s->reconnect_delay * NANOSECONDS_PER_SECOND; uint64_t timeout = 1 * NANOSECONDS_PER_SECOND; uint64_t max_timeout = 16 * NANOSECONDS_PER_SECOND; + if (s->state == NBD_CLIENT_CONNECTING_WAIT) { + reconnect_delay_timer_init(s, qemu_clock_get_ns(QEMU_CLOCK_REALTIME) + + s->reconnect_delay * NANOSECONDS_PER_SECOND); + } + nbd_reconnect_attempt(s); while (nbd_client_connecting(s)) { - if (s->state == NBD_CLIENT_CONNECTING_WAIT && - qemu_clock_get_ns(QEMU_CLOCK_REALTIME) - start_time_ns > delay_ns) - { - s->state = NBD_CLIENT_CONNECTING_NOWAIT; - qemu_co_queue_restart_all(&s->free_sema); - } - if (s->drained) { bdrv_dec_in_flight(s->bs); s->wait_drained_end = true; @@ -617,6 +661,9 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s) } else { qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout, &s->connection_co_sleep_ns_state); + if (s->drained) { + continue; + } if (timeout < max_timeout) { timeout *= 2; } @@ -624,6 +671,8 @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s) nbd_reconnect_attempt(s); } + + reconnect_delay_timer_del(s); } static coroutine_fn void nbd_connection_entry(void *opaque) @@ -702,7 +751,7 @@ static coroutine_fn void nbd_connection_entry(void *opaque) s->connection_co = NULL; if (s->ioc) { - nbd_client_detach_aio_context(s->bs); + qio_channel_detach_aio_context(QIO_CHANNEL(s->ioc)); object_unref(OBJECT(s->sioc)); s->sioc = NULL; object_unref(OBJECT(s->ioc)); diff --git a/nbd/server.c b/nbd/server.c index f74766add7..e75c825879 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016-2018 Red Hat, Inc. + * Copyright (C) 2016-2020 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <anthony@codemonkey.ws> * * Network Block Device Server Side @@ -301,10 +301,11 @@ nbd_opt_invalid(NBDClient *client, Error **errp, const char *fmt, ...) } /* Read size bytes from the unparsed payload of the current option. + * If @check_nul, require that no NUL bytes appear in buffer. * Return -errno on I/O error, 0 if option was completely handled by * sending a reply about inconsistent lengths, or 1 on success. */ static int nbd_opt_read(NBDClient *client, void *buffer, size_t size, - Error **errp) + bool check_nul, Error **errp) { if (size > client->optlen) { return nbd_opt_invalid(client, errp, @@ -312,7 +313,16 @@ static int nbd_opt_read(NBDClient *client, void *buffer, size_t size, nbd_opt_lookup(client->opt)); } client->optlen -= size; - return qio_channel_read_all(client->ioc, buffer, size, errp) < 0 ? -EIO : 1; + if (qio_channel_read_all(client->ioc, buffer, size, errp) < 0) { + return -EIO; + } + + if (check_nul && strnlen(buffer, size) != size) { + return nbd_opt_invalid(client, errp, + "Unexpected embedded NUL in option %s", + nbd_opt_lookup(client->opt)); + } + return 1; } /* Drop size bytes from the unparsed payload of the current option. @@ -349,7 +359,7 @@ static int nbd_opt_read_name(NBDClient *client, char **name, uint32_t *length, g_autofree char *local_name = NULL; *name = NULL; - ret = nbd_opt_read(client, &len, sizeof(len), errp); + ret = nbd_opt_read(client, &len, sizeof(len), false, errp); if (ret <= 0) { return ret; } @@ -361,7 +371,7 @@ static int nbd_opt_read_name(NBDClient *client, char **name, uint32_t *length, } local_name = g_malloc(len + 1); - ret = nbd_opt_read(client, local_name, len, errp); + ret = nbd_opt_read(client, local_name, len, true, errp); if (ret <= 0) { return ret; } @@ -556,7 +566,7 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp) NBDExport *exp; uint16_t requests; uint16_t request; - uint32_t namelen; + uint32_t namelen = 0; bool sendname = false; bool blocksize = false; uint32_t sizes[3]; @@ -576,14 +586,14 @@ static int nbd_negotiate_handle_info(NBDClient *client, Error **errp) } trace_nbd_negotiate_handle_export_name_request(name); - rc = nbd_opt_read(client, &requests, sizeof(requests), errp); + rc = nbd_opt_read(client, &requests, sizeof(requests), false, errp); if (rc <= 0) { return rc; } requests = be16_to_cpu(requests); trace_nbd_negotiate_handle_info_requests(requests); while (requests--) { - rc = nbd_opt_read(client, &request, sizeof(request), errp); + rc = nbd_opt_read(client, &request, sizeof(request), false, errp); if (rc <= 0) { return rc; } @@ -787,135 +797,95 @@ static int nbd_negotiate_send_meta_context(NBDClient *client, return qio_channel_writev_all(client->ioc, iov, 2, errp) < 0 ? -EIO : 0; } -/* Read strlen(@pattern) bytes, and set @match to true if they match @pattern. - * @match is never set to false. - * - * Return -errno on I/O error, 0 if option was completely handled by - * sending a reply about inconsistent lengths, or 1 on success. - * - * Note: return code = 1 doesn't mean that we've read exactly @pattern. - * It only means that there are no errors. +/* + * Return true if @query matches @pattern, or if @query is empty when + * the @client is performing _LIST_. */ -static int nbd_meta_pattern(NBDClient *client, const char *pattern, bool *match, - Error **errp) +static bool nbd_meta_empty_or_pattern(NBDClient *client, const char *pattern, + const char *query) { - int ret; - char *query; - size_t len = strlen(pattern); - - assert(len); - - query = g_malloc(len); - ret = nbd_opt_read(client, query, len, errp); - if (ret <= 0) { - g_free(query); - return ret; + if (!*query) { + trace_nbd_negotiate_meta_query_parse("empty"); + return client->opt == NBD_OPT_LIST_META_CONTEXT; } - - if (strncmp(query, pattern, len) == 0) { + if (strcmp(query, pattern) == 0) { trace_nbd_negotiate_meta_query_parse(pattern); - *match = true; - } else { - trace_nbd_negotiate_meta_query_skip("pattern not matched"); + return true; } - g_free(query); - - return 1; + trace_nbd_negotiate_meta_query_skip("pattern not matched"); + return false; } /* - * Read @len bytes, and set @match to true if they match @pattern, or if @len - * is 0 and the client is performing _LIST_. @match is never set to false. - * - * Return -errno on I/O error, 0 if option was completely handled by - * sending a reply about inconsistent lengths, or 1 on success. - * - * Note: return code = 1 doesn't mean that we've read exactly @pattern. - * It only means that there are no errors. + * Return true and adjust @str in place if it begins with @prefix. */ -static int nbd_meta_empty_or_pattern(NBDClient *client, const char *pattern, - uint32_t len, bool *match, Error **errp) +static bool nbd_strshift(const char **str, const char *prefix) { - if (len == 0) { - if (client->opt == NBD_OPT_LIST_META_CONTEXT) { - *match = true; - } - trace_nbd_negotiate_meta_query_parse("empty"); - return 1; - } + size_t len = strlen(prefix); - if (len != strlen(pattern)) { - trace_nbd_negotiate_meta_query_skip("different lengths"); - return nbd_opt_skip(client, len, errp); + if (strncmp(*str, prefix, len) == 0) { + *str += len; + return true; } - - return nbd_meta_pattern(client, pattern, match, errp); + return false; } /* nbd_meta_base_query * * Handle queries to 'base' namespace. For now, only the base:allocation - * context is available. 'len' is the amount of text remaining to be read from - * the current name, after the 'base:' portion has been stripped. - * - * Return -errno on I/O error, 0 if option was completely handled by - * sending a reply about inconsistent lengths, or 1 on success. + * context is available. Return true if @query has been handled. */ -static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta, - uint32_t len, Error **errp) +static bool nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta, + const char *query) { - return nbd_meta_empty_or_pattern(client, "allocation", len, - &meta->base_allocation, errp); + if (!nbd_strshift(&query, "base:")) { + return false; + } + trace_nbd_negotiate_meta_query_parse("base:"); + + if (nbd_meta_empty_or_pattern(client, "allocation", query)) { + meta->base_allocation = true; + } + return true; } -/* nbd_meta_bitmap_query +/* nbd_meta_qemu_query * - * Handle query to 'qemu:' namespace. - * @len is the amount of text remaining to be read from the current name, after - * the 'qemu:' portion has been stripped. - * - * Return -errno on I/O error, 0 if option was completely handled by - * sending a reply about inconsistent lengths, or 1 on success. */ -static int nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, - uint32_t len, Error **errp) + * Handle queries to 'qemu' namespace. For now, only the qemu:dirty-bitmap: + * context is available. Return true if @query has been handled. + */ +static bool nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, + const char *query) { - bool dirty_bitmap = false; - size_t dirty_bitmap_len = strlen("dirty-bitmap:"); - int ret; - - if (!meta->exp->export_bitmap) { - trace_nbd_negotiate_meta_query_skip("no dirty-bitmap exported"); - return nbd_opt_skip(client, len, errp); + if (!nbd_strshift(&query, "qemu:")) { + return false; } + trace_nbd_negotiate_meta_query_parse("qemu:"); - if (len == 0) { + if (!*query) { if (client->opt == NBD_OPT_LIST_META_CONTEXT) { - meta->bitmap = true; + meta->bitmap = !!meta->exp->export_bitmap; } trace_nbd_negotiate_meta_query_parse("empty"); - return 1; - } - - if (len < dirty_bitmap_len) { - trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:"); - return nbd_opt_skip(client, len, errp); + return true; } - len -= dirty_bitmap_len; - ret = nbd_meta_pattern(client, "dirty-bitmap:", &dirty_bitmap, errp); - if (ret <= 0) { - return ret; - } - if (!dirty_bitmap) { - trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:"); - return nbd_opt_skip(client, len, errp); + if (nbd_strshift(&query, "dirty-bitmap:")) { + trace_nbd_negotiate_meta_query_parse("dirty-bitmap:"); + if (!meta->exp->export_bitmap) { + trace_nbd_negotiate_meta_query_skip("no dirty-bitmap exported"); + return true; + } + if (nbd_meta_empty_or_pattern(client, + meta->exp->export_bitmap_context + + strlen("qemu:dirty-bitmap:"), query)) { + meta->bitmap = true; + } + return true; } - trace_nbd_negotiate_meta_query_parse("dirty-bitmap:"); - - return nbd_meta_empty_or_pattern( - client, meta->exp->export_bitmap_context + - strlen("qemu:dirty_bitmap:"), len, &meta->bitmap, errp); + trace_nbd_negotiate_meta_query_skip("not dirty-bitmap"); + return true; } /* nbd_negotiate_meta_query @@ -925,25 +895,16 @@ static int nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta, * * The only supported namespaces are 'base' and 'qemu'. * - * The function aims not wasting time and memory to read long unknown namespace - * names. - * * Return -errno on I/O error, 0 if option was completely handled by * sending a reply about inconsistent lengths, or 1 on success. */ static int nbd_negotiate_meta_query(NBDClient *client, NBDExportMetaContexts *meta, Error **errp) { - /* - * Both 'qemu' and 'base' namespaces have length = 5 including a - * colon. If another length namespace is later introduced, this - * should certainly be refactored. - */ int ret; - size_t ns_len = 5; - char ns[5]; + g_autofree char *query = NULL; uint32_t len; - ret = nbd_opt_read(client, &len, sizeof(len), errp); + ret = nbd_opt_read(client, &len, sizeof(len), false, errp); if (ret <= 0) { return ret; } @@ -953,27 +914,23 @@ static int nbd_negotiate_meta_query(NBDClient *client, trace_nbd_negotiate_meta_query_skip("length too long"); return nbd_opt_skip(client, len, errp); } - if (len < ns_len) { - trace_nbd_negotiate_meta_query_skip("length too short"); - return nbd_opt_skip(client, len, errp); - } - len -= ns_len; - ret = nbd_opt_read(client, ns, ns_len, errp); + query = g_malloc(len + 1); + ret = nbd_opt_read(client, query, len, true, errp); if (ret <= 0) { return ret; } + query[len] = '\0'; - if (!strncmp(ns, "base:", ns_len)) { - trace_nbd_negotiate_meta_query_parse("base:"); - return nbd_meta_base_query(client, meta, len, errp); - } else if (!strncmp(ns, "qemu:", ns_len)) { - trace_nbd_negotiate_meta_query_parse("qemu:"); - return nbd_meta_qemu_query(client, meta, len, errp); + if (nbd_meta_base_query(client, meta, query)) { + return 1; + } + if (nbd_meta_qemu_query(client, meta, query)) { + return 1; } trace_nbd_negotiate_meta_query_skip("unknown namespace"); - return nbd_opt_skip(client, len, errp); + return 1; } /* nbd_negotiate_meta_queries @@ -1016,7 +973,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client, "export '%s' not present", sane_name); } - ret = nbd_opt_read(client, &nb_queries, sizeof(nb_queries), errp); + ret = nbd_opt_read(client, &nb_queries, sizeof(nb_queries), false, errp); if (ret <= 0) { return ret; } diff --git a/qemu-nbd.c b/qemu-nbd.c index bacb69b089..bc644a0670 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -154,13 +154,13 @@ QEMU_COPYRIGHT "\n" , name); } -#if HAVE_NBD_DEVICE +#ifdef CONFIG_POSIX static void termsig_handler(int signum) { qatomic_cmpxchg(&state, RUNNING, TERMINATE); qemu_notify_event(); } -#endif /* HAVE_NBD_DEVICE */ +#endif /* CONFIG_POSIX */ static int qemu_nbd_client_list(SocketAddress *saddr, QCryptoTLSCreds *tls, const char *hostname) @@ -581,17 +581,18 @@ int main(int argc, char **argv) const char *pid_file_name = NULL; BlockExportOptions *export_opts; -#if HAVE_NBD_DEVICE - /* The client thread uses SIGTERM to interrupt the server. A signal - * handler ensures that "qemu-nbd -v -c" exits with a nice status code. +#ifdef CONFIG_POSIX + /* + * Exit gracefully on various signals, which includes SIGTERM used + * by 'qemu-nbd -v -c'. */ struct sigaction sa_sigterm; memset(&sa_sigterm, 0, sizeof(sa_sigterm)); sa_sigterm.sa_handler = termsig_handler; sigaction(SIGTERM, &sa_sigterm, NULL); -#endif /* HAVE_NBD_DEVICE */ + sigaction(SIGINT, &sa_sigterm, NULL); + sigaction(SIGHUP, &sa_sigterm, NULL); -#ifdef CONFIG_POSIX signal(SIGPIPE, SIG_IGN); #endif |