3:50 p.m.
With some crufty sed scripts we can generate functions that map from
NBD protocol flags (eg. NBD_CMD_READ) to strings ("NBD_CMD_READ").
This works on GNU sed and with FreeBSD, also with GNU sed's --posix
option, so I guess the sed code is POSIX-compatible.
Rich.
3:50 p.m.
New subject: [PATCH nbdkit 1/5] src: Mark synopsis.c as BUILT_SOURCES.
---
src/Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/Makefile.am b/src/Makefile.am
index 3490c0f..65f9498 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -80,6 +80,7 @@ nbdkit_LDFLAGS = \
# synopsis.c is generated from docs/synopsis.txt where it is also
# used to generate the man page. It is included in main.c.
+BUILT_SOURCES = synopsis.c
EXTRA_DIST = synopsis.c
nbdkit_DEPENDENCIES = synopsis.c
CLEANFILES += synopsis.c
--
2.19.0.rc0
7:25 a.m.
New subject: [PATCH nbdkit 1/5] src: Mark synopsis.c as BUILT_SOURCES.
On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
---
src/Makefile.am | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/Makefile.am b/src/Makefile.am
index 3490c0f..65f9498 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -80,6 +80,7 @@ nbdkit_LDFLAGS = \
# synopsis.c is generated from docs/synopsis.txt where it is also
# used to generate the man page. It is included in main.c.
+BUILT_SOURCES = synopsis.c
EXTRA_DIST = synopsis.c
nbdkit_DEPENDENCIES = synopsis.c
CLEANFILES += synopsis.c
Did this make an actual difference during parallel 'make', given that
you already have nbdkit_DEPENDENCIES to trigger normal make
prerequisites? Automake's implementation of BUILT_SOURCES states that
they will be rebuilt even when Makefile itself is out of date (because
configure.ac or Makefile.am was touched, for example), which is really
early in the process (and annoying when trying to use tab completion to
learn what targets make supports, since BUILT_SOURCES are built prior to
computing what output to present). But while it might be annoying, I'm
not opposed to the patch, especially if you did hit a situation in this
series where it mattered.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
8:22 a.m.
New subject: [PATCH nbdkit 1/5] src: Mark synopsis.c as BUILT_SOURCES.
On Fri, Dec 07, 2018 at 07:25:07AM -0600, Eric Blake wrote:
On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
>---
> src/Makefile.am | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/src/Makefile.am b/src/Makefile.am
>index 3490c0f..65f9498 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -80,6 +80,7 @@ nbdkit_LDFLAGS = \
> # synopsis.c is generated from docs/synopsis.txt where it is also
> # used to generate the man page. It is included in main.c.
>+BUILT_SOURCES = synopsis.c
> EXTRA_DIST = synopsis.c
> nbdkit_DEPENDENCIES = synopsis.c
> CLEANFILES += synopsis.c
Did this make an actual difference during parallel 'make', given
that you already have nbdkit_DEPENDENCIES to trigger normal make
prerequisites? Automake's implementation of BUILT_SOURCES states
that they will be rebuilt even when Makefile itself is out of date
(because configure.ac or Makefile.am was touched, for example),
which is really early in the process (and annoying when trying to
use tab completion to learn what targets make supports, since
BUILT_SOURCES are built prior to computing what output to present).
But while it might be annoying, I'm not opposed to the patch,
especially if you did hit a situation in this series where it
mattered.
So I didn't hit any particular problem.
However if my reading of this part of the automake documentation is
right then it is necessary (because we #include "synopsis.c"):
Moreover, all built sources do not necessarily have to be
listed in ‘BUILT_SOURCES’. For instance, a generated ‘.c’ file doesn’t
need to appear in ‘BUILT_SOURCES’ (unless it is included by another
source), because it’s a known dependency of the associated object.
However it's unclear if this is superceded by having the file listed
in *_DEPENDENCIES as you say above.
There's a later patch where I use BUILT_SOURCES for protostrings.c,
where according to that documentation it is _not_ necessary, but seems
like it does no harm either.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
3:50 p.m.
New subject: [PATCH nbdkit 2/5] protocol: Add whitespace to separate NBD_CMD_* and NBD_CMD_FLAG_*.
These are separate fields in the request.
---
src/protocol.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/protocol.h b/src/protocol.h
index 088dcab..0444641 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -154,6 +154,7 @@ struct reply {
#define NBD_CMD_FLUSH 3
#define NBD_CMD_TRIM 4
#define NBD_CMD_WRITE_ZEROES 6
+
#define NBD_CMD_FLAG_FUA (1<<0)
#define NBD_CMD_FLAG_NO_HOLE (1<<1)
--
2.19.0.rc0
3:50 p.m.
New subject: [PATCH nbdkit 3/5] protocol: Generate map functions from NBD protocol flags to printable strings.
This generates small functions which map from various integer NBD
protocol flags to the string equivalent.
eg:
name_of_nbd_cmd (NBD_CMD_READ)
---> "NBD_CMD_READ"
This commit uses some hairy sed scripting to ensure that we don't add
any more dependencies to nbdkit.
---
src/protocol.h | 9 +++++++
.gitignore | 1 +
src/Makefile.am | 17 +++++++++++--
src/protostrings.sed | 59 ++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/src/protocol.h b/src/protocol.h
index 0444641..6709ddc 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -81,10 +81,12 @@ struct fixed_new_option_reply {
#define NBD_REP_MAGIC UINT64_C(0x3e889045565a9)
/* Global flags. */
+extern const char *name_of_nbd_global_flag (int);
#define NBD_FLAG_FIXED_NEWSTYLE 1
#define NBD_FLAG_NO_ZEROES 2
/* Per-export flags. */
+extern const char *name_of_nbd_flag (int);
#define NBD_FLAG_HAS_FLAGS (1 << 0)
#define NBD_FLAG_READ_ONLY (1 << 1)
#define NBD_FLAG_SEND_FLUSH (1 << 2)
@@ -94,6 +96,7 @@ struct fixed_new_option_reply {
#define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6)
/* NBD options (new style handshake only). */
+extern const char *name_of_nbd_opt (int);
#define NBD_OPT_EXPORT_NAME 1
#define NBD_OPT_ABORT 2
#define NBD_OPT_LIST 3
@@ -101,6 +104,7 @@ struct fixed_new_option_reply {
#define NBD_OPT_INFO 6
#define NBD_OPT_GO 7
+extern const char *name_of_nbd_rep (int);
#define NBD_REP_ACK 1
#define NBD_REP_SERVER 2
#define NBD_REP_INFO 3
@@ -110,6 +114,7 @@ struct fixed_new_option_reply {
#define NBD_REP_ERR_PLATFORM 0x80000004
#define NBD_REP_ERR_TLS_REQD 0x80000005
+extern const char *name_of_nbd_info (int);
#define NBD_INFO_EXPORT 0
/* NBD_INFO_EXPORT reply (follows fixed_new_option_reply). */
@@ -148,6 +153,8 @@ struct reply {
#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_REPLY_MAGIC 0x67446698
+/* NBD commands. */
+extern const char *name_of_nbd_cmd (int);
#define NBD_CMD_READ 0
#define NBD_CMD_WRITE 1
#define NBD_CMD_DISC 2 /* Disconnect. */
@@ -155,12 +162,14 @@ struct reply {
#define NBD_CMD_TRIM 4
#define NBD_CMD_WRITE_ZEROES 6
+extern const char *name_of_nbd_cmd_flag (int);
#define NBD_CMD_FLAG_FUA (1<<0)
#define NBD_CMD_FLAG_NO_HOLE (1<<1)
/* Error codes (previously errno).
* See
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd0095b317785b...
*/
+extern const char *name_of_nbd_error (int);
#define NBD_SUCCESS 0
#define NBD_EPERM 1
#define NBD_EIO 5
diff --git a/.gitignore b/.gitignore
index 86ef6cb..8cfe734 100644
--- a/.gitignore
+++ b/.gitignore
@@ -47,6 +47,7 @@ Makefile.in
/podwrapper.pl
/src/nbdkit
/src/nbdkit.pc
+/src/protostrings.c
/src/synopsis.c
/src/test-utils
/stamp-h1
diff --git a/src/Makefile.am b/src/Makefile.am
index 65f9498..1563d74 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -49,6 +49,7 @@ nbdkit_SOURCES = \
options.h \
plugins.c \
protocol.h \
+ protostrings.c \
sockets.c \
threadlocal.c \
utils.c \
@@ -77,11 +78,23 @@ nbdkit_LDFLAGS = \
$(PTHREAD_LIBS) \
$(DL_LDFLAGS)
+# protostrings.c is generated from the protocol.h header file where it
+# is used to map NBD protocol flags to strings.
+
+BUILT_SOURCES = protostrings.c
+EXTRA_DIST = protostrings.c
+CLEANFILES += protostrings.c
+protostrings.c: protocol.h protostrings.sed Makefile
+ rm -f $@ $@-t
+ $(SED) -n -f protostrings.sed < $< > $@-t
+ mv $@-t $@
+ chmod 0444 $@
+
# synopsis.c is generated from docs/synopsis.txt where it is also
# used to generate the man page. It is included in main.c.
-BUILT_SOURCES = synopsis.c
-EXTRA_DIST = synopsis.c
+BUILT_SOURCES += synopsis.c
+EXTRA_DIST += synopsis.c
nbdkit_DEPENDENCIES = synopsis.c
CLEANFILES += synopsis.c
main.c: synopsis.c
diff --git a/src/protostrings.sed b/src/protostrings.sed
new file mode 100644
index 0000000..9a94f75
--- /dev/null
+++ b/src/protostrings.sed
@@ -0,0 +1,59 @@
+# nbdkit
+# Copyright (C) 2018 Red Hat Inc.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Generate the protostrings.c file from protocol.h.
+
+# Prologue.
+1i\
+/* Generated from protocol.h by protostrings.sed. */\
+\#include "protocol.h"\
+
+
+# Match the precise sections of the source file.
+/^extern const char \*name_of_/,/^$/ {
+
+ # Convert extern function prototype into a definition.
+ s/extern \(const char \*name_of_.*\) (int);/\1 (int fl) {\
+ switch (fl) {/;
+
+ # Convert #define lines into cases.
+ s/^#define \([_A-Z]*\).*/ case \1: return "\1\";/;
+
+ # Append closing brace.
+ s/^$/ default: return "unknown";\
+ }\
+}/;
+
+ # Print pattern buffer.
+ p;
+
+}
--
2.19.0.rc0
8:36 a.m.
New subject: [PATCH nbdkit 3/5] protocol: Generate map functions from NBD protocol flags to printable strings.
On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
This generates small functions which map from various integer NBD
protocol flags to the string equivalent.
eg:
name_of_nbd_cmd (NBD_CMD_READ)
---> "NBD_CMD_READ"
This commit uses some hairy sed scripting to ensure that we don't add
any more dependencies to nbdkit.
Hairy, but well-commented. It forces some rather strict formatting to
keep things working, but we touch the file seldom enough that I don't
think that's a problem.
Series looks good to me; I'll probably post a followup to
plugins/nbd/nbd.c to also take advantage of the new functions.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
9:35 a.m.
New subject: [PATCH nbdkit 3/5] protocol: Generate map functions from NBD protocol flags to printable strings.
On Fri, Dec 07, 2018 at 08:36:14AM -0600, Eric Blake wrote:
On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
>This generates small functions which map from various integer NBD
>protocol flags to the string equivalent.
>
>eg:
> name_of_nbd_cmd (NBD_CMD_READ)
> ---> "NBD_CMD_READ"
>
>This commit uses some hairy sed scripting to ensure that we don't add
>any more dependencies to nbdkit.
Hairy, but well-commented. It forces some rather strict formatting
to keep things working, but we touch the file seldom enough that I
don't think that's a problem.
Series looks good to me; I'll probably post a followup to
plugins/nbd/nbd.c to also take advantage of the new functions.
Thanks Eric, pushed it now so please go ahead.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
11:40 a.m.
New subject: [PATCH nbdkit 3/5] protocol: Generate map functions from NBD protocol flags to printable strings.
On 12/6/18 3:50 PM, Richard W.M. Jones wrote:
This generates small functions which map from various integer NBD
protocol flags to the string equivalent.
eg:
name_of_nbd_cmd (NBD_CMD_READ)
---> "NBD_CMD_READ"
This commit uses some hairy sed scripting to ensure that we don't add
any more dependencies to nbdkit.
---
+++ b/src/Makefile.am
@@ -49,6 +49,7 @@ nbdkit_SOURCES = \
options.h \
plugins.c \
protocol.h \
+ protostrings.c \
sockets.c \
threadlocal.c \
utils.c \
@@ -77,11 +78,23 @@ nbdkit_LDFLAGS = \
$(PTHREAD_LIBS) \
$(DL_LDFLAGS)
+# protostrings.c is generated from the protocol.h header file where it
+# is used to map NBD protocol flags to strings.
+
+BUILT_SOURCES = protostrings.c
+EXTRA_DIST = protostrings.c
+CLEANFILES += protostrings.c
+protostrings.c: protocol.h protostrings.sed Makefile
+ rm -f $@ $@-t
+ $(SED) -n -f protostrings.sed < $< > $@-t
'make distcheck' caught a couple issues with this (we want
protostrings.sed in the tarball, and sed can't find the script in VPATH
builds). Now fixed.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
3:50 p.m.
New subject: [PATCH nbdkit 4/5] protocol: Use new ‘name_of_nbd_opt’ function instead of open-coding.
---
src/connections.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/connections.c b/src/connections.c
index 410a893..0f99b67 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -777,7 +777,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
case NBD_OPT_INFO:
case NBD_OPT_GO:
- optname = option == NBD_OPT_INFO ? "NBD_OPT_INFO" :
"NBD_OPT_GO";
+ optname = name_of_nbd_opt (option);
if (conn->recv (conn, data, optlen) == -1) {
nbdkit_error ("read: %m");
return -1;
--
2.19.0.rc0
3:50 p.m.
New subject: [PATCH nbdkit 5/5] protocol: Put the command or option in more debug and error messages.
---
src/connections.c | 53 +++++++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 22 deletions(-)
diff --git a/src/connections.c b/src/connections.c
index 0f99b67..58ed6b0 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -676,14 +676,14 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
switch (option) {
case NBD_OPT_EXPORT_NAME:
if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %m");
+ nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
return -1;
}
/* Apart from printing it, ignore the export name. */
data[optlen] = '\0';
- debug ("newstyle negotiation: NBD_OPT_EXPORT_NAME: "
+ debug ("newstyle negotiation: %s: "
"client requested export '%s' (ignored)",
- data);
+ name_of_nbd_opt (option), data);
/* We have to finish the handshake by sending handshake_finish. */
if (finish_newstyle_options (conn) == -1)
@@ -706,7 +706,8 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
case NBD_OPT_ABORT:
if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1)
return -1;
- debug ("client sent NBD_OPT_ABORT to abort the connection");
+ debug ("client sent %s to abort the connection",
+ name_of_nbd_opt (option));
return -1;
case NBD_OPT_LIST:
@@ -715,14 +716,15 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
== -1)
return -1;
if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %m");
+ nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
return -1;
}
continue;
}
/* Send back the exportname. */
- debug ("newstyle negotiation: advertising export '%s'",
exportname);
+ debug ("newstyle negotiation: %s: advertising export '%s'",
+ name_of_nbd_opt (option), exportname);
if (send_newstyle_option_reply_exportname (conn, option, NBD_REP_SERVER,
exportname) == -1)
return -1;
@@ -737,7 +739,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
== -1)
return -1;
if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %m");
+ nbdkit_error ("read: %s: %m", name_of_nbd_opt (option));
return -1;
}
continue;
@@ -779,7 +781,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
case NBD_OPT_GO:
optname = name_of_nbd_opt (option);
if (conn->recv (conn, data, optlen) == -1) {
- nbdkit_error ("read: %m");
+ nbdkit_error ("read: %s: %m", optname);
return -1;
}
@@ -859,7 +861,8 @@ _negotiate_handshake_newstyle_options (struct connection *conn)
case NBD_INFO_EXPORT: /* ignore - reply sent above */ break;
default:
debug ("newstyle negotiation: %s: "
- "ignoring NBD_INFO_* request %u", optname, (unsigned)
info);
+ "ignoring NBD_INFO_* request %u (%s)",
+ optname, (unsigned) info, name_of_nbd_info (info));
break;
}
}
@@ -979,7 +982,8 @@ validate_request (struct connection *conn,
if (conn->readonly &&
(cmd == NBD_CMD_WRITE || cmd == NBD_CMD_TRIM ||
cmd == NBD_CMD_WRITE_ZEROES)) {
- nbdkit_error ("invalid request: write request on readonly connection");
+ nbdkit_error ("invalid request: %s: write request on readonly connection",
+ name_of_nbd_cmd (cmd));
*error = EROFS;
return false;
}
@@ -992,9 +996,9 @@ validate_request (struct connection *conn,
case NBD_CMD_WRITE_ZEROES:
if (!valid_range (conn, offset, count)) {
/* XXX Allow writes to extend the disk? */
- nbdkit_error ("invalid request: offset and count are out of range: "
+ nbdkit_error ("invalid request: %s: offset and count are out of range: "
"offset=%" PRIu64 " count=%" PRIu32,
- offset, count);
+ name_of_nbd_cmd (cmd), offset, count);
*error = (cmd == NBD_CMD_WRITE ||
cmd == NBD_CMD_WRITE_ZEROES) ? ENOSPC : EINVAL;
return false;
@@ -1003,7 +1007,8 @@ validate_request (struct connection *conn,
case NBD_CMD_FLUSH:
if (offset != 0 || count != 0) {
- nbdkit_error ("invalid flush request: expecting offset and count = 0");
+ nbdkit_error ("invalid request: %s: expecting offset and count = 0",
+ name_of_nbd_cmd (cmd));
*error = EINVAL;
return false;
}
@@ -1037,29 +1042,33 @@ validate_request (struct connection *conn,
/* Refuse over-large read and write requests. */
if ((cmd == NBD_CMD_WRITE || cmd == NBD_CMD_READ) &&
count > MAX_REQUEST_SIZE) {
- nbdkit_error ("invalid request: data request is too large (%" PRIu32
- " > %d)", count, MAX_REQUEST_SIZE);
+ nbdkit_error ("invalid request: %s: data request is too large (%" PRIu32
+ " > %d)",
+ name_of_nbd_cmd (cmd), count, MAX_REQUEST_SIZE);
*error = ENOMEM;
return false;
}
/* Flush allowed? */
if (!conn->can_flush && cmd == NBD_CMD_FLUSH) {
- nbdkit_error ("invalid request: flush operation not supported");
+ nbdkit_error ("invalid request: %s: flush operation not supported",
+ name_of_nbd_cmd (cmd));
*error = EINVAL;
return false;
}
/* Trim allowed? */
if (!conn->can_trim && cmd == NBD_CMD_TRIM) {
- nbdkit_error ("invalid request: trim operation not supported");
+ nbdkit_error ("invalid request: %s: trim operation not supported",
+ name_of_nbd_cmd (cmd));
*error = EINVAL;
return false;
}
/* Zero allowed? */
if (!conn->can_zero && cmd == NBD_CMD_WRITE_ZEROES) {
- nbdkit_error ("invalid request: write zeroes operation not supported");
+ nbdkit_error ("invalid request: %s: write zeroes operation not supported",
+ name_of_nbd_cmd (cmd));
*error = EINVAL;
return false;
}
@@ -1229,7 +1238,7 @@ recv_request_send_reply (struct connection *conn)
count = be32toh (request.count);
if (cmd == NBD_CMD_DISC) {
- debug ("client sent disconnect command, closing connection");
+ debug ("client sent %s, closing connection", name_of_nbd_cmd (cmd));
return set_status (conn, 0); /* disconnect */
}
@@ -1262,7 +1271,7 @@ recv_request_send_reply (struct connection *conn)
r = -1;
}
if (r == -1) {
- nbdkit_error ("read data: %m");
+ nbdkit_error ("read data: %s: %m", name_of_nbd_cmd (cmd));
return set_status (conn, -1);
}
}
@@ -1300,7 +1309,7 @@ recv_request_send_reply (struct connection *conn)
r = conn->send (conn, &reply, sizeof reply);
if (r == -1) {
- nbdkit_error ("write reply: %m");
+ nbdkit_error ("write reply: %s: %m", name_of_nbd_cmd (cmd));
return set_status (conn, -1);
}
@@ -1308,7 +1317,7 @@ recv_request_send_reply (struct connection *conn)
if (cmd == NBD_CMD_READ && !error) {
r = conn->send (conn, buf, count);
if (r == -1) {
- nbdkit_error ("write data: %m");
+ nbdkit_error ("write data: %s: %m", name_of_nbd_cmd (cmd));
return set_status (conn, -1);
}
}
--
2.19.0.rc0
10 a.m.
New subject: [nbdkit PATCH 6/5] nbd: More debug details
Use new 'name_of_*' functions to offer a bit more details about
messages being forwarded on to the remote server.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/nbd/nbd.c | 8 +++++---
plugins/nbd/Makefile.am | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index 672f35b..6bd8861 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -267,8 +267,9 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type,
int r;
pthread_mutex_lock (&h->write_lock);
- nbdkit_debug ("sending request with type %d and cookie %#" PRIx64, type,
- cookie);
+ nbdkit_debug ("sending request type %d (%s), flags %#x, offset %#" PRIx64
+ ", count %#x, cookie %#" PRIx64, type, name_of_nbd_cmd(type),
+ flags, offset, count, cookie);
r = write_full (h->fd, &req, sizeof req);
if (buf && !r)
r = write_full (h->fd, buf, count);
@@ -353,7 +354,8 @@ nbd_reply_raw (struct handle *h, int *fd)
return nbd_mark_dead (h);
if (be32toh (rep.magic) != NBD_REPLY_MAGIC)
return nbd_mark_dead (h);
- nbdkit_debug ("received reply for cookie %#" PRIx64, rep.handle);
+ nbdkit_debug ("received reply for cookie %#" PRIx64 ", status %s",
+ rep.handle, name_of_nbd_error(be32toh (rep.error)));
trans = find_trans_by_cookie (h, rep.handle);
if (!trans) {
nbdkit_error ("reply with unexpected cookie %#" PRIx64, rep.handle);
diff --git a/plugins/nbd/Makefile.am b/plugins/nbd/Makefile.am
index e998a28..9f08057 100644
--- a/plugins/nbd/Makefile.am
+++ b/plugins/nbd/Makefile.am
@@ -38,7 +38,8 @@ plugin_LTLIBRARIES = nbdkit-nbd-plugin.la
nbdkit_nbd_plugin_la_SOURCES = \
nbd.c \
- $(top_srcdir)/include/nbdkit-plugin.h
+ $(top_srcdir)/include/nbdkit-plugin.h \
+ $(top_srcdir)/src/protostrings.c
nbdkit_nbd_plugin_la_CPPFLAGS = \
-I$(top_srcdir)/include \
--
2.17.2
10:12 a.m.
New subject: [nbdkit PATCH 6/5] nbd: More debug details
On Fri, Dec 07, 2018 at 10:00:42AM -0600, Eric Blake wrote:
Use new 'name_of_*' functions to offer a bit more details
about
messages being forwarded on to the remote server.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
plugins/nbd/nbd.c | 8 +++++---
plugins/nbd/Makefile.am | 3 ++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index 672f35b..6bd8861 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -267,8 +267,9 @@ nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type,
int r;
pthread_mutex_lock (&h->write_lock);
- nbdkit_debug ("sending request with type %d and cookie %#" PRIx64, type,
- cookie);
+ nbdkit_debug ("sending request type %d (%s), flags %#x, offset %#" PRIx64
+ ", count %#x, cookie %#" PRIx64, type, name_of_nbd_cmd(type),
+ flags, offset, count, cookie);
r = write_full (h->fd, &req, sizeof req);
if (buf && !r)
r = write_full (h->fd, buf, count);
@@ -353,7 +354,8 @@ nbd_reply_raw (struct handle *h, int *fd)
return nbd_mark_dead (h);
if (be32toh (rep.magic) != NBD_REPLY_MAGIC)
return nbd_mark_dead (h);
- nbdkit_debug ("received reply for cookie %#" PRIx64, rep.handle);
+ nbdkit_debug ("received reply for cookie %#" PRIx64 ", status
%s",
+ rep.handle, name_of_nbd_error(be32toh (rep.error)));
trans = find_trans_by_cookie (h, rep.handle);
if (!trans) {
nbdkit_error ("reply with unexpected cookie %#" PRIx64, rep.handle);
diff --git a/plugins/nbd/Makefile.am b/plugins/nbd/Makefile.am
index e998a28..9f08057 100644
--- a/plugins/nbd/Makefile.am
+++ b/plugins/nbd/Makefile.am
@@ -38,7 +38,8 @@ plugin_LTLIBRARIES = nbdkit-nbd-plugin.la
nbdkit_nbd_plugin_la_SOURCES = \
nbd.c \
- $(top_srcdir)/include/nbdkit-plugin.h
+ $(top_srcdir)/include/nbdkit-plugin.h \
+ $(top_srcdir)/src/protostrings.c
nbdkit_nbd_plugin_la_CPPFLAGS = \
-I$(top_srcdir)/include \
Patch 6/5 had me confused for a while there :-)
ACK.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW
1:38 p.m.
New subject: [nbdkit PATCH 6/5] nbd: More debug details
On 12/7/18 10:12 AM, Richard W.M. Jones wrote:
On Fri, Dec 07, 2018 at 10:00:42AM -0600, Eric Blake wrote:
> Use new 'name_of_*' functions to offer a bit more details about
> messages being forwarded on to the remote server.
>
> Signed-off-by: Eric Blake <eblake(a)redhat.com>
> ---
> plugins/nbd/nbd.c | 8 +++++---
> plugins/nbd/Makefile.am | 3 ++-
> 2 files changed, 7 insertions(+), 4 deletions(-)
> +++ b/plugins/nbd/Makefile.am
> @@ -38,7 +38,8 @@ plugin_LTLIBRARIES = nbdkit-nbd-plugin.la
>
> nbdkit_nbd_plugin_la_SOURCES = \
> nbd.c \
> - $(top_srcdir)/include/nbdkit-plugin.h
> + $(top_srcdir)/include/nbdkit-plugin.h \
> + $(top_srcdir)/src/protostrings.c
>
> nbdkit_nbd_plugin_la_CPPFLAGS = \
> -I$(top_srcdir)/include \
Patch 6/5 had me confused for a while there :-)
ACK.
Now that I've pushed, I'm noticing:
plugins/nbd/Makefile.am:39: warning: source file
'$(top_srcdir)/src/protostrings.c' is in a subdirectory,
plugins/nbd/Makefile.am:39: but option 'subdir-objects' is disabled
automake-1.15: warning: possible forward-incompatibility.
automake-1.15: At least a source file is in a subdirectory, but the
'subdir-objects'
automake-1.15: automake option hasn't been enabled. For now, the
corresponding output
automake-1.15: object file(s) will be placed in the top-level directory.
However,
automake-1.15: this behaviour will change in future Automake versions:
they will
automake-1.15: unconditionally cause object files to be placed in the
same subdirectory
automake-1.15: of the corresponding sources.
automake-1.15: You are advised to start using 'subdir-objects' option
throughout your
automake-1.15: project, to avoid future incompatibilities.
I don't think using subdir-objects is what we want to do, so better may
be to compile protostrings.c under common/protocol as a convenience
library. I'll play with that idea and post patches to see how it looks.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
2208
days inactive
2209
days old
13 comments
2 participants
participants (2)
-
Eric Blake -
Richard W.M. Jones