* [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2
@ 2017-07-07 15:29 Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 01/10] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT Vladimir Sementsov-Ogievskiy
` (10 more replies)
0 siblings, 11 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
This is last part of "nbd errors and traces refactoring".
Patches 01-10 from "nbd errors and traces refactoring" are already merged.
Patches 11-13 from it are rejected - do not regret them.
So, these series is update of 14-19 patches from "nbd errors and traces
refactoring"
v3:
01: reword commit message
add Eric's r-b
s/legal/value/
02: new patch (proposed by Eric)
03: rebased on 02
04,05: add Eric's r-b
06: do not remove extra empty line
do not add additional var nbd_magic (reuse existent magic)
use ldq_be_p to reduce memory copying
07,08: new patches
09: new patch, split out 'behavior changing' part of 09, apply most of
comments by Eric and Max, some other changes
10: rebased on 08, now more 'mechanical', may be some changes in text
messages of traces are left here.
v2:
01: EPIPE not handled
imporve comments
02: rebased on absence of old patch 13, errors are not saved into
separate ret variable
TRACE on fail of sending NBD_REP_ACK reply to NBD_OPT_ABORT
instead of error propagating and EPIPE handling
03: fix error_prepend
04: just remove this TRACE
05: just trace nbd magic as 64bit number
06: improve commit message
simplify trace_nbd_receive_negotiate_server_flags - do not trace
separate flags
add trace_nbd_opt_abort_reply_failed
remove tail periods from traces
Vladimir Sementsov-Ogievskiy (10):
nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT
nbd/server: refactor nbd_negotiate
nbd/server: use errp instead of LOG
nbd/server: add errp to nbd_send_reply()
nbd/common: nbd_tls_handshake: remove extra TRACE
nbd/client: refactor TRACE of NBD_MAGIC
nbd/server: fix TRACE in nbd_negotiate_send_rep_len
nbd/server: rename clientflags var in nbd_negotiate_options
nbd: refactor tracing
nbd: use generic trace subsystem instead of TRACE macro
Makefile.objs | 1 +
nbd/client.c | 82 +++++------
nbd/common.c | 4 +-
nbd/nbd-internal.h | 19 ---
nbd/server.c | 398 +++++++++++++++++++++++++++++------------------------
nbd/trace-events | 58 ++++++++
6 files changed, 312 insertions(+), 250 deletions(-)
create mode 100644 nbd/trace-events
--
2.11.1
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 01/10] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 02/10] nbd/server: refactor nbd_negotiate Vladimir Sementsov-Ogievskiy
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Separate the case when a client sends NBD_OPT_ABORT from all other
errors. It will be needed for the following patch, where errors will be
reported.
This particular case is not actually an error - it honestly follows the
NBD protocol. Therefore it should not be reported like an error.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 8a70c054a6..3963972337 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -349,9 +349,13 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
return QIO_CHANNEL(tioc);
}
-
-/* Process all NBD_OPT_* client option commands.
- * Return -errno on error, 0 on success. */
+/* nbd_negotiate_options
+ * Process all NBD_OPT_* client option commands.
+ * Return:
+ * -errno on error
+ * 0 on successful negotiation
+ * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect
+ */
static int nbd_negotiate_options(NBDClient *client)
{
uint32_t flags;
@@ -459,7 +463,7 @@ static int nbd_negotiate_options(NBDClient *client)
}
/* Let the client keep trying, unless they asked to quit */
if (clientflags == NBD_OPT_ABORT) {
- return -EINVAL;
+ return 1;
}
break;
}
@@ -477,7 +481,7 @@ static int nbd_negotiate_options(NBDClient *client)
* disconnecting, but that we must also tolerate
* guests that don't wait for our reply. */
nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags);
- return -EINVAL;
+ return 1;
case NBD_OPT_EXPORT_NAME:
return nbd_negotiate_handle_export_name(client, length);
@@ -533,6 +537,12 @@ static int nbd_negotiate_options(NBDClient *client)
}
}
+/* nbd_negotiate
+ * Return:
+ * -errno on error
+ * 0 on successful negotiation
+ * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect
+ */
static coroutine_fn int nbd_negotiate(NBDClient *client)
{
char buf[8 + 8 + 8 + 128];
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 02/10] nbd/server: refactor nbd_negotiate
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 01/10] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:42 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 03/10] nbd/server: use errp instead of LOG Vladimir Sementsov-Ogievskiy
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Combine two successive "if (oldStyle) {...} else {...}" into one.
Block "if (client->tlscreds)" under "if (oldStyle)" is unreachable,
as we have "oldStyle = client->exp != NULL && !client->tlscreds;".
So, delete this block.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/server.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 3963972337..a6a57ce7c1 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -584,21 +584,15 @@ static coroutine_fn int nbd_negotiate(NBDClient *client)
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
stq_be_p(buf + 16, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
- } else {
- stq_be_p(buf + 8, NBD_OPTS_MAGIC);
- stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
- }
- if (oldStyle) {
- if (client->tlscreds) {
- TRACE("TLS cannot be enabled with oldstyle protocol");
- return -EINVAL;
- }
if (nbd_write(client->ioc, buf, sizeof(buf), NULL) < 0) {
LOG("write failed");
return -EINVAL;
}
} else {
+ stq_be_p(buf + 8, NBD_OPTS_MAGIC);
+ stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
+
if (nbd_write(client->ioc, buf, 18, NULL) < 0) {
LOG("write failed");
return -EINVAL;
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 03/10] nbd/server: use errp instead of LOG
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 01/10] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 02/10] nbd/server: refactor nbd_negotiate Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:48 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 04/10] nbd/server: add errp to nbd_send_reply() Vladimir Sementsov-Ogievskiy
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Move to modern errp scheme from just LOGging errors.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/server.c | 266 +++++++++++++++++++++++++++++++++++------------------------
1 file changed, 160 insertions(+), 106 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index a6a57ce7c1..00a7c606ae 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -134,7 +134,7 @@ static void nbd_client_receive_next_request(NBDClient *client);
/* Send a reply header, including length, but no payload.
* Return -errno on error, 0 on success. */
static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
- uint32_t opt, uint32_t len)
+ uint32_t opt, uint32_t len, Error **errp)
{
uint64_t magic;
@@ -142,23 +142,26 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
type, opt, len);
magic = cpu_to_be64(NBD_REP_MAGIC);
- if (nbd_write(ioc, &magic, sizeof(magic), NULL) < 0) {
- LOG("write failed (rep magic)");
+ if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) {
+ error_prepend(errp, "write failed (rep magic): ");
return -EINVAL;
}
+
opt = cpu_to_be32(opt);
- if (nbd_write(ioc, &opt, sizeof(opt), NULL) < 0) {
- LOG("write failed (rep opt)");
+ if (nbd_write(ioc, &opt, sizeof(opt), errp) < 0) {
+ error_prepend(errp, "write failed (rep opt): ");
return -EINVAL;
}
+
type = cpu_to_be32(type);
- if (nbd_write(ioc, &type, sizeof(type), NULL) < 0) {
- LOG("write failed (rep type)");
+ if (nbd_write(ioc, &type, sizeof(type), errp) < 0) {
+ error_prepend(errp, "write failed (rep type): ");
return -EINVAL;
}
+
len = cpu_to_be32(len);
- if (nbd_write(ioc, &len, sizeof(len), NULL) < 0) {
- LOG("write failed (rep data length)");
+ if (nbd_write(ioc, &len, sizeof(len), errp) < 0) {
+ error_prepend(errp, "write failed (rep data length): ");
return -EINVAL;
}
return 0;
@@ -166,16 +169,17 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
/* Send a reply header with default 0 length.
* Return -errno on error, 0 on success. */
-static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
+static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt,
+ Error **errp)
{
- return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
+ return nbd_negotiate_send_rep_len(ioc, type, opt, 0, errp);
}
/* Send an error reply.
* Return -errno on error, 0 on success. */
-static int GCC_FMT_ATTR(4, 5)
+static int GCC_FMT_ATTR(5, 6)
nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
- uint32_t opt, const char *fmt, ...)
+ uint32_t opt, Error **errp, const char *fmt, ...)
{
va_list va;
char *msg;
@@ -188,16 +192,17 @@ nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
len = strlen(msg);
assert(len < 4096);
TRACE("sending error message \"%s\"", msg);
- ret = nbd_negotiate_send_rep_len(ioc, type, opt, len);
+ ret = nbd_negotiate_send_rep_len(ioc, type, opt, len, errp);
if (ret < 0) {
goto out;
}
- if (nbd_write(ioc, msg, len, NULL) < 0) {
- LOG("write failed (error message)");
+ if (nbd_write(ioc, msg, len, errp) < 0) {
+ error_prepend(errp, "write failed (error message): ");
ret = -EIO;
} else {
ret = 0;
}
+
out:
g_free(msg);
return ret;
@@ -205,7 +210,8 @@ out:
/* Send a single NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
* Return -errno on error, 0 on success. */
-static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
+static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp,
+ Error **errp)
{
size_t name_len, desc_len;
uint32_t len;
@@ -217,53 +223,60 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
name_len = strlen(name);
desc_len = strlen(desc);
len = name_len + desc_len + sizeof(len);
- ret = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST, len);
+ ret = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST, len,
+ errp);
if (ret < 0) {
return ret;
}
len = cpu_to_be32(name_len);
- if (nbd_write(ioc, &len, sizeof(len), NULL) < 0) {
- LOG("write failed (name length)");
+ if (nbd_write(ioc, &len, sizeof(len), errp) < 0) {
+ error_prepend(errp, "write failed (name length): ");
return -EINVAL;
}
- if (nbd_write(ioc, name, name_len, NULL) < 0) {
- LOG("write failed (name buffer)");
+
+ if (nbd_write(ioc, name, name_len, errp) < 0) {
+ error_prepend(errp, "write failed (name buffer): ");
return -EINVAL;
}
- if (nbd_write(ioc, desc, desc_len, NULL) < 0) {
- LOG("write failed (description buffer)");
+
+ if (nbd_write(ioc, desc, desc_len, errp) < 0) {
+ error_prepend(errp, "write failed (description buffer): ");
return -EINVAL;
}
+
return 0;
}
/* Process the NBD_OPT_LIST command, with a potential series of replies.
* Return -errno on error, 0 on success. */
-static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)
+static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length,
+ Error **errp)
{
NBDExport *exp;
if (length) {
- if (nbd_drop(client->ioc, length, NULL) < 0) {
+ if (nbd_drop(client->ioc, length, errp) < 0) {
return -EIO;
}
return nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_INVALID, NBD_OPT_LIST,
+ errp,
"OPT_LIST should not have length");
}
/* For each export, send a NBD_REP_SERVER reply. */
QTAILQ_FOREACH(exp, &exports, next) {
- if (nbd_negotiate_send_rep_list(client->ioc, exp)) {
+ if (nbd_negotiate_send_rep_list(client->ioc, exp, errp)) {
return -EINVAL;
}
}
/* Finish with a NBD_REP_ACK. */
- return nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, NBD_OPT_LIST);
+ return nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, NBD_OPT_LIST, errp);
}
-static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
+static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length,
+ Error **errp)
{
char name[NBD_MAX_NAME_SIZE + 1];
@@ -272,11 +285,11 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
*/
TRACE("Checking length");
if (length >= sizeof(name)) {
- LOG("Bad length received");
+ error_setg(errp, "Bad length received");
return -EINVAL;
}
- if (nbd_read(client->ioc, name, length, NULL) < 0) {
- LOG("read failed");
+ if (nbd_read(client->ioc, name, length, errp) < 0) {
+ error_prepend(errp, "read failed: ");
return -EINVAL;
}
name[length] = '\0';
@@ -285,7 +298,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
client->exp = nbd_export_find(name);
if (!client->exp) {
- LOG("export not found");
+ error_setg(errp, "export not found");
return -EINVAL;
}
@@ -298,7 +311,8 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)
/* Handle NBD_OPT_STARTTLS. Return NULL to drop connection, or else the
* new channel for all further (now-encrypted) communication. */
static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
- uint32_t length)
+ uint32_t length,
+ Error **errp)
{
QIOChannel *ioc;
QIOChannelTLS *tioc;
@@ -307,23 +321,24 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
TRACE("Setting up TLS");
ioc = client->ioc;
if (length) {
- if (nbd_drop(ioc, length, NULL) < 0) {
+ if (nbd_drop(ioc, length, errp) < 0) {
return NULL;
}
nbd_negotiate_send_rep_err(ioc, NBD_REP_ERR_INVALID, NBD_OPT_STARTTLS,
+ errp,
"OPT_STARTTLS should not have length");
return NULL;
}
if (nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK,
- NBD_OPT_STARTTLS) < 0) {
+ NBD_OPT_STARTTLS, errp) < 0) {
return NULL;
}
tioc = qio_channel_tls_new_server(ioc,
client->tlscreds,
client->tlsaclname,
- NULL);
+ errp);
if (!tioc) {
return NULL;
}
@@ -342,7 +357,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
g_main_loop_unref(data.loop);
if (data.error) {
object_unref(OBJECT(tioc));
- error_free(data.error);
+ error_propagate(errp, data.error);
return NULL;
}
@@ -352,14 +367,16 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
/* nbd_negotiate_options
* Process all NBD_OPT_* client option commands.
* Return:
- * -errno on error
- * 0 on successful negotiation
- * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect
+ * -errno on error, errp is set
+ * 0 on successful negotiation, errp is not set
+ * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect,
+ * errp is not set
*/
-static int nbd_negotiate_options(NBDClient *client)
+static int nbd_negotiate_options(NBDClient *client, Error **errp)
{
uint32_t flags;
bool fixedNewstyle = false;
+ Error *local_err = NULL;
/* Client sends:
[ 0 .. 3] client flags
@@ -375,8 +392,8 @@ static int nbd_negotiate_options(NBDClient *client)
... Rest of request
*/
- if (nbd_read(client->ioc, &flags, sizeof(flags), NULL) < 0) {
- LOG("read failed");
+ if (nbd_read(client->ioc, &flags, sizeof(flags), errp) < 0) {
+ error_prepend(errp, "read failed: ");
return -EIO;
}
TRACE("Checking client flags");
@@ -392,7 +409,7 @@ static int nbd_negotiate_options(NBDClient *client)
flags &= ~NBD_FLAG_C_NO_ZEROES;
}
if (flags != 0) {
- TRACE("Unknown client flags 0x%" PRIx32 " received", flags);
+ error_setg(errp, "Unknown client flags 0x%" PRIx32 " received", flags);
return -EIO;
}
@@ -401,26 +418,25 @@ static int nbd_negotiate_options(NBDClient *client)
uint32_t clientflags, length;
uint64_t magic;
- if (nbd_read(client->ioc, &magic, sizeof(magic), NULL) < 0) {
- LOG("read failed");
+ if (nbd_read(client->ioc, &magic, sizeof(magic), errp) < 0) {
+ error_prepend(errp, "read failed: ");
return -EINVAL;
}
TRACE("Checking opts magic");
if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
- LOG("Bad magic received");
+ error_setg(errp, "Bad magic received");
return -EINVAL;
}
if (nbd_read(client->ioc, &clientflags,
- sizeof(clientflags), NULL) < 0)
- {
- LOG("read failed");
+ sizeof(clientflags), errp) < 0) {
+ error_prepend(errp, "read failed: ");
return -EINVAL;
}
clientflags = be32_to_cpu(clientflags);
- if (nbd_read(client->ioc, &length, sizeof(length), NULL) < 0) {
- LOG("read failed");
+ if (nbd_read(client->ioc, &length, sizeof(length), errp) < 0) {
+ error_prepend(errp, "read failed: ");
return -EINVAL;
}
length = be32_to_cpu(length);
@@ -430,12 +446,12 @@ static int nbd_negotiate_options(NBDClient *client)
client->ioc == (QIOChannel *)client->sioc) {
QIOChannel *tioc;
if (!fixedNewstyle) {
- TRACE("Unsupported option 0x%" PRIx32, clientflags);
+ error_setg(errp, "Unsupported option 0x%" PRIx32, clientflags);
return -EINVAL;
}
switch (clientflags) {
case NBD_OPT_STARTTLS:
- tioc = nbd_negotiate_handle_starttls(client, length);
+ tioc = nbd_negotiate_handle_starttls(client, length, errp);
if (!tioc) {
return -EIO;
}
@@ -445,16 +461,17 @@ static int nbd_negotiate_options(NBDClient *client)
case NBD_OPT_EXPORT_NAME:
/* No way to return an error to client, so drop connection */
- TRACE("Option 0x%x not permitted before TLS", clientflags);
+ error_setg(errp, "Option 0x%x not permitted before TLS",
+ clientflags);
return -EINVAL;
default:
- if (nbd_drop(client->ioc, length, NULL) < 0) {
+ if (nbd_drop(client->ioc, length, errp) < 0) {
return -EIO;
}
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_TLS_REQD,
- clientflags,
+ clientflags, errp,
"Option 0x%" PRIx32
"not permitted before TLS",
clientflags);
@@ -470,7 +487,7 @@ static int nbd_negotiate_options(NBDClient *client)
} else if (fixedNewstyle) {
switch (clientflags) {
case NBD_OPT_LIST:
- ret = nbd_negotiate_handle_list(client, length);
+ ret = nbd_negotiate_handle_list(client, length, errp);
if (ret < 0) {
return ret;
}
@@ -480,25 +497,33 @@ static int nbd_negotiate_options(NBDClient *client)
/* NBD spec says we must try to reply before
* disconnecting, but that we must also tolerate
* guests that don't wait for our reply. */
- nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags);
+ nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags,
+ &local_err);
+
+ if (local_err != NULL) {
+ TRACE("Reply to NBD_OPT_ABORT request failed: %s",
+ error_get_pretty(local_err));
+ error_free(local_err);
+ }
+
return 1;
case NBD_OPT_EXPORT_NAME:
- return nbd_negotiate_handle_export_name(client, length);
+ return nbd_negotiate_handle_export_name(client, length, errp);
case NBD_OPT_STARTTLS:
- if (nbd_drop(client->ioc, length, NULL) < 0) {
+ if (nbd_drop(client->ioc, length, errp) < 0) {
return -EIO;
}
if (client->tlscreds) {
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_INVALID,
- clientflags,
+ clientflags, errp,
"TLS already enabled");
} else {
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_POLICY,
- clientflags,
+ clientflags, errp,
"TLS not configured");
}
if (ret < 0) {
@@ -506,12 +531,12 @@ static int nbd_negotiate_options(NBDClient *client)
}
break;
default:
- if (nbd_drop(client->ioc, length, NULL) < 0) {
+ if (nbd_drop(client->ioc, length, errp) < 0) {
return -EIO;
}
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_UNSUP,
- clientflags,
+ clientflags, errp,
"Unsupported option 0x%"
PRIx32,
clientflags);
@@ -527,10 +552,10 @@ static int nbd_negotiate_options(NBDClient *client)
*/
switch (clientflags) {
case NBD_OPT_EXPORT_NAME:
- return nbd_negotiate_handle_export_name(client, length);
+ return nbd_negotiate_handle_export_name(client, length, errp);
default:
- TRACE("Unsupported option 0x%" PRIx32, clientflags);
+ error_setg(errp, "Unsupported option 0x%" PRIx32, clientflags);
return -EINVAL;
}
}
@@ -539,11 +564,12 @@ static int nbd_negotiate_options(NBDClient *client)
/* nbd_negotiate
* Return:
- * -errno on error
- * 0 on successful negotiation
- * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect
+ * -errno on error, errp is set
+ * 0 on successful negotiation, errp is not set
+ * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect,
+ * errp is not set
*/
-static coroutine_fn int nbd_negotiate(NBDClient *client)
+static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
{
char buf[8 + 8 + 8 + 128];
int ret;
@@ -585,21 +611,23 @@ static coroutine_fn int nbd_negotiate(NBDClient *client)
stq_be_p(buf + 16, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
- if (nbd_write(client->ioc, buf, sizeof(buf), NULL) < 0) {
- LOG("write failed");
+ if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) {
+ error_prepend(errp, "write failed: ");
return -EINVAL;
}
} else {
stq_be_p(buf + 8, NBD_OPTS_MAGIC);
stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
- if (nbd_write(client->ioc, buf, 18, NULL) < 0) {
- LOG("write failed");
+ if (nbd_write(client->ioc, buf, 18, errp) < 0) {
+ error_prepend(errp, "write failed: ");
return -EINVAL;
}
- ret = nbd_negotiate_options(client);
+ ret = nbd_negotiate_options(client, errp);
if (ret != 0) {
- LOG("option negotiation failed");
+ if (ret < 0) {
+ error_prepend(errp, "option negotiation failed: ");
+ }
return ret;
}
@@ -608,9 +636,9 @@ static coroutine_fn int nbd_negotiate(NBDClient *client)
stq_be_p(buf + 18, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
len = client->no_zeroes ? 10 : sizeof(buf) - 18;
- ret = nbd_write(client->ioc, buf + 18, len, NULL);
+ ret = nbd_write(client->ioc, buf + 18, len, errp);
if (ret < 0) {
- LOG("write failed");
+ error_prepend(errp, "write failed: ");
return ret;
}
}
@@ -620,13 +648,14 @@ static coroutine_fn int nbd_negotiate(NBDClient *client)
return 0;
}
-static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request)
+static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request,
+ Error **errp)
{
uint8_t buf[NBD_REQUEST_SIZE];
uint32_t magic;
int ret;
- ret = nbd_read(ioc, buf, sizeof(buf), NULL);
+ ret = nbd_read(ioc, buf, sizeof(buf), errp);
if (ret < 0) {
return ret;
}
@@ -652,7 +681,7 @@ static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request)
magic, request->flags, request->type, request->from, request->len);
if (magic != NBD_REQUEST_MAGIC) {
- LOG("invalid magic (got 0x%" PRIx32 ")", magic);
+ error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", magic);
return -EINVAL;
}
return 0;
@@ -998,13 +1027,14 @@ static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len)
* the client (although the caller may still need to disconnect after reporting
* the error).
*/
-static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request)
+static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
+ Error **errp)
{
NBDClient *client = req->client;
g_assert(qemu_in_coroutine());
assert(client->recv_coroutine == qemu_coroutine_self());
- if (nbd_receive_request(client->ioc, request) < 0) {
+ if (nbd_receive_request(client->ioc, request, errp) < 0) {
return -EIO;
}
@@ -1026,27 +1056,29 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request)
* checks as possible until after reading any NBD_CMD_WRITE
* payload, so we can try and keep the connection alive. */
if ((request->from + request->len) < request->from) {
- LOG("integer overflow detected, you're probably being attacked");
+ error_setg(errp,
+ "integer overflow detected, you're probably being attacked");
return -EINVAL;
}
if (request->type == NBD_CMD_READ || request->type == NBD_CMD_WRITE) {
if (request->len > NBD_MAX_BUFFER_SIZE) {
- LOG("len (%" PRIu32" ) is larger than max len (%u)",
- request->len, NBD_MAX_BUFFER_SIZE);
+ error_setg(errp, "len (%" PRIu32" ) is larger than max len (%u)",
+ request->len, NBD_MAX_BUFFER_SIZE);
return -EINVAL;
}
req->data = blk_try_blockalign(client->exp->blk, request->len);
if (req->data == NULL) {
+ error_setg(errp, "No memory");
return -ENOMEM;
}
}
if (request->type == NBD_CMD_WRITE) {
TRACE("Reading %" PRIu32 " byte(s)", request->len);
- if (nbd_read(client->ioc, req->data, request->len, NULL) < 0) {
- LOG("reading from socket failed");
+ if (nbd_read(client->ioc, req->data, request->len, errp) < 0) {
+ error_prepend(errp, "reading from socket failed: ");
return -EIO;
}
req->complete = true;
@@ -1054,18 +1086,18 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request)
/* Sanity checks, part 2. */
if (request->from + request->len > client->exp->size) {
- LOG("operation past EOF; From: %" PRIu64 ", Len: %" PRIu32
- ", Size: %" PRIu64, request->from, request->len,
- (uint64_t)client->exp->size);
+ error_setg(errp, "operation past EOF; From: %" PRIu64 ", Len: %" PRIu32
+ ", Size: %" PRIu64, request->from, request->len,
+ (uint64_t)client->exp->size);
return request->type == NBD_CMD_WRITE ? -ENOSPC : -EINVAL;
}
if (request->flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE)) {
- LOG("unsupported flags (got 0x%x)", request->flags);
+ error_setg(errp, "unsupported flags (got 0x%x)", request->flags);
return -EINVAL;
}
if (request->type != NBD_CMD_WRITE_ZEROES &&
(request->flags & NBD_CMD_FLAG_NO_HOLE)) {
- LOG("unexpected flags (got 0x%x)", request->flags);
+ error_setg(errp, "unexpected flags (got 0x%x)", request->flags);
return -EINVAL;
}
@@ -1083,6 +1115,7 @@ static coroutine_fn void nbd_trip(void *opaque)
int ret;
int flags;
int reply_data_len = 0;
+ Error *local_err = NULL;
TRACE("Reading request.");
if (client->closing) {
@@ -1091,7 +1124,7 @@ static coroutine_fn void nbd_trip(void *opaque)
}
req = nbd_request_get(client);
- ret = nbd_co_receive_request(req, &request);
+ ret = nbd_co_receive_request(req, &request, &local_err);
client->recv_coroutine = NULL;
nbd_client_receive_next_request(client);
if (ret == -EIO) {
@@ -1122,7 +1155,7 @@ static coroutine_fn void nbd_trip(void *opaque)
if (request.flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->blk);
if (ret < 0) {
- LOG("flush failed");
+ error_setg_errno(&local_err, -ret, "flush failed");
reply.error = -ret;
break;
}
@@ -1131,7 +1164,7 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_pread(exp->blk, request.from + exp->dev_offset,
req->data, request.len);
if (ret < 0) {
- LOG("reading from file failed");
+ error_setg_errno(&local_err, -ret, "reading from file failed");
reply.error = -ret;
break;
}
@@ -1158,7 +1191,7 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_pwrite(exp->blk, request.from + exp->dev_offset,
req->data, request.len, flags);
if (ret < 0) {
- LOG("writing to file failed");
+ error_setg_errno(&local_err, -ret, "writing to file failed");
reply.error = -ret;
}
@@ -1167,7 +1200,7 @@ static coroutine_fn void nbd_trip(void *opaque)
TRACE("Request type is WRITE_ZEROES");
if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
- TRACE("Server is read-only, return error");
+ error_setg(&local_err, "Server is read-only, return error");
reply.error = EROFS;
break;
}
@@ -1184,7 +1217,7 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_pwrite_zeroes(exp->blk, request.from + exp->dev_offset,
request.len, flags);
if (ret < 0) {
- LOG("writing to file failed");
+ error_setg_errno(&local_err, -ret, "writing to file failed");
reply.error = -ret;
}
@@ -1198,7 +1231,7 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_co_flush(exp->blk);
if (ret < 0) {
- LOG("flush failed");
+ error_setg_errno(&local_err, -ret, "flush failed");
reply.error = -ret;
}
@@ -1208,21 +1241,35 @@ static coroutine_fn void nbd_trip(void *opaque)
ret = blk_co_pdiscard(exp->blk, request.from + exp->dev_offset,
request.len);
if (ret < 0) {
- LOG("discard failed");
+ error_setg_errno(&local_err, -ret, "discard failed");
reply.error = -ret;
}
break;
default:
- LOG("invalid request type (%" PRIu32 ") received", request.type);
+ error_setg(&local_err, "invalid request type (%" PRIu32 ") received",
+ request.type);
reply.error = EINVAL;
}
reply:
+ if (local_err) {
+ /* If we are here local_err is not fatal error, already stored in
+ * reply.error */
+ error_report_err(local_err);
+ local_err = NULL;
+ }
+
+ if (nbd_co_send_reply(req, &reply, reply_data_len) < 0) {
+ error_setg(&local_err, "Failed to send reply");
+ goto disconnect;
+ }
+
/* We must disconnect after NBD_CMD_WRITE if we did not
* read the payload.
*/
- if (nbd_co_send_reply(req, &reply, reply_data_len) < 0 || !req->complete) {
+ if (!req->complete) {
+ error_setg(&local_err, "Request handling failed in intermediate state");
goto disconnect;
}
@@ -1234,6 +1281,9 @@ done:
return;
disconnect:
+ if (local_err) {
+ error_reportf_err(local_err, "Disconnect client, due to: ");
+ }
nbd_request_put(req);
client_close(client, true);
nbd_client_put(client);
@@ -1252,6 +1302,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
{
NBDClient *client = opaque;
NBDExport *exp = client->exp;
+ Error *local_err = NULL;
if (exp) {
nbd_export_get(exp);
@@ -1259,7 +1310,10 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
}
qemu_co_mutex_init(&client->send_lock);
- if (nbd_negotiate(client)) {
+ if (nbd_negotiate(client, &local_err)) {
+ if (local_err) {
+ error_report_err(local_err);
+ }
client_close(client, false);
return;
}
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 04/10] nbd/server: add errp to nbd_send_reply()
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 03/10] nbd/server: use errp instead of LOG Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 05/10] nbd/common: nbd_tls_handshake: remove extra TRACE Vladimir Sementsov-Ogievskiy
` (6 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 00a7c606ae..e3fd0ca8fe 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -687,7 +687,7 @@ static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request,
return 0;
}
-static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply)
+static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
{
uint8_t buf[NBD_REPLY_SIZE];
@@ -706,7 +706,7 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply)
stl_be_p(buf + 4, reply->error);
stq_be_p(buf + 8, reply->handle);
- return nbd_write(ioc, buf, sizeof(buf), NULL);
+ return nbd_write(ioc, buf, sizeof(buf), errp);
}
#define MAX_NBD_REQUESTS 16
@@ -993,7 +993,8 @@ void nbd_export_close_all(void)
}
}
-static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len)
+static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len,
+ Error **errp)
{
NBDClient *client = req->client;
int ret;
@@ -1003,12 +1004,12 @@ static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len)
client->send_coroutine = qemu_coroutine_self();
if (!len) {
- ret = nbd_send_reply(client->ioc, reply);
+ ret = nbd_send_reply(client->ioc, reply, errp);
} else {
qio_channel_set_cork(client->ioc, true);
- ret = nbd_send_reply(client->ioc, reply);
+ ret = nbd_send_reply(client->ioc, reply, errp);
if (ret == 0) {
- ret = nbd_write(client->ioc, req->data, len, NULL);
+ ret = nbd_write(client->ioc, req->data, len, errp);
if (ret < 0) {
ret = -EIO;
}
@@ -1260,8 +1261,8 @@ reply:
local_err = NULL;
}
- if (nbd_co_send_reply(req, &reply, reply_data_len) < 0) {
- error_setg(&local_err, "Failed to send reply");
+ if (nbd_co_send_reply(req, &reply, reply_data_len, &local_err) < 0) {
+ error_prepend(&local_err, "Failed to send reply: ");
goto disconnect;
}
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 05/10] nbd/common: nbd_tls_handshake: remove extra TRACE
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 04/10] nbd/server: add errp to nbd_send_reply() Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 06/10] nbd/client: refactor TRACE of NBD_MAGIC Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Error is propagated to the caller, TRACE is not needed.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
nbd/common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/nbd/common.c b/nbd/common.c
index 6b5c1b7b02..4dab41e2c0 100644
--- a/nbd/common.c
+++ b/nbd/common.c
@@ -97,9 +97,7 @@ void nbd_tls_handshake(QIOTask *task,
{
struct NBDTLSHandshakeData *data = opaque;
- if (qio_task_propagate_error(task, &data->error)) {
- TRACE("TLS failed %s", error_get_pretty(data->error));
- }
+ qio_task_propagate_error(task, &data->error);
data->complete = true;
g_main_loop_quit(data->loop);
}
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 06/10] nbd/client: refactor TRACE of NBD_MAGIC
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 05/10] nbd/common: nbd_tls_handshake: remove extra TRACE Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:49 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 07/10] nbd/server: fix TRACE in nbd_negotiate_send_rep_len Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
We are going to switch from TRACE macro to trace points,
this TRACE complicates things, this patch simplifies it.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/client.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index b97143fa60..2125321ebd 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -461,15 +461,8 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
- TRACE("Magic is %c%c%c%c%c%c%c%c",
- qemu_isprint(buf[0]) ? buf[0] : '.',
- qemu_isprint(buf[1]) ? buf[1] : '.',
- qemu_isprint(buf[2]) ? buf[2] : '.',
- qemu_isprint(buf[3]) ? buf[3] : '.',
- qemu_isprint(buf[4]) ? buf[4] : '.',
- qemu_isprint(buf[5]) ? buf[5] : '.',
- qemu_isprint(buf[6]) ? buf[6] : '.',
- qemu_isprint(buf[7]) ? buf[7] : '.');
+ magic = ldq_be_p(buf);
+ TRACE("Magic is 0x%" PRIx64, magic);
if (memcmp(buf, "NBDMAGIC", 8) != 0) {
error_setg(errp, "Invalid magic received");
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 07/10] nbd/server: fix TRACE in nbd_negotiate_send_rep_len
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 06/10] nbd/client: refactor TRACE of NBD_MAGIC Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:50 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 08/10] nbd/server: rename clientflags var in nbd_negotiate_options Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Fix wrong order of TRACE arguments.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index e3fd0ca8fe..1eeafccaca 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -139,7 +139,7 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
uint64_t magic;
TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
- type, opt, len);
+ opt, type, len);
magic = cpu_to_be64(NBD_REP_MAGIC);
if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) {
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 08/10] nbd/server: rename clientflags var in nbd_negotiate_options
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (6 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 07/10] nbd/server: fix TRACE in nbd_negotiate_send_rep_len Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:50 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 09/10] nbd: refactor tracing Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Rename 'clientflags' to just 'option'. This variable has nothing to do
with flags, but is a single integer representing the option requested
by the client.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/server.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 1eeafccaca..c4d64fb3ad 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -415,7 +415,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
while (1) {
int ret;
- uint32_t clientflags, length;
+ uint32_t option, length;
uint64_t magic;
if (nbd_read(client->ioc, &magic, sizeof(magic), errp) < 0) {
@@ -428,12 +428,12 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
return -EINVAL;
}
- if (nbd_read(client->ioc, &clientflags,
- sizeof(clientflags), errp) < 0) {
+ if (nbd_read(client->ioc, &option,
+ sizeof(option), errp) < 0) {
error_prepend(errp, "read failed: ");
return -EINVAL;
}
- clientflags = be32_to_cpu(clientflags);
+ option = be32_to_cpu(option);
if (nbd_read(client->ioc, &length, sizeof(length), errp) < 0) {
error_prepend(errp, "read failed: ");
@@ -441,15 +441,15 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
}
length = be32_to_cpu(length);
- TRACE("Checking option 0x%" PRIx32, clientflags);
+ TRACE("Checking option 0x%" PRIx32, option);
if (client->tlscreds &&
client->ioc == (QIOChannel *)client->sioc) {
QIOChannel *tioc;
if (!fixedNewstyle) {
- error_setg(errp, "Unsupported option 0x%" PRIx32, clientflags);
+ error_setg(errp, "Unsupported option 0x%" PRIx32, option);
return -EINVAL;
}
- switch (clientflags) {
+ switch (option) {
case NBD_OPT_STARTTLS:
tioc = nbd_negotiate_handle_starttls(client, length, errp);
if (!tioc) {
@@ -462,7 +462,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
case NBD_OPT_EXPORT_NAME:
/* No way to return an error to client, so drop connection */
error_setg(errp, "Option 0x%x not permitted before TLS",
- clientflags);
+ option);
return -EINVAL;
default:
@@ -471,21 +471,21 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
}
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_TLS_REQD,
- clientflags, errp,
+ option, errp,
"Option 0x%" PRIx32
"not permitted before TLS",
- clientflags);
+ option);
if (ret < 0) {
return ret;
}
/* Let the client keep trying, unless they asked to quit */
- if (clientflags == NBD_OPT_ABORT) {
+ if (option == NBD_OPT_ABORT) {
return 1;
}
break;
}
} else if (fixedNewstyle) {
- switch (clientflags) {
+ switch (option) {
case NBD_OPT_LIST:
ret = nbd_negotiate_handle_list(client, length, errp);
if (ret < 0) {
@@ -497,7 +497,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
/* NBD spec says we must try to reply before
* disconnecting, but that we must also tolerate
* guests that don't wait for our reply. */
- nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, clientflags,
+ nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option,
&local_err);
if (local_err != NULL) {
@@ -518,12 +518,12 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
if (client->tlscreds) {
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_INVALID,
- clientflags, errp,
+ option, errp,
"TLS already enabled");
} else {
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_POLICY,
- clientflags, errp,
+ option, errp,
"TLS not configured");
}
if (ret < 0) {
@@ -536,10 +536,10 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
}
ret = nbd_negotiate_send_rep_err(client->ioc,
NBD_REP_ERR_UNSUP,
- clientflags, errp,
+ option, errp,
"Unsupported option 0x%"
PRIx32,
- clientflags);
+ option);
if (ret < 0) {
return ret;
}
@@ -550,12 +550,12 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
* If broken new-style we should drop the connection
* for anything except NBD_OPT_EXPORT_NAME
*/
- switch (clientflags) {
+ switch (option) {
case NBD_OPT_EXPORT_NAME:
return nbd_negotiate_handle_export_name(client, length, errp);
default:
- error_setg(errp, "Unsupported option 0x%" PRIx32, clientflags);
+ error_setg(errp, "Unsupported option 0x%" PRIx32, option);
return -EINVAL;
}
}
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 09/10] nbd: refactor tracing
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (7 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 08/10] nbd/server: rename clientflags var in nbd_negotiate_options Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 15:56 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 10/10] nbd: use generic trace subsystem instead of TRACE macro Vladimir Sementsov-Ogievskiy
2017-07-07 16:06 ` [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Paolo Bonzini
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Reorganize traces: move, reword, add information, drop extra ones.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
nbd/client.c | 3 ---
nbd/server.c | 30 +++++++++---------------------
2 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 2125321ebd..1ae4ff8a8e 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -489,12 +489,10 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
TRACE("Global flags are %" PRIx32, globalflags);
if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
fixedNewStyle = true;
- TRACE("Server supports fixed new style");
clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
}
if (globalflags & NBD_FLAG_NO_ZEROES) {
zeroes = false;
- TRACE("Server supports no zeroes");
clientflags |= NBD_FLAG_C_NO_ZEROES;
}
/* client requested flags */
@@ -565,7 +563,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
*size = be64_to_cpu(s);
- TRACE("Size is %" PRIu64, *size);
if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) {
error_prepend(errp, "Failed to read export flags");
diff --git a/nbd/server.c b/nbd/server.c
index c4d64fb3ad..4bbce5fe76 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1000,6 +1000,10 @@ static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len,
int ret;
g_assert(qemu_in_coroutine());
+
+ TRACE("Send reply: handle = %" PRIu64 ", error = %" PRIu32 ", len = %d",
+ reply->handle, reply->error, len);
+
qemu_co_mutex_lock(&client->send_lock);
client->send_coroutine = qemu_coroutine_self();
@@ -1039,7 +1043,8 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
return -EIO;
}
- TRACE("Decoding type");
+ TRACE("Decoding type: handle = %" PRIu64 ", type = %" PRIu16,
+ request->handle, request->type);
if (request->type != NBD_CMD_WRITE) {
/* No payload, we are ready to read the next request. */
@@ -1049,7 +1054,6 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
if (request->type == NBD_CMD_DISC) {
/* Special case: we're going to disconnect without a reply,
* whether or not flags, from, or len are bogus */
- TRACE("Request type is DISCONNECT");
return -EIO;
}
@@ -1076,13 +1080,14 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
}
}
if (request->type == NBD_CMD_WRITE) {
- TRACE("Reading %" PRIu32 " byte(s)", request->len);
-
if (nbd_read(client->ioc, req->data, request->len, errp) < 0) {
error_prepend(errp, "reading from socket failed: ");
return -EIO;
}
req->complete = true;
+
+ TRACE("Payload received: handle = %" PRIu64 ", len = %" PRIu32,
+ request->handle, request->len);
}
/* Sanity checks, part 2. */
@@ -1150,8 +1155,6 @@ static coroutine_fn void nbd_trip(void *opaque)
switch (request.type) {
case NBD_CMD_READ:
- TRACE("Request type is READ");
-
/* XXX: NBD Protocol only documents use of FUA with WRITE */
if (request.flags & NBD_CMD_FLAG_FUA) {
ret = blk_co_flush(exp->blk);
@@ -1171,20 +1174,14 @@ static coroutine_fn void nbd_trip(void *opaque)
}
reply_data_len = request.len;
- TRACE("Read %" PRIu32" byte(s)", request.len);
break;
case NBD_CMD_WRITE:
- TRACE("Request type is WRITE");
-
if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
- TRACE("Server is read-only, return error");
reply.error = EROFS;
break;
}
- TRACE("Writing to device");
-
flags = 0;
if (request.flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA;
@@ -1198,16 +1195,12 @@ static coroutine_fn void nbd_trip(void *opaque)
break;
case NBD_CMD_WRITE_ZEROES:
- TRACE("Request type is WRITE_ZEROES");
-
if (exp->nbdflags & NBD_FLAG_READ_ONLY) {
error_setg(&local_err, "Server is read-only, return error");
reply.error = EROFS;
break;
}
- TRACE("Writing to device");
-
flags = 0;
if (request.flags & NBD_CMD_FLAG_FUA) {
flags |= BDRV_REQ_FUA;
@@ -1228,8 +1221,6 @@ static coroutine_fn void nbd_trip(void *opaque)
abort();
case NBD_CMD_FLUSH:
- TRACE("Request type is FLUSH");
-
ret = blk_co_flush(exp->blk);
if (ret < 0) {
error_setg_errno(&local_err, -ret, "flush failed");
@@ -1238,7 +1229,6 @@ static coroutine_fn void nbd_trip(void *opaque)
break;
case NBD_CMD_TRIM:
- TRACE("Request type is TRIM");
ret = blk_co_pdiscard(exp->blk, request.from + exp->dev_offset,
request.len);
if (ret < 0) {
@@ -1274,8 +1264,6 @@ reply:
goto disconnect;
}
- TRACE("Request/Reply complete");
-
done:
nbd_request_put(req);
nbd_client_put(client);
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH v3 10/10] nbd: use generic trace subsystem instead of TRACE macro
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (8 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 09/10] nbd: refactor tracing Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:29 ` Vladimir Sementsov-Ogievskiy
2017-07-07 16:04 ` Eric Blake
2017-07-07 16:06 ` [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Paolo Bonzini
10 siblings, 1 reply; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-07 15:29 UTC (permalink / raw)
To: qemu-devel; +Cc: stefanha, eblake, pbonzini, berrange, den, vsementsov
Let NBD use the trace mechanisms already present in qemu. Now you can
use the -trace optino of qemu, or the -T/--trace option of qemu-img,
qemu-io, and qemu-nbd, to select nbd traces. For qemu, the QMP commands
trace-event-{get,set}-state can also toggle tracing on the fly.
Example:
qemu-nbd --trace 'nbd_*' <image file> # enables all nbd traces
Recompilation with CFLAGS=-DDEBUG_NBD is no more needed, furthermore,
DEBUG_NBD macro is removed from the code.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
Makefile.objs | 1 +
nbd/client.c | 70 ++++++++++++++++++++++++------------------------------
nbd/nbd-internal.h | 19 ---------------
nbd/server.c | 67 ++++++++++++++++++++++++---------------------------
nbd/trace-events | 58 ++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 121 insertions(+), 94 deletions(-)
create mode 100644 nbd/trace-events
diff --git a/Makefile.objs b/Makefile.objs
index b2e6322ef0..a66ea34cc4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -168,6 +168,7 @@ trace-events-subdirs += linux-user
trace-events-subdirs += qapi
trace-events-subdirs += accel/tcg
trace-events-subdirs += accel/kvm
+trace-events-subdirs += nbd
trace-events-files = $(SRC_PATH)/trace-events $(trace-events-subdirs:%=$(SRC_PATH)/%/trace-events)
diff --git a/nbd/client.c b/nbd/client.c
index 1ae4ff8a8e..ab4665307b 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -19,6 +19,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "trace.h"
#include "nbd-internal.h"
static int nbd_errno_to_system_errno(int err)
@@ -44,7 +45,7 @@ static int nbd_errno_to_system_errno(int err)
ret = ESHUTDOWN;
break;
default:
- TRACE("Squashing unexpected error %d to EINVAL", err);
+ trace_nbd_unknown_error(err);
/* fallthrough */
case NBD_EINVAL:
ret = EINVAL;
@@ -103,7 +104,7 @@ static int nbd_send_option_request(QIOChannel *ioc, uint32_t opt,
if (len == -1) {
req.length = len = strlen(data);
}
- TRACE("Sending option request %" PRIu32", len %" PRIu32, opt, len);
+ trace_nbd_send_option_request(opt, len);
stq_be_p(&req.magic, NBD_OPTS_MAGIC);
stl_be_p(&req.option, opt);
@@ -153,8 +154,7 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
be32_to_cpus(&reply->type);
be32_to_cpus(&reply->length);
- TRACE("Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32,
- reply->option, reply->type, reply->length);
+ trace_nbd_receive_option_reply(reply->option, reply->type, reply->length);
if (reply->magic != NBD_REP_MAGIC) {
error_setg(errp, "Unexpected option reply magic");
@@ -201,8 +201,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
switch (reply->type) {
case NBD_REP_ERR_UNSUP:
- TRACE("server doesn't understand request %" PRIx32
- ", attempting fallback", reply->option);
+ trace_nbd_reply_err_unsup(reply->option);
result = 0;
goto cleanup;
@@ -342,12 +341,12 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
{
bool foundExport = false;
- TRACE("Querying export list for '%s'", wantname);
+ trace_nbd_receive_query_exports_start(wantname);
if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) {
return -1;
}
- TRACE("Reading available export names");
+ trace_nbd_receive_query_exports_loop();
while (1) {
int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
@@ -362,7 +361,7 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
nbd_send_opt_abort(ioc);
return -1;
}
- TRACE("Found desired export name '%s'", wantname);
+ trace_nbd_receive_query_exports_success(wantname);
return 0;
}
}
@@ -376,12 +375,12 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
QIOChannelTLS *tioc;
struct NBDTLSHandshakeData data = { 0 };
- TRACE("Requesting TLS from server");
+ trace_nbd_receive_starttls_request();
if (nbd_send_option_request(ioc, NBD_OPT_STARTTLS, 0, NULL, errp) < 0) {
return NULL;
}
- TRACE("Getting TLS reply from server");
+ trace_nbd_receive_starttls_reply();
if (nbd_receive_option_reply(ioc, NBD_OPT_STARTTLS, &reply, errp) < 0) {
return NULL;
}
@@ -400,14 +399,14 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
return NULL;
}
- TRACE("TLS request approved, setting up TLS");
+ trace_nbd_receive_starttls_new_client();
tioc = qio_channel_tls_new_client(ioc, tlscreds, hostname, errp);
if (!tioc) {
return NULL;
}
qio_channel_set_name(QIO_CHANNEL(tioc), "nbd-client-tls");
data.loop = g_main_loop_new(g_main_context_default(), FALSE);
- TRACE("Starting TLS handshake");
+ trace_nbd_receive_starttls_tls_handshake();
qio_channel_tls_handshake(tioc,
nbd_tls_handshake,
&data,
@@ -437,8 +436,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
int rc;
bool zeroes = true;
- TRACE("Receiving negotiation tlscreds=%p hostname=%s.",
- tlscreds, hostname ? hostname : "<null>");
+ trace_nbd_receive_negotiate(tlscreds, hostname ? hostname : "<null>");
rc = -EINVAL;
@@ -462,7 +460,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
magic = ldq_be_p(buf);
- TRACE("Magic is 0x%" PRIx64, magic);
+ trace_nbd_receive_negotiate_magic(magic);
if (memcmp(buf, "NBDMAGIC", 8) != 0) {
error_setg(errp, "Invalid magic received");
@@ -474,7 +472,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
magic = be64_to_cpu(magic);
- TRACE("Magic is 0x%" PRIx64, magic);
+ trace_nbd_receive_negotiate_magic2(magic);
if (magic == NBD_OPTS_MAGIC) {
uint32_t clientflags = 0;
@@ -486,7 +484,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
globalflags = be16_to_cpu(globalflags);
- TRACE("Global flags are %" PRIx32, globalflags);
+ trace_nbd_receive_negotiate_server_flags(globalflags);
if (globalflags & NBD_FLAG_FIXED_NEWSTYLE) {
fixedNewStyle = true;
clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
@@ -514,7 +512,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
}
}
if (!name) {
- TRACE("Using default NBD export name \"\"");
+ trace_nbd_receive_negotiate_default_name();
name = "";
}
if (fixedNewStyle) {
@@ -579,7 +577,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
- TRACE("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags);
+ trace_nbd_receive_negotiate_size_flags(*size, *flags);
if (zeroes && nbd_drop(ioc, 124, errp) < 0) {
error_prepend(errp, "Failed to read reserved block");
goto fail;
@@ -601,7 +599,7 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size,
return -E2BIG;
}
- TRACE("Setting NBD socket");
+ trace_nbd_init_set_socket();
if (ioctl(fd, NBD_SET_SOCK, (unsigned long) sioc->fd) < 0) {
int serrno = errno;
@@ -609,7 +607,7 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size,
return -serrno;
}
- TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZE);
+ trace_nbd_init_set_block_size(BDRV_SECTOR_SIZE);
if (ioctl(fd, NBD_SET_BLKSIZE, (unsigned long)BDRV_SECTOR_SIZE) < 0) {
int serrno = errno;
@@ -617,10 +615,9 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size,
return -serrno;
}
- TRACE("Setting size to %lu block(s)", sectors);
+ trace_nbd_init_set_size(sectors);
if (size % BDRV_SECTOR_SIZE) {
- TRACE("Ignoring trailing %d bytes of export",
- (int) (size % BDRV_SECTOR_SIZE));
+ trace_nbd_init_trailing_bytes(size % BDRV_SECTOR_SIZE);
}
if (ioctl(fd, NBD_SET_SIZE_BLOCKS, sectors) < 0) {
@@ -632,7 +629,7 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size,
if (ioctl(fd, NBD_SET_FLAGS, (unsigned long) flags) < 0) {
if (errno == ENOTTY) {
int read_only = (flags & NBD_FLAG_READ_ONLY) != 0;
- TRACE("Setting readonly attribute");
+ trace_nbd_init_set_readonly();
if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
int serrno = errno;
@@ -646,7 +643,7 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size,
}
}
- TRACE("Negotiation ended");
+ trace_nbd_init_finish();
return 0;
}
@@ -656,7 +653,7 @@ int nbd_client(int fd)
int ret;
int serrno;
- TRACE("Doing NBD loop");
+ trace_nbd_client_loop();
ret = ioctl(fd, NBD_DO_IT);
if (ret < 0 && errno == EPIPE) {
@@ -668,12 +665,12 @@ int nbd_client(int fd)
}
serrno = errno;
- TRACE("NBD loop returned %d: %s", ret, strerror(serrno));
+ trace_nbd_client_loop_ret(ret, strerror(serrno));
- TRACE("Clearing NBD queue");
+ trace_nbd_client_clear_queue();
ioctl(fd, NBD_CLEAR_QUE);
- TRACE("Clearing NBD socket");
+ trace_nbd_client_clear_socket();
ioctl(fd, NBD_CLEAR_SOCK);
errno = serrno;
@@ -710,11 +707,8 @@ ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request)
{
uint8_t buf[NBD_REQUEST_SIZE];
- TRACE("Sending request to server: "
- "{ .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64
- ", .flags = %" PRIx16 ", .type = %" PRIu16 " }",
- request->from, request->len, request->handle,
- request->flags, request->type);
+ trace_nbd_send_request(request->from, request->len, request->handle,
+ request->flags, request->type);
stl_be_p(buf, NBD_REQUEST_MAGIC);
stw_be_p(buf + 4, request->flags);
@@ -759,9 +753,7 @@ ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
error_setg(errp, "server shutting down");
return -EINVAL;
}
- TRACE("Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32
- ", handle = %" PRIu64" }",
- magic, reply->error, reply->handle);
+ trace_nbd_receive_reply(magic, reply->error, reply->handle);
if (magic != NBD_REPLY_MAGIC) {
error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", magic);
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 39bfed177c..cf6ecbf358 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -31,25 +31,6 @@
#include "qemu/queue.h"
#include "qemu/main-loop.h"
-/* #define DEBUG_NBD */
-
-#ifdef DEBUG_NBD
-#define DEBUG_NBD_PRINT 1
-#else
-#define DEBUG_NBD_PRINT 0
-#endif
-
-#define TRACE(msg, ...) do { \
- if (DEBUG_NBD_PRINT) { \
- LOG(msg, ## __VA_ARGS__); \
- } \
-} while (0)
-
-#define LOG(msg, ...) do { \
- fprintf(stderr, "%s:%s():L%d: " msg "\n", \
- __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
-} while (0)
-
/* This is all part of the "official" NBD API.
*
* The most up-to-date documentation is available at:
diff --git a/nbd/server.c b/nbd/server.c
index 4bbce5fe76..ca95f431d4 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -19,6 +19,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "trace.h"
#include "nbd-internal.h"
static int system_errno_to_nbd_errno(int err)
@@ -138,8 +139,7 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
{
uint64_t magic;
- TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
- opt, type, len);
+ trace_nbd_negotiate_send_rep_len(opt, type, len);
magic = cpu_to_be64(NBD_REP_MAGIC);
if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) {
@@ -191,7 +191,7 @@ nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,
va_end(va);
len = strlen(msg);
assert(len < 4096);
- TRACE("sending error message \"%s\"", msg);
+ trace_nbd_negotiate_send_rep_err(msg);
ret = nbd_negotiate_send_rep_len(ioc, type, opt, len, errp);
if (ret < 0) {
goto out;
@@ -219,7 +219,7 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp,
const char *desc = exp->description ? exp->description : "";
int ret;
- TRACE("Advertising export name '%s' description '%s'", name, desc);
+ trace_nbd_negotiate_send_rep_list(name, desc);
name_len = strlen(name);
desc_len = strlen(desc);
len = name_len + desc_len + sizeof(len);
@@ -283,7 +283,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length,
/* Client sends:
[20 .. xx] export name (length bytes)
*/
- TRACE("Checking length");
+ trace_nbd_negotiate_handle_export_name();
if (length >= sizeof(name)) {
error_setg(errp, "Bad length received");
return -EINVAL;
@@ -294,7 +294,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length,
}
name[length] = '\0';
- TRACE("Client requested export '%s'", name);
+ trace_nbd_negotiate_handle_export_name_request(name);
client->exp = nbd_export_find(name);
if (!client->exp) {
@@ -318,7 +318,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
QIOChannelTLS *tioc;
struct NBDTLSHandshakeData data = { 0 };
- TRACE("Setting up TLS");
+ trace_nbd_negotiate_handle_starttls();
ioc = client->ioc;
if (length) {
if (nbd_drop(ioc, length, errp) < 0) {
@@ -344,7 +344,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,
}
qio_channel_set_name(QIO_CHANNEL(tioc), "nbd-server-tls");
- TRACE("Starting TLS handshake");
+ trace_nbd_negotiate_handle_starttls_handshake();
data.loop = g_main_loop_new(g_main_context_default(), FALSE);
qio_channel_tls_handshake(tioc,
nbd_tls_handshake,
@@ -396,15 +396,15 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
error_prepend(errp, "read failed: ");
return -EIO;
}
- TRACE("Checking client flags");
+ trace_nbd_negotiate_options_flags();
be32_to_cpus(&flags);
if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
- TRACE("Client supports fixed newstyle handshake");
+ trace_nbd_negotiate_options_newstyle();
fixedNewstyle = true;
flags &= ~NBD_FLAG_C_FIXED_NEWSTYLE;
}
if (flags & NBD_FLAG_C_NO_ZEROES) {
- TRACE("Client supports no zeroes at handshake end");
+ trace_nbd_negotiate_options_no_zeroes();
client->no_zeroes = true;
flags &= ~NBD_FLAG_C_NO_ZEROES;
}
@@ -422,7 +422,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
error_prepend(errp, "read failed: ");
return -EINVAL;
}
- TRACE("Checking opts magic");
+ trace_nbd_negotiate_options_check_magic();
if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
error_setg(errp, "Bad magic received");
return -EINVAL;
@@ -441,7 +441,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
}
length = be32_to_cpu(length);
- TRACE("Checking option 0x%" PRIx32, option);
+ trace_nbd_negotiate_options_check_option(option);
if (client->tlscreds &&
client->ioc == (QIOChannel *)client->sioc) {
QIOChannel *tioc;
@@ -501,8 +501,8 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
&local_err);
if (local_err != NULL) {
- TRACE("Reply to NBD_OPT_ABORT request failed: %s",
- error_get_pretty(local_err));
+ const char *error = error_get_pretty(local_err);
+ trace_nbd_opt_abort_reply_failed(error);
error_free(local_err);
}
@@ -599,14 +599,14 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
qio_channel_set_blocking(client->ioc, false, NULL);
- TRACE("Beginning negotiation.");
+ trace_nbd_negotiate_begin();
memset(buf, 0, sizeof(buf));
memcpy(buf, "NBDMAGIC", 8);
oldStyle = client->exp != NULL && !client->tlscreds;
if (oldStyle) {
- TRACE("advertising size %" PRIu64 " and flags %x",
- client->exp->size, client->exp->nbdflags | myflags);
+ trace_nbd_negotiate_old_style(client->exp->size,
+ client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
stq_be_p(buf + 16, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
@@ -631,8 +631,8 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
return ret;
}
- TRACE("advertising size %" PRIu64 " and flags %x",
- client->exp->size, client->exp->nbdflags | myflags);
+ trace_nbd_negotiate_new_style_size_flags(
+ client->exp->size, client->exp->nbdflags | myflags);
stq_be_p(buf + 18, client->exp->size);
stw_be_p(buf + 26, client->exp->nbdflags | myflags);
len = client->no_zeroes ? 10 : sizeof(buf) - 18;
@@ -643,7 +643,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
}
}
- TRACE("Negotiation succeeded.");
+ trace_nbd_negotiate_success();
return 0;
}
@@ -676,9 +676,8 @@ static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request,
request->from = ldq_be_p(buf + 16);
request->len = ldl_be_p(buf + 24);
- TRACE("Got request: { magic = 0x%" PRIx32 ", .flags = %" PRIx16
- ", .type = %" PRIx16 ", from = %" PRIu64 ", len = %" PRIu32 " }",
- magic, request->flags, request->type, request->from, request->len);
+ trace_nbd_receive_request(magic, request->flags, request->type,
+ request->from, request->len);
if (magic != NBD_REQUEST_MAGIC) {
error_setg(errp, "invalid magic (got 0x%" PRIx32 ")", magic);
@@ -693,9 +692,7 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
reply->error = system_errno_to_nbd_errno(reply->error);
- TRACE("Sending response to client: { .error = %" PRId32
- ", handle = %" PRIu64 " }",
- reply->error, reply->handle);
+ trace_nbd_send_reply(reply->error, reply->handle);
/* Reply
[ 0 .. 3] magic (NBD_REPLY_MAGIC)
@@ -792,7 +789,7 @@ static void blk_aio_attached(AioContext *ctx, void *opaque)
NBDExport *exp = opaque;
NBDClient *client;
- TRACE("Export %s: Attaching clients to AIO context %p\n", exp->name, ctx);
+ trace_nbd_blk_aio_attached(exp->name, ctx);
exp->ctx = ctx;
@@ -812,7 +809,7 @@ static void blk_aio_detach(void *opaque)
NBDExport *exp = opaque;
NBDClient *client;
- TRACE("Export %s: Detaching clients from AIO context %p\n", exp->name, exp->ctx);
+ trace_nbd_blk_aio_detach(exp->name, exp->ctx);
QTAILQ_FOREACH(client, &exp->clients, next) {
qio_channel_detach_aio_context(client->ioc);
@@ -1001,8 +998,7 @@ static int nbd_co_send_reply(NBDRequestData *req, NBDReply *reply, int len,
g_assert(qemu_in_coroutine());
- TRACE("Send reply: handle = %" PRIu64 ", error = %" PRIu32 ", len = %d",
- reply->handle, reply->error, len);
+ trace_nbd_co_send_reply(reply->handle, reply->error, len);
qemu_co_mutex_lock(&client->send_lock);
client->send_coroutine = qemu_coroutine_self();
@@ -1043,8 +1039,7 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
return -EIO;
}
- TRACE("Decoding type: handle = %" PRIu64 ", type = %" PRIu16,
- request->handle, request->type);
+ trace_nbd_co_receive_request_decode_type(request->handle, request->type);
if (request->type != NBD_CMD_WRITE) {
/* No payload, we are ready to read the next request. */
@@ -1086,8 +1081,8 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
}
req->complete = true;
- TRACE("Payload received: handle = %" PRIu64 ", len = %" PRIu32,
- request->handle, request->len);
+ trace_nbd_co_receive_request_payload_received(request->handle,
+ request->len);
}
/* Sanity checks, part 2. */
@@ -1123,7 +1118,7 @@ static coroutine_fn void nbd_trip(void *opaque)
int reply_data_len = 0;
Error *local_err = NULL;
- TRACE("Reading request.");
+ trace_nbd_trip();
if (client->closing) {
nbd_client_put(client);
return;
diff --git a/nbd/trace-events b/nbd/trace-events
new file mode 100644
index 0000000000..b20165a585
--- /dev/null
+++ b/nbd/trace-events
@@ -0,0 +1,58 @@
+# nbd/client.c
+nbd_unknown_error(int err) "Squashing unexpected error %d to EINVAL"
+nbd_send_option_request(uint32_t opt, uint32_t len) "Sending option request %" PRIu32", len %" PRIu32
+nbd_receive_option_reply(uint32_t option, uint32_t type, uint32_t length) "Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32
+nbd_reply_err_unsup(uint32_t option) "server doesn't understand request %" PRIx32 ", attempting fallback"
+nbd_receive_query_exports_start(const char *wantname) "Querying export list for '%s'"
+nbd_receive_query_exports_loop(void) "Reading available export names"
+nbd_receive_query_exports_success(const char *wantname) "Found desired export name '%s'"
+nbd_receive_starttls_request(void) "Requesting TLS from server"
+nbd_receive_starttls_reply(void) "Getting TLS reply from server"
+nbd_receive_starttls_new_client(void) "TLS request approved, setting up TLS"
+nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
+nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving negotiation tlscreds=%p hostname=%s"
+nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
+nbd_receive_negotiate_magic2(uint64_t magic) "Magic is 0x%" PRIx64
+nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are %" PRIx32
+nbd_receive_negotiate_default_name(void) "Using default NBD export name \"\""
+nbd_receive_negotiate_size_flags(uint64_t size, uint16_t flags) "Size is %" PRIu64 ", export flags %" PRIx16
+nbd_init_set_socket(void) "Setting NBD socket"
+nbd_init_set_block_size(unsigned long block_size) "Setting block size to %lu"
+nbd_init_set_size(unsigned long sectors) "Setting size to %lu block(s)"
+nbd_init_trailing_bytes(int ignored_bytes) "Ignoring trailing %d bytes of export"
+nbd_init_set_readonly(void) "Setting readonly attribute"
+nbd_init_finish(void) "Negotiation ended"
+nbd_client_loop(void) "Doing NBD loop"
+nbd_client_loop_ret(int ret, const char *error) "NBD loop returned %d: %s"
+nbd_client_clear_queue(void) "Clearing NBD queue"
+nbd_client_clear_socket(void) "Clearing NBD socket"
+nbd_send_request(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = %" PRIx16 ", .type = %" PRIu16 " }"
+nbd_receive_reply(uint32_t magic, int32_t error, uint64_t handle) "Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32 ", handle = %" PRIu64" }"
+
+# nbd/server.c
+nbd_negotiate_send_rep_len(uint32_t opt, uint32_t type, uint32_t len) "Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32
+nbd_negotiate_send_rep_err(const char *msg) "sending error message \"%s\""
+nbd_negotiate_send_rep_list(const char *name, const char *desc) "Advertising export name '%s' description '%s'"
+nbd_negotiate_handle_export_name(void) "Checking length"
+nbd_negotiate_handle_export_name_request(const char *name) "Client requested export '%s'"
+nbd_negotiate_handle_starttls(void) "Setting up TLS"
+nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake"
+nbd_negotiate_options_flags(void) "Checking client flags"
+nbd_negotiate_options_newstyle(void) "Client supports fixed newstyle handshake"
+nbd_negotiate_options_no_zeroes(void) "Client supports no zeroes at handshake end"
+nbd_negotiate_options_check_magic(void) "Checking opts magic"
+nbd_negotiate_options_check_option(uint32_t option) "Checking option 0x%" PRIx32
+nbd_opt_abort_reply_failed(const char *error) "Reply to NBD_OPT_ABORT request failed: %s"
+nbd_negotiate_begin(void) "Beginning negotiation"
+nbd_negotiate_old_style(uint64_t size, unsigned flags) "advertising size %" PRIu64 " and flags %x"
+nbd_negotiate_new_style_size_flags(uint64_t size, unsigned flags) "advertising size %" PRIu64 " and flags %x"
+nbd_negotiate_success(void) "Negotiation succeeded"
+nbd_receive_request(uint32_t magic, uint16_t flags, uint16_t type, uint64_t from, uint32_t len) "Got request: { magic = 0x%" PRIx32 ", .flags = %" PRIx16 ", .type = %" PRIx16 ", from = %" PRIu64 ", len = %" PRIu32 " }"
+nbd_send_reply(int32_t error, uint64_t handle) "Sending response to client: { .error = %" PRId32 ", handle = %" PRIu64 " }"
+nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching clients to AIO context %p\n"
+nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clients from AIO context %p\n"
+nbd_co_send_reply(uint64_t handle, uint32_t error, int len) "Send reply: handle = %" PRIu64 ", error = %" PRIu32 ", len = %d"
+nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type) "Decoding type: handle = %" PRIu64 ", type = %" PRIu16
+nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
+nbd_co_receive_request_cmd_write(uint32_t len) "Reading %" PRIu32 " byte(s)"
+nbd_trip(void) "Reading request"
--
2.11.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 02/10] nbd/server: refactor nbd_negotiate
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 02/10] nbd/server: refactor nbd_negotiate Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:42 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 15:42 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 652 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Combine two successive "if (oldStyle) {...} else {...}" into one.
>
> Block "if (client->tlscreds)" under "if (oldStyle)" is unreachable,
> as we have "oldStyle = client->exp != NULL && !client->tlscreds;".
> So, delete this block.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/server.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 03/10] nbd/server: use errp instead of LOG
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 03/10] nbd/server: use errp instead of LOG Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:48 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 15:48 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Move to modern errp scheme from just LOGging errors.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/server.c | 266 +++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 160 insertions(+), 106 deletions(-)
>
> @@ -142,23 +142,26 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
> type, opt, len);
>
> magic = cpu_to_be64(NBD_REP_MAGIC);
> - if (nbd_write(ioc, &magic, sizeof(magic), NULL) < 0) {
> - LOG("write failed (rep magic)");
> + if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) {
> + error_prepend(errp, "write failed (rep magic): ");
> return -EINVAL;
I wonder if we should try harder to preserve the errno returned by
nbd_write() instead of blindly slamming -EINVAL/-EIO everywhere. But
I'm not going to let it hold up my review of this patch.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 06/10] nbd/client: refactor TRACE of NBD_MAGIC
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 06/10] nbd/client: refactor TRACE of NBD_MAGIC Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:49 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 15:49 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 535 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to switch from TRACE macro to trace points,
> this TRACE complicates things, this patch simplifies it.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/client.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 07/10] nbd/server: fix TRACE in nbd_negotiate_send_rep_len
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 07/10] nbd/server: fix TRACE in nbd_negotiate_send_rep_len Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:50 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 15:50 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 438 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Fix wrong order of TRACE arguments.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/server.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 08/10] nbd/server: rename clientflags var in nbd_negotiate_options
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 08/10] nbd/server: rename clientflags var in nbd_negotiate_options Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:50 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 15:50 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Rename 'clientflags' to just 'option'. This variable has nothing to do
> with flags, but is a single integer representing the option requested
> by the client.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/server.c | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 09/10] nbd: refactor tracing
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 09/10] nbd: refactor tracing Vladimir Sementsov-Ogievskiy
@ 2017-07-07 15:56 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 15:56 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Reorganize traces: move, reword, add information, drop extra ones.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> nbd/client.c | 3 ---
> nbd/server.c | 30 +++++++++---------------------
> 2 files changed, 9 insertions(+), 24 deletions(-)
>
> +++ b/nbd/server.c
> @@ -1039,7 +1043,8 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
> return -EIO;
> }
>
> - TRACE("Decoding type");
> + TRACE("Decoding type: handle = %" PRIu64 ", type = %" PRIu16,
> + request->handle, request->type);
Now just prints a number,
>
> if (request->type != NBD_CMD_WRITE) {
> /* No payload, we are ready to read the next request. */
> @@ -1049,7 +1054,6 @@ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request,
> if (request->type == NBD_CMD_DISC) {
> /* Special case: we're going to disconnect without a reply,
> * whether or not flags, from, or len are bogus */
> - TRACE("Request type is DISCONNECT");
where we used to print a name. But for debugging purposes, we can still
figure things out, so not a show-stopper.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 10/10] nbd: use generic trace subsystem instead of TRACE macro
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 10/10] nbd: use generic trace subsystem instead of TRACE macro Vladimir Sementsov-Ogievskiy
@ 2017-07-07 16:04 ` Eric Blake
2017-07-07 19:09 ` [Qemu-devel] [PATCH] fixup! " Eric Blake
0 siblings, 1 reply; 22+ messages in thread
From: Eric Blake @ 2017-07-07 16:04 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, pbonzini, berrange, den
[-- Attachment #1: Type: text/plain, Size: 4489 bytes --]
On 07/07/2017 10:29 AM, Vladimir Sementsov-Ogievskiy wrote:
> Let NBD use the trace mechanisms already present in qemu. Now you can
> use the -trace optino of qemu, or the -T/--trace option of qemu-img,
s/optino/option/
> qemu-io, and qemu-nbd, to select nbd traces. For qemu, the QMP commands
> trace-event-{get,set}-state can also toggle tracing on the fly.
>
> Example:
> qemu-nbd --trace 'nbd_*' <image file> # enables all nbd traces
>
> Recompilation with CFLAGS=-DDEBUG_NBD is no more needed, furthermore,
> DEBUG_NBD macro is removed from the code.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> Makefile.objs | 1 +
> nbd/client.c | 70 ++++++++++++++++++++++++------------------------------
> nbd/nbd-internal.h | 19 ---------------
> nbd/server.c | 67 ++++++++++++++++++++++++---------------------------
> nbd/trace-events | 58 ++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 121 insertions(+), 94 deletions(-)
> create mode 100644 nbd/trace-events
We may still come up with further tweaks, but I have a series that I
plan on posting on top of this, so I'm fine if it goes in as-is (with
the commit message typo fixed).
Reviewed-by: Eric Blake <eblake@redhat.com>
> @@ -342,12 +341,12 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
> {
> bool foundExport = false;
>
> - TRACE("Querying export list for '%s'", wantname);
> + trace_nbd_receive_query_exports_start(wantname);
> if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) {
> return -1;
> }
>
> - TRACE("Reading available export names");
> + trace_nbd_receive_query_exports_loop();
Not sure this one adds much useful information.
> @@ -462,7 +460,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
> }
>
> magic = ldq_be_p(buf);
> - TRACE("Magic is 0x%" PRIx64, magic);
> + trace_nbd_receive_negotiate_magic(magic);
>
> if (memcmp(buf, "NBDMAGIC", 8) != 0) {
> error_setg(errp, "Invalid magic received");
> @@ -474,7 +472,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
> goto fail;
> }
> magic = be64_to_cpu(magic);
> - TRACE("Magic is 0x%" PRIx64, magic);
> + trace_nbd_receive_negotiate_magic2(magic);
I still think you only need one trace function, called twice (rather
than two separate names with identical implementation)
> @@ -396,15 +396,15 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
> error_prepend(errp, "read failed: ");
> return -EIO;
> }
> - TRACE("Checking client flags");
> + trace_nbd_negotiate_options_flags();
> be32_to_cpus(&flags);
> if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
> - TRACE("Client supports fixed newstyle handshake");
> + trace_nbd_negotiate_options_newstyle();
> fixedNewstyle = true;
> flags &= ~NBD_FLAG_C_FIXED_NEWSTYLE;
> }
> if (flags & NBD_FLAG_C_NO_ZEROES) {
> - TRACE("Client supports no zeroes at handshake end");
> + trace_nbd_negotiate_options_no_zeroes();
My followup series will merge these three into one trace (print the
value once, rather than one trace per bit)
> @@ -422,7 +422,7 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
> error_prepend(errp, "read failed: ");
> return -EINVAL;
> }
> - TRACE("Checking opts magic");
> + trace_nbd_negotiate_options_check_magic();
Tracing the magic received might be useful.
> @@ -501,8 +501,8 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
> &local_err);
>
> if (local_err != NULL) {
> - TRACE("Reply to NBD_OPT_ABORT request failed: %s",
> - error_get_pretty(local_err));
> + const char *error = error_get_pretty(local_err);
> + trace_nbd_opt_abort_reply_failed(error);
> error_free(local_err);
> }
Overkill. Who cares if sending the reply to NBD_OPT_ABORT fails. I'll
clean that up in my followup series.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
` (9 preceding siblings ...)
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 10/10] nbd: use generic trace subsystem instead of TRACE macro Vladimir Sementsov-Ogievskiy
@ 2017-07-07 16:06 ` Paolo Bonzini
2017-07-07 16:13 ` Eric Blake
10 siblings, 1 reply; 22+ messages in thread
From: Paolo Bonzini @ 2017-07-07 16:06 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel; +Cc: stefanha, eblake, berrange, den
On 07/07/2017 17:29, Vladimir Sementsov-Ogievskiy wrote:
> This is last part of "nbd errors and traces refactoring".
>
> Patches 01-10 from "nbd errors and traces refactoring" are already merged.
> Patches 11-13 from it are rejected - do not regret them.
> So, these series is update of 14-19 patches from "nbd errors and traces
> refactoring"
Looks good---Eric, want to send a pull request yourself?
Paolo
> v3:
> 01: reword commit message
> add Eric's r-b
> s/legal/value/
> 02: new patch (proposed by Eric)
> 03: rebased on 02
> 04,05: add Eric's r-b
> 06: do not remove extra empty line
> do not add additional var nbd_magic (reuse existent magic)
> use ldq_be_p to reduce memory copying
> 07,08: new patches
> 09: new patch, split out 'behavior changing' part of 09, apply most of
> comments by Eric and Max, some other changes
> 10: rebased on 08, now more 'mechanical', may be some changes in text
> messages of traces are left here.
>
>
> v2:
> 01: EPIPE not handled
> imporve comments
> 02: rebased on absence of old patch 13, errors are not saved into
> separate ret variable
> TRACE on fail of sending NBD_REP_ACK reply to NBD_OPT_ABORT
> instead of error propagating and EPIPE handling
> 03: fix error_prepend
> 04: just remove this TRACE
> 05: just trace nbd magic as 64bit number
> 06: improve commit message
> simplify trace_nbd_receive_negotiate_server_flags - do not trace
> separate flags
> add trace_nbd_opt_abort_reply_failed
> remove tail periods from traces
>
>
> Vladimir Sementsov-Ogievskiy (10):
> nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT
> nbd/server: refactor nbd_negotiate
> nbd/server: use errp instead of LOG
> nbd/server: add errp to nbd_send_reply()
> nbd/common: nbd_tls_handshake: remove extra TRACE
> nbd/client: refactor TRACE of NBD_MAGIC
> nbd/server: fix TRACE in nbd_negotiate_send_rep_len
> nbd/server: rename clientflags var in nbd_negotiate_options
> nbd: refactor tracing
> nbd: use generic trace subsystem instead of TRACE macro
>
> Makefile.objs | 1 +
> nbd/client.c | 82 +++++------
> nbd/common.c | 4 +-
> nbd/nbd-internal.h | 19 ---
> nbd/server.c | 398 +++++++++++++++++++++++++++++------------------------
> nbd/trace-events | 58 ++++++++
> 6 files changed, 312 insertions(+), 250 deletions(-)
> create mode 100644 nbd/trace-events
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2
2017-07-07 16:06 ` [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Paolo Bonzini
@ 2017-07-07 16:13 ` Eric Blake
0 siblings, 0 replies; 22+ messages in thread
From: Eric Blake @ 2017-07-07 16:13 UTC (permalink / raw)
To: Paolo Bonzini, Vladimir Sementsov-Ogievskiy, qemu-devel
Cc: stefanha, berrange, den
[-- Attachment #1: Type: text/plain, Size: 845 bytes --]
On 07/07/2017 11:06 AM, Paolo Bonzini wrote:
>
>
> On 07/07/2017 17:29, Vladimir Sementsov-Ogievskiy wrote:
>> This is last part of "nbd errors and traces refactoring".
>>
>> Patches 01-10 from "nbd errors and traces refactoring" are already merged.
>> Patches 11-13 from it are rejected - do not regret them.
>> So, these series is update of 14-19 patches from "nbd errors and traces
>> refactoring"
>
> Looks good---Eric, want to send a pull request yourself?
Aha - I see where this is going. I'll submit a patch to MAINTAINERS to
propose myself as the new active maintainer of all things NBD, and then
figure out (off-list, with Paolo's help) how to submit this as my first
PULL request.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [Qemu-devel] [PATCH] fixup! nbd: use generic trace subsystem instead of TRACE macro
2017-07-07 16:04 ` Eric Blake
@ 2017-07-07 19:09 ` Eric Blake
2017-07-10 8:53 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 22+ messages in thread
From: Eric Blake @ 2017-07-07 19:09 UTC (permalink / raw)
To: qemu-devel; +Cc: vsementsov, den, Paolo Bonzini, open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
[eblake minor tweaks to a couple of traces]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
If you are okay with this, I plan to squash this into your patch 10
when I send my pull request
nbd/client.c | 3 +--
nbd/server.c | 5 +++--
nbd/trace-events | 4 +---
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index ab46653..9c52b9b 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -346,7 +346,6 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
return -1;
}
- trace_nbd_receive_query_exports_loop();
while (1) {
int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
@@ -472,7 +471,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
goto fail;
}
magic = be64_to_cpu(magic);
- trace_nbd_receive_negotiate_magic2(magic);
+ trace_nbd_receive_negotiate_magic(magic);
if (magic == NBD_OPTS_MAGIC) {
uint32_t clientflags = 0;
diff --git a/nbd/server.c b/nbd/server.c
index ca95f43..9b0c588 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -422,8 +422,9 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
error_prepend(errp, "read failed: ");
return -EINVAL;
}
- trace_nbd_negotiate_options_check_magic();
- if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
+ magic = be64_to_cpu(magic);
+ trace_nbd_negotiate_options_check_magic(magic);
+ if (magic != NBD_OPTS_MAGIC) {
error_setg(errp, "Bad magic received");
return -EINVAL;
}
diff --git a/nbd/trace-events b/nbd/trace-events
index b20165a..4b233b8 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -4,7 +4,6 @@ nbd_send_option_request(uint32_t opt, uint32_t len) "Sending option request %" P
nbd_receive_option_reply(uint32_t option, uint32_t type, uint32_t length) "Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32
nbd_reply_err_unsup(uint32_t option) "server doesn't understand request %" PRIx32 ", attempting fallback"
nbd_receive_query_exports_start(const char *wantname) "Querying export list for '%s'"
-nbd_receive_query_exports_loop(void) "Reading available export names"
nbd_receive_query_exports_success(const char *wantname) "Found desired export name '%s'"
nbd_receive_starttls_request(void) "Requesting TLS from server"
nbd_receive_starttls_reply(void) "Getting TLS reply from server"
@@ -12,7 +11,6 @@ nbd_receive_starttls_new_client(void) "TLS request approved, setting up TLS"
nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving negotiation tlscreds=%p hostname=%s"
nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
-nbd_receive_negotiate_magic2(uint64_t magic) "Magic is 0x%" PRIx64
nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are %" PRIx32
nbd_receive_negotiate_default_name(void) "Using default NBD export name \"\""
nbd_receive_negotiate_size_flags(uint64_t size, uint16_t flags) "Size is %" PRIu64 ", export flags %" PRIx16
@@ -40,7 +38,7 @@ nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake"
nbd_negotiate_options_flags(void) "Checking client flags"
nbd_negotiate_options_newstyle(void) "Client supports fixed newstyle handshake"
nbd_negotiate_options_no_zeroes(void) "Client supports no zeroes at handshake end"
-nbd_negotiate_options_check_magic(void) "Checking opts magic"
+nbd_negotiate_options_check_magic(uint64_t magic) "Checking opts magic 0x%" PRIx64
nbd_negotiate_options_check_option(uint32_t option) "Checking option 0x%" PRIx32
nbd_opt_abort_reply_failed(const char *error) "Reply to NBD_OPT_ABORT request failed: %s"
nbd_negotiate_begin(void) "Beginning negotiation"
--
2.9.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Qemu-devel] [PATCH] fixup! nbd: use generic trace subsystem instead of TRACE macro
2017-07-07 19:09 ` [Qemu-devel] [PATCH] fixup! " Eric Blake
@ 2017-07-10 8:53 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-07-10 8:53 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: den, Paolo Bonzini, open list:Network Block Dev...
07.07.2017 22:09, Eric Blake wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> [eblake minor tweaks to a couple of traces]
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>
> If you are okay with this, I plan to squash this into your patch 10
> when I send my pull request
Ok.
>
> nbd/client.c | 3 +--
> nbd/server.c | 5 +++--
> nbd/trace-events | 4 +---
> 3 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index ab46653..9c52b9b 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -346,7 +346,6 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
> return -1;
> }
>
> - trace_nbd_receive_query_exports_loop();
> while (1) {
> int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
>
> @@ -472,7 +471,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *flags,
> goto fail;
> }
> magic = be64_to_cpu(magic);
> - trace_nbd_receive_negotiate_magic2(magic);
> + trace_nbd_receive_negotiate_magic(magic);
>
> if (magic == NBD_OPTS_MAGIC) {
> uint32_t clientflags = 0;
> diff --git a/nbd/server.c b/nbd/server.c
> index ca95f43..9b0c588 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -422,8 +422,9 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
> error_prepend(errp, "read failed: ");
> return -EINVAL;
> }
> - trace_nbd_negotiate_options_check_magic();
> - if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
> + magic = be64_to_cpu(magic);
> + trace_nbd_negotiate_options_check_magic(magic);
> + if (magic != NBD_OPTS_MAGIC) {
> error_setg(errp, "Bad magic received");
> return -EINVAL;
> }
> diff --git a/nbd/trace-events b/nbd/trace-events
> index b20165a..4b233b8 100644
> --- a/nbd/trace-events
> +++ b/nbd/trace-events
> @@ -4,7 +4,6 @@ nbd_send_option_request(uint32_t opt, uint32_t len) "Sending option request %" P
> nbd_receive_option_reply(uint32_t option, uint32_t type, uint32_t length) "Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32
> nbd_reply_err_unsup(uint32_t option) "server doesn't understand request %" PRIx32 ", attempting fallback"
> nbd_receive_query_exports_start(const char *wantname) "Querying export list for '%s'"
> -nbd_receive_query_exports_loop(void) "Reading available export names"
> nbd_receive_query_exports_success(const char *wantname) "Found desired export name '%s'"
> nbd_receive_starttls_request(void) "Requesting TLS from server"
> nbd_receive_starttls_reply(void) "Getting TLS reply from server"
> @@ -12,7 +11,6 @@ nbd_receive_starttls_new_client(void) "TLS request approved, setting up TLS"
> nbd_receive_starttls_tls_handshake(void) "Starting TLS handshake"
> nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving negotiation tlscreds=%p hostname=%s"
> nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64
> -nbd_receive_negotiate_magic2(uint64_t magic) "Magic is 0x%" PRIx64
> nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are %" PRIx32
> nbd_receive_negotiate_default_name(void) "Using default NBD export name \"\""
> nbd_receive_negotiate_size_flags(uint64_t size, uint16_t flags) "Size is %" PRIu64 ", export flags %" PRIx16
> @@ -40,7 +38,7 @@ nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake"
> nbd_negotiate_options_flags(void) "Checking client flags"
> nbd_negotiate_options_newstyle(void) "Client supports fixed newstyle handshake"
> nbd_negotiate_options_no_zeroes(void) "Client supports no zeroes at handshake end"
> -nbd_negotiate_options_check_magic(void) "Checking opts magic"
> +nbd_negotiate_options_check_magic(uint64_t magic) "Checking opts magic 0x%" PRIx64
> nbd_negotiate_options_check_option(uint32_t option) "Checking option 0x%" PRIx32
> nbd_opt_abort_reply_failed(const char *error) "Reply to NBD_OPT_ABORT request failed: %s"
> nbd_negotiate_begin(void) "Beginning negotiation"
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-07-10 8:53 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-07 15:29 [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 01/10] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 02/10] nbd/server: refactor nbd_negotiate Vladimir Sementsov-Ogievskiy
2017-07-07 15:42 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 03/10] nbd/server: use errp instead of LOG Vladimir Sementsov-Ogievskiy
2017-07-07 15:48 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 04/10] nbd/server: add errp to nbd_send_reply() Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 05/10] nbd/common: nbd_tls_handshake: remove extra TRACE Vladimir Sementsov-Ogievskiy
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 06/10] nbd/client: refactor TRACE of NBD_MAGIC Vladimir Sementsov-Ogievskiy
2017-07-07 15:49 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 07/10] nbd/server: fix TRACE in nbd_negotiate_send_rep_len Vladimir Sementsov-Ogievskiy
2017-07-07 15:50 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 08/10] nbd/server: rename clientflags var in nbd_negotiate_options Vladimir Sementsov-Ogievskiy
2017-07-07 15:50 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 09/10] nbd: refactor tracing Vladimir Sementsov-Ogievskiy
2017-07-07 15:56 ` Eric Blake
2017-07-07 15:29 ` [Qemu-devel] [PATCH v3 10/10] nbd: use generic trace subsystem instead of TRACE macro Vladimir Sementsov-Ogievskiy
2017-07-07 16:04 ` Eric Blake
2017-07-07 19:09 ` [Qemu-devel] [PATCH] fixup! " Eric Blake
2017-07-10 8:53 ` Vladimir Sementsov-Ogievskiy
2017-07-07 16:06 ` [Qemu-devel] [PATCH v3 00/10] nbd refactoring part 2 Paolo Bonzini
2017-07-07 16:13 ` Eric Blake
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.