mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nbd patches for 2020-10-08

Browse Source 
- silence compilation warnings
 - more fixes to prevent reconnect hangs
 - improve 'qemu-nbd' termination behavior
 - cleaner NBD protocol compliance on string handling
 -----BEGIN PGP SIGNATURE-----
 
 iQEzBAABCAAdFiEEccLMIrHEYCkn0vOqp6FrSiUnQ2oFAl+AwooACgkQp6FrSiUn
 Q2o1bAgAkBePla+O40JtmkgbLDBupONiP1z2p4VzlHcUW5KJHJmXe59zkTI1P5Zy
 dsDVZzZ8910Jcjm4qZZHevQEGUbf/3Fu2at6rNlDnXyg/3CZbpEJ3Xg/5QW7ZBVq
 /rgWQjIBB0byHiPX+GLVDJqdkuMBmE/E1BRhVF2Y9kqy4ZWBSvFh4Wfs+OEay26Z
 tyOsU8vf9jK/l7CfDD8wmkTKUHC8AVaQWSJbOc1lZvM4gyzxDI7ci1MpgRizOh1g
 /0svdrEtQ5gyMo6Ael8fR5WOE7qfBIDe3YKou+VqDQHZpEAiofGkt3ZnNUM4FLGE
 IthdUArI4TAuLjyGD76ME72qnAUC4w==
 =Dd7e
 -----END PGP SIGNATURE-----

Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-10-08-v3' into staging

nbd patches for 2020-10-08

- silence compilation warnings
- more fixes to prevent reconnect hangs
- improve 'qemu-nbd' termination behavior
- cleaner NBD protocol compliance on string handling

# gpg: Signature made Fri 09 Oct 2020 21:05:30 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2020-10-08-v3:
  nbd: Simplify meta-context parsing
  nbd/server: Reject embedded NUL in NBD strings
  qemu-nbd: Honor SIGINT and SIGHUP
  block/nbd: nbd_co_reconnect_loop(): don't connect if drained
  block/nbd: fix reconnect-delay
  block/nbd: correctly use qio_channel_detach_aio_context when needed
  block/nbd: fix drain dead-lock because of nbd reconnect-delay
  nbd: silence maybe-uninitialized warnings

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This commit is contained in:
Peter Maydell 2020-10-09 22:55:46 +01:00
commit b433f2cb01
3 changed files with 155 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
block/nbd.c
View File

@ -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));

217
nbd/server.c
View File

@ -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;
return true;
}
if (len < dirty_bitmap_len) {
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;
}
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);
}
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;
}

15
qemu-nbd.c
View File

@ -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