* [Qemu-devel] [PULL 01/11] MAINTAINERS: Promote NBD to supported, with new maintainer
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 14:47 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 02/11] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT Eric Blake
` (10 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
We are promising more than just odd fixes, and Paolo is hoping
to offload the pull requests to me. Also, enough of NBD is related
to the block layer that it is worth including qemu-block on patches.
While at it, include blockdev-nbd.c and qemu-nbd.texi in the set
of maintained files.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170707182151.29872-1-eblake@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
---
MAINTAINERS | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 4e17216..9916e9a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1348,15 +1348,6 @@ W: http://info.iet.unipi.it/~luigi/netmap/
S: Maintained
F: net/netmap.c
-Network Block Device (NBD)
-M: Paolo Bonzini <pbonzini@redhat.com>
-S: Odd Fixes
-F: block/nbd*
-F: nbd/
-F: include/block/nbd*
-F: qemu-nbd.c
-T: git git://github.com/bonzini/qemu.git nbd-next
-
NUMA
M: Eduardo Habkost <ehabkost@redhat.com>
S: Maintained
@@ -1710,6 +1701,19 @@ S: Supported
F: block/iscsi.c
F: block/iscsi-opts.c
+Network Block Device (NBD)
+M: Eric Blake <eblake@redhat.com>
+M: Paolo Bonzini <pbonzini@redhat.com>
+L: qemu-block@nongnu.org
+S: Maintained
+F: block/nbd*
+F: nbd/
+F: include/block/nbd*
+F: qemu-nbd.*
+F: blockdev-nbd.c
+T: git git://repo.or.cz/qemu/ericb.git nbd
+T: git git://github.com/bonzini/qemu.git nbd-next
+
NFS
M: Jeff Cody <jcody@redhat.com>
M: Peter Lieven <pl@kamp.de>
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PULL 01/11] MAINTAINERS: Promote NBD to supported, with new maintainer
2017-07-10 12:14 ` [Qemu-devel] [PULL 01/11] MAINTAINERS: Promote NBD to supported, with new maintainer Eric Blake
@ 2017-07-10 14:47 ` Eric Blake
0 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 14:47 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]
On 07/10/2017 07:14 AM, Eric Blake wrote:
> We are promising more than just odd fixes, and Paolo is hoping
> to offload the pull requests to me. Also, enough of NBD is related
> to the block layer that it is worth including qemu-block on patches.
>
> While at it, include blockdev-nbd.c and qemu-nbd.texi in the set
> of maintained files.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Message-Id: <20170707182151.29872-1-eblake@redhat.com>
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> MAINTAINERS | 22 +++++++++++++---------
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> +Network Block Device (NBD)
> +M: Eric Blake <eblake@redhat.com>
> +M: Paolo Bonzini <pbonzini@redhat.com>
> +L: qemu-block@nongnu.org
> +S: Maintained
> +F: block/nbd*
> +F: nbd/
> +F: include/block/nbd*
> +F: qemu-nbd.*
> +F: blockdev-nbd.c
> +T: git git://repo.or.cz/qemu/ericb.git nbd
> +T: git git://github.com/bonzini/qemu.git nbd-next
As it is, Paolo's reply crossed mail with my pull request, so on my v2
pull request, I'll drop the second T:
--
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] 15+ messages in thread
* [Qemu-devel] [PULL 02/11] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 01/11] MAINTAINERS: Promote NBD to supported, with new maintainer Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 03/11] nbd/server: refactor nbd_negotiate Eric Blake
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
Message-Id: <20170707152918.23086-2-vsementsov@virtuozzo.com>
Signed-off-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 8a70c05..3963972 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 03/11] nbd/server: refactor nbd_negotiate
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 01/11] MAINTAINERS: Promote NBD to supported, with new maintainer Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 02/11] nbd/server: nbd_negotiate: return 1 on NBD_OPT_ABORT Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 04/11] nbd/server: use errp instead of LOG Eric Blake
` (8 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
Message-Id: <20170707152918.23086-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 3963972..a6a57ce 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);
+
+ 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 (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 {
if (nbd_write(client->ioc, buf, 18, NULL) < 0) {
LOG("write failed");
return -EINVAL;
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 04/11] nbd/server: use errp instead of LOG
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (2 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 03/11] nbd/server: refactor nbd_negotiate Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 05/11] nbd/server: add errp to nbd_send_reply() Eric Blake
` (7 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Move to modern errp scheme from just LOGging errors.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-4-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 266 +++++++++++++++++++++++++++++++++++------------------------
1 file changed, 160 insertions(+), 106 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index a6a57ce..00a7c60 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 05/11] nbd/server: add errp to nbd_send_reply()
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (3 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 04/11] nbd/server: use errp instead of LOG Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 06/11] nbd/common: nbd_tls_handshake: remove extra TRACE Eric Blake
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170707152918.23086-5-vsementsov@virtuozzo.com>
Signed-off-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 00a7c60..e3fd0ca 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 06/11] nbd/common: nbd_tls_handshake: remove extra TRACE
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (4 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 05/11] nbd/server: add errp to nbd_send_reply() Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 07/11] nbd/client: refactor TRACE of NBD_MAGIC Eric Blake
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
Message-Id: <20170707152918.23086-6-vsementsov@virtuozzo.com>
Signed-off-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 6b5c1b7..4dab41e 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 07/11] nbd/client: refactor TRACE of NBD_MAGIC
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (5 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 06/11] nbd/common: nbd_tls_handshake: remove extra TRACE Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 08/11] nbd/server: fix TRACE in nbd_negotiate_send_rep_len Eric Blake
` (4 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
Message-Id: <20170707152918.23086-7-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/client.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index b97143f..2125321 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 08/11] nbd/server: fix TRACE in nbd_negotiate_send_rep_len
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (6 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 07/11] nbd/client: refactor TRACE of NBD_MAGIC Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 09/11] nbd/server: rename clientflags var in nbd_negotiate_options Eric Blake
` (3 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Fix wrong order of TRACE arguments.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-8-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index e3fd0ca..1eeafcc 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 09/11] nbd/server: rename clientflags var in nbd_negotiate_options
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (7 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 08/11] nbd/server: fix TRACE in nbd_negotiate_send_rep_len Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 10/11] nbd: refactor tracing Eric Blake
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
Message-Id: <20170707152918.23086-9-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 1eeafcc..c4d64fb 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 10/11] nbd: refactor tracing
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (8 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 09/11] nbd/server: rename clientflags var in nbd_negotiate_options Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 12:14 ` [Qemu-devel] [PULL 11/11] nbd: use generic trace subsystem instead of TRACE macro Eric Blake
2017-07-10 14:24 ` [Qemu-devel] [PULL 00/11] NBD patches Peter Maydell
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reorganize traces: move, reword, add information, drop extra ones.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20170707152918.23086-10-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.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 2125321..1ae4ff8 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 c4d64fb..4bbce5f 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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 11/11] nbd: use generic trace subsystem instead of TRACE macro
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (9 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 10/11] nbd: refactor tracing Eric Blake
@ 2017-07-10 12:14 ` Eric Blake
2017-07-10 14:24 ` [Qemu-devel] [PULL 00/11] NBD patches Peter Maydell
11 siblings, 0 replies; 15+ messages in thread
From: Eric Blake @ 2017-07-10 12:14 UTC (permalink / raw)
To: qemu-devel
Cc: Vladimir Sementsov-Ogievskiy, Paolo Bonzini,
open list:Network Block Dev...
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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>
Message-Id: <20170707152918.23086-11-vsementsov@virtuozzo.com>
[eblake: minor tweaks to a couple of traces]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/nbd-internal.h | 19 ---------------
nbd/client.c | 69 +++++++++++++++++++++++------------------------------
nbd/server.c | 70 +++++++++++++++++++++++++-----------------------------
Makefile.objs | 1 +
nbd/trace-events | 56 +++++++++++++++++++++++++++++++++++++++++++
5 files changed, 120 insertions(+), 95 deletions(-)
create mode 100644 nbd/trace-events
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 39bfed1..cf6ecbf 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/client.c b/nbd/client.c
index 1ae4ff8..9c52b9b 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,11 @@ 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");
while (1) {
int ret = nbd_receive_list(ioc, wantname, &foundExport, errp);
@@ -362,7 +360,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 +374,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 +398,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 +435,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 +459,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 +471,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_magic(magic);
if (magic == NBD_OPTS_MAGIC) {
uint32_t clientflags = 0;
@@ -486,7 +483,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 +511,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 +576,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 +598,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 +606,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 +614,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 +628,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 +642,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 +652,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 +664,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 +706,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 +752,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/server.c b/nbd/server.c
index 4bbce5f..9b0c588 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,8 +422,9 @@ static int nbd_negotiate_options(NBDClient *client, Error **errp)
error_prepend(errp, "read failed: ");
return -EINVAL;
}
- TRACE("Checking opts 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;
}
@@ -441,7 +442,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 +502,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 +600,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 +632,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 +644,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
}
}
- TRACE("Negotiation succeeded.");
+ trace_nbd_negotiate_success();
return 0;
}
@@ -676,9 +677,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 +693,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 +790,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 +810,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 +999,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 +1040,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 +1082,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 +1119,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/Makefile.objs b/Makefile.objs
index 756644c..3e24c32 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/trace-events b/nbd/trace-events
new file mode 100644
index 0000000..4b233b8
--- /dev/null
+++ b/nbd/trace-events
@@ -0,0 +1,56 @@
+# 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_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_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(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"
+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.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PULL 00/11] NBD patches
2017-07-10 12:14 [Qemu-devel] [PULL 00/11] NBD patches Eric Blake
` (10 preceding siblings ...)
2017-07-10 12:14 ` [Qemu-devel] [PULL 11/11] nbd: use generic trace subsystem instead of TRACE macro Eric Blake
@ 2017-07-10 14:24 ` Peter Maydell
2017-07-10 14:44 ` Eric Blake
11 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2017-07-10 14:24 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU Developers
On 10 July 2017 at 13:14, Eric Blake <eblake@redhat.com> wrote:
> The following changes since commit 77d472291812cf04f97974dadbda767e59e31fde:
>
> Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20170707-tag' into staging (2017-07-10 10:29:11 +0100)
>
> are available in the git repository at:
>
> ericb@repo.or.cz:/srv/git/qemu/ericb.git tags/pull-nbd-2017-07-10
This doesn't look like a valid public-facing git repo URL...
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread