From 7a43d2686266bf3a7006c7513bbb6c2c053e13b3 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 26 Jul 2024 14:57:44 +0100 Subject: [PATCH] Send the last error to the NBD client resolves: RHEL-50666 --- ...Fix-fallback-functions-used-when-gnu.patch | 51 +++++ ...preserve-errno-to-log_verror-functio.patch | 149 +++++++++++++++ ...readlocal_-set-get-_error-to-._errno.patch | 175 ++++++++++++++++++ ...uce-threadlocal_-set-get-_last_error.patch | 93 ++++++++++ ...read-local-copy-of-the-last-call-to-.patch | 93 ++++++++++ ...end-the-last-error-to-the-NBD-client.patch | 175 ++++++++++++++++++ copy-patches.sh | 2 +- nbdkit.spec | 17 +- 8 files changed, 752 insertions(+), 3 deletions(-) create mode 100644 0001-server-crypto.c-Fix-fallback-functions-used-when-gnu.patch create mode 100644 0002-server-log-Move-preserve-errno-to-log_verror-functio.patch create mode 100644 0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch create mode 100644 0004-server-Introduce-threadlocal_-set-get-_last_error.patch create mode 100644 0005-server-Take-a-thread-local-copy-of-the-last-call-to-.patch create mode 100644 0006-server-Send-the-last-error-to-the-NBD-client.patch diff --git a/0001-server-crypto.c-Fix-fallback-functions-used-when-gnu.patch b/0001-server-crypto.c-Fix-fallback-functions-used-when-gnu.patch new file mode 100644 index 0000000..1ff262b --- /dev/null +++ b/0001-server-crypto.c-Fix-fallback-functions-used-when-gnu.patch @@ -0,0 +1,51 @@ +From 7563596fdb7587dfb8708b5eeeb4b97d738ba79d Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 24 Jul 2024 11:22:28 +0100 +Subject: [PATCH] server/crypto.c: Fix fallback functions used when gnutls is + unavailable +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The return value was wrong resulting in these errors: + +crypto.c: In function ‘nbdkit_peer_tls_dn’: +crypto.c:945:3: error: return makes pointer from integer without a cast [-Werror] + return -1; + ^ +crypto.c: In function ‘nbdkit_peer_tls_issuer_dn’: +crypto.c:953:3: error: return makes pointer from integer without a cast [-Werror] + return -1; + ^ + +Fixes: commit a3759199c93300cf9de24a129c4141609dacb047 +Fixes: commit bf77f76112e3ffbc9149a54fbeb0a505310f28a2 +--- + server/crypto.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/server/crypto.c b/server/crypto.c +index abdc4801..fa408c75 100644 +--- a/server/crypto.c ++++ b/server/crypto.c +@@ -942,7 +942,7 @@ nbdkit_peer_tls_dn (void) + { + nbdkit_error ("%s is not supported on this platform", + "nbdkit_peer_tls_dn"); +- return -1; ++ return NULL; + } + + NBDKIT_DLL_PUBLIC char * +@@ -950,7 +950,7 @@ nbdkit_peer_tls_issuer_dn (void) + { + nbdkit_error ("%s is not supported on this platform", + "nbdkit_peer_tls_issuer_dn"); +- return -1; ++ return NULL; + } + + #endif /* !HAVE_GNUTLS */ +-- +2.43.0 + diff --git a/0002-server-log-Move-preserve-errno-to-log_verror-functio.patch b/0002-server-log-Move-preserve-errno-to-log_verror-functio.patch new file mode 100644 index 0000000..0986656 --- /dev/null +++ b/0002-server-log-Move-preserve-errno-to-log_verror-functio.patch @@ -0,0 +1,149 @@ +From f2c644d4495d5e75883ff729936102c90489e8d8 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 23 Jul 2024 14:46:41 +0100 +Subject: [PATCH] server: log: Move preserve errno to log_verror function + +This neutral code refactoring just moves the place where we preserve +errno out one layer, but should have no other effect. +--- + server/internal.h | 8 ++++---- + server/log-stderr.c | 9 ++------- + server/log-syslog.c | 13 ++++--------- + server/log.c | 12 ++++++++---- + 4 files changed, 18 insertions(+), 24 deletions(-) + +diff --git a/server/internal.h b/server/internal.h +index 0b0507e4..1a783e3f 100644 +--- a/server/internal.h ++++ b/server/internal.h +@@ -340,10 +340,10 @@ extern void free_debug_flags (void); + extern void log_verror (const char *fs, va_list args); + + /* log-*.c */ +-extern void log_stderr_verror (const char *fs, va_list args) +- ATTRIBUTE_FORMAT_PRINTF (1, 0); +-extern void log_syslog_verror (const char *fs, va_list args) +- ATTRIBUTE_FORMAT_PRINTF (1, 0); ++extern void log_stderr_verror (int orig_errno, const char *fs, va_list args) ++ ATTRIBUTE_FORMAT_PRINTF (2, 0); ++extern void log_syslog_verror (int orig_errno, const char *fs, va_list args) ++ ATTRIBUTE_FORMAT_PRINTF (2, 0); + + /* vfprintf.c */ + #if !HAVE_VFPRINTF_PERCENT_M +diff --git a/server/log-stderr.c b/server/log-stderr.c +index 8a55f5df..4d8b09da 100644 +--- a/server/log-stderr.c ++++ b/server/log-stderr.c +@@ -43,12 +43,9 @@ + + #include "internal.h" + +-/* Note: preserves the previous value of errno. */ + void +-log_stderr_verror (const char *fs, va_list args) ++log_stderr_verror (int orig_errno, const char *fs, va_list args) + { +- int err = errno; /* must be first line of function */ +- + const char *name = threadlocal_get_name (); + size_t instance_num = threadlocal_get_instance_num (); + int tty; +@@ -69,7 +66,7 @@ log_stderr_verror (const char *fs, va_list args) + } + + fprintf (stderr, "error: "); +- errno = err; /* must restore in case fs contains %m */ ++ errno = orig_errno; /* must restore in case fs contains %m */ + vfprintf (stderr, fs, args); + fprintf (stderr, "\n"); + +@@ -78,6 +75,4 @@ log_stderr_verror (const char *fs, va_list args) + #ifdef HAVE_FUNLOCKFILE + funlockfile (stderr); + #endif +- +- errno = err; /* must be last line of function */ + } +diff --git a/server/log-syslog.c b/server/log-syslog.c +index 76c5035b..29a7a825 100644 +--- a/server/log-syslog.c ++++ b/server/log-syslog.c +@@ -45,11 +45,9 @@ + /* Tempted to use LOG_FTP instead of LOG_DAEMON! */ + static const int PRIORITY = LOG_DAEMON|LOG_ERR; + +-/* Note: preserves the previous value of errno. */ + void +-log_syslog_verror (const char *fs, va_list args) ++log_syslog_verror (int orig_errno, const char *fs, va_list args) + { +- int err = errno; + const char *name = threadlocal_get_name (); + size_t instance_num = threadlocal_get_instance_num (); + CLEANUP_FREE char *msg = NULL; +@@ -59,9 +57,9 @@ log_syslog_verror (const char *fs, va_list args) + fp = open_memstream (&msg, &len); + if (fp == NULL) { + /* Fallback to logging using fs, args directly. */ +- errno = err; /* Must restore in case fs contains %m */ ++ errno = orig_errno; /* must restore in case fs contains %m */ + vsyslog (PRIORITY, fs, args); +- goto out; ++ return; + } + + if (name) { +@@ -71,12 +69,9 @@ log_syslog_verror (const char *fs, va_list args) + fprintf (fp, ": "); + } + +- errno = err; /* Must restore in case fs contains %m */ ++ errno = orig_errno; /* must restore in case fs contains %m */ + vfprintf (fp, fs, args); + close_memstream (fp); + + syslog (PRIORITY, "%s", msg); +- +- out: +- errno = err; + } +diff --git a/server/log.c b/server/log.c +index 464e4f9a..9c1f667a 100644 +--- a/server/log.c ++++ b/server/log.c +@@ -46,23 +46,27 @@ + void + log_verror (const char *fs, va_list args) + { ++ int orig_errno = errno; ++ + switch (log_to) { + case LOG_TO_DEFAULT: + if (forked_into_background) +- log_syslog_verror (fs, args); ++ log_syslog_verror (orig_errno, fs, args); + else +- log_stderr_verror (fs, args); ++ log_stderr_verror (orig_errno, fs, args); + break; + case LOG_TO_SYSLOG: +- log_syslog_verror (fs, args); ++ log_syslog_verror (orig_errno, fs, args); + break; + case LOG_TO_STDERR: +- log_stderr_verror (fs, args); ++ log_stderr_verror (orig_errno, fs, args); + break; + case LOG_TO_NULL: + /* nothing */ + break; + } ++ ++ errno = orig_errno; /* Restore errno before leaving the function. */ + } + + /* Note: preserves the previous value of errno. */ +-- +2.43.0 + diff --git a/0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch b/0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch new file mode 100644 index 0000000..3e8468f --- /dev/null +++ b/0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch @@ -0,0 +1,175 @@ +From 1d7f655726ad3483d0e8086741182aada7ae8595 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 24 Jul 2024 10:29:13 +0100 +Subject: [PATCH] server: Rename threadlocal_{set,get}_error to .._errno + +A simple mechanical change, to avoid confusion with +threadlocal_{set,get}_last_error introduced in the following commit. +--- + server/internal.h | 4 ++-- + server/plugins.c | 27 +++++++++++++-------------- + server/protocol.c | 5 +++-- + server/threadlocal.c | 4 ++-- + 4 files changed, 20 insertions(+), 20 deletions(-) + +diff --git a/server/internal.h b/server/internal.h +index 1a783e3f..8102ccde 100644 +--- a/server/internal.h ++++ b/server/internal.h +@@ -569,8 +569,8 @@ extern void threadlocal_set_name (const char *name) + extern const char *threadlocal_get_name (void); + extern void threadlocal_set_instance_num (size_t instance_num); + extern size_t threadlocal_get_instance_num (void); +-extern void threadlocal_set_error (int err); +-extern int threadlocal_get_error (void); ++extern void threadlocal_set_errno (int err); ++extern int threadlocal_get_errno (void); + extern void *threadlocal_buffer (size_t size); + extern void threadlocal_set_conn (struct connection *conn); + extern struct connection *threadlocal_get_conn (void); +diff --git a/server/plugins.c b/server/plugins.c +index ca89ac7a..3c7df0d2 100644 +--- a/server/plugins.c ++++ b/server/plugins.c +@@ -633,15 +633,14 @@ plugin_can_cache (struct context *c) + NBDKIT_DLL_PUBLIC void + nbdkit_set_error (int err) + { +- threadlocal_set_error (err); ++ threadlocal_set_errno (err); + } + +-/* Grab the appropriate error value. +- */ ++/* Grab the appropriate error value. */ + static int +-get_error (struct backend_plugin *p) ++get_errno (struct backend_plugin *p) + { +- int ret = threadlocal_get_error (); ++ int ret = threadlocal_get_errno (); + + if (!ret && p->plugin.errno_is_preserved != 0) + ret = errno; +@@ -664,7 +663,7 @@ plugin_pread (struct context *c, + else + r = p->plugin._pread_v1 (c->handle, buf, count, offset); + if (r == -1) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +@@ -685,7 +684,7 @@ plugin_flush (struct context *c, + return -1; + } + if (r == -1) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +@@ -715,7 +714,7 @@ plugin_pwrite (struct context *c, + if (r != -1 && need_flush) + r = plugin_flush (c, 0, err); + if (r == -1 && !*err) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +@@ -744,7 +743,7 @@ plugin_trim (struct context *c, + if (r != -1 && need_flush) + r = plugin_flush (c, 0, err); + if (r == -1 && !*err) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +@@ -782,7 +781,7 @@ plugin_zero (struct context *c, + else + emulate = true; + if (r == -1) +- *err = emulate ? EOPNOTSUPP : get_error (p); ++ *err = emulate ? EOPNOTSUPP : get_errno (p); + if (r == 0 || (*err != EOPNOTSUPP && *err != ENOTSUP)) + goto done; + } +@@ -794,7 +793,7 @@ plugin_zero (struct context *c, + } + + flags &= ~NBDKIT_FLAG_MAY_TRIM; +- threadlocal_set_error (0); ++ threadlocal_set_errno (0); + *err = 0; + + while (count) { +@@ -814,7 +813,7 @@ plugin_zero (struct context *c, + if (r != -1 && need_flush) + r = plugin_flush (c, 0, err); + if (r == -1 && !*err) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +@@ -839,7 +838,7 @@ plugin_extents (struct context *c, + r = -1; + } + if (r == -1) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +@@ -859,7 +858,7 @@ plugin_cache (struct context *c, + + r = p->plugin.cache (c->handle, count, offset, flags); + if (r == -1) +- *err = get_error (p); ++ *err = get_errno (p); + return r; + } + +diff --git a/server/protocol.c b/server/protocol.c +index 9b63f789..677da05c 100644 +--- a/server/protocol.c ++++ b/server/protocol.c +@@ -235,8 +235,9 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, + int err = 0; + + /* Clear the error, so that we know if the plugin calls +- * nbdkit_set_error() or relied on errno. */ +- threadlocal_set_error (0); ++ * nbdkit_set_error() or relied on errno. ++ */ ++ threadlocal_set_errno (0); + + switch (cmd) { + case NBD_CMD_READ: +diff --git a/server/threadlocal.c b/server/threadlocal.c +index 088fe55a..9bb656bc 100644 +--- a/server/threadlocal.c ++++ b/server/threadlocal.c +@@ -154,7 +154,7 @@ threadlocal_get_instance_num (void) + } + + void +-threadlocal_set_error (int err) ++threadlocal_set_errno (int err) + { + struct threadlocal *threadlocal = pthread_getspecific (threadlocal_key); + +@@ -167,7 +167,7 @@ threadlocal_set_error (int err) + /* This preserves errno, for convenience. + */ + int +-threadlocal_get_error (void) ++threadlocal_get_errno (void) + { + int err = errno; + struct threadlocal *threadlocal = pthread_getspecific (threadlocal_key); +-- +2.43.0 + diff --git a/0004-server-Introduce-threadlocal_-set-get-_last_error.patch b/0004-server-Introduce-threadlocal_-set-get-_last_error.patch new file mode 100644 index 0000000..2443cc7 --- /dev/null +++ b/0004-server-Introduce-threadlocal_-set-get-_last_error.patch @@ -0,0 +1,93 @@ +From fa5055ae2b9f96af941d697de39198c96ee2580a Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Wed, 24 Jul 2024 10:37:58 +0100 +Subject: [PATCH] server: Introduce threadlocal_{set,get}_last_error + +Plus a function to clear the last_error field. +--- + server/internal.h | 3 +++ + server/threadlocal.c | 40 ++++++++++++++++++++++++++++++++++++++++ + 2 files changed, 43 insertions(+) + +diff --git a/server/internal.h b/server/internal.h +index 8102ccde..c45384a6 100644 +--- a/server/internal.h ++++ b/server/internal.h +@@ -571,6 +571,9 @@ extern void threadlocal_set_instance_num (size_t instance_num); + extern size_t threadlocal_get_instance_num (void); + extern void threadlocal_set_errno (int err); + extern int threadlocal_get_errno (void); ++extern void threadlocal_set_last_error (char *msg); ++extern void threadlocal_clear_last_error (void); ++extern const char *threadlocal_get_last_error (void); + extern void *threadlocal_buffer (size_t size); + extern void threadlocal_set_conn (struct connection *conn); + extern struct connection *threadlocal_get_conn (void); +diff --git a/server/threadlocal.c b/server/threadlocal.c +index 9bb656bc..74a3c4e5 100644 +--- a/server/threadlocal.c ++++ b/server/threadlocal.c +@@ -56,6 +56,7 @@ struct threadlocal { + char *name; /* Can be NULL. */ + size_t instance_num; /* Can be 0. */ + int err; ++ char *last_error; /* Can be NULL. */ + void *buffer; /* Can be NULL. */ + size_t buffer_size; + struct connection *conn; /* Can be NULL. */ +@@ -70,6 +71,7 @@ free_threadlocal (void *threadlocalv) + struct threadlocal *threadlocal = threadlocalv; + + free (threadlocal->name); ++ free (threadlocal->last_error); + free (threadlocal->buffer); + free (threadlocal); + } +@@ -176,6 +178,44 @@ threadlocal_get_errno (void) + return threadlocal ? threadlocal->err : 0; + } + ++/* Set the last_error field. The ownership of the 'msg' string is ++ * passed to the threadlocal and will be freed here. ++ */ ++void ++threadlocal_set_last_error (char *msg) ++{ ++ struct threadlocal *threadlocal = pthread_getspecific (threadlocal_key); ++ ++ if (threadlocal) { ++ free (threadlocal->last_error); ++ threadlocal->last_error = msg; ++ } ++ else { ++ /* ... otherwise throw it away, it's informational. */ ++ free (msg); ++ } ++} ++ ++void ++threadlocal_clear_last_error (void) ++{ ++ threadlocal_set_last_error (NULL); ++} ++ ++/* Get the last_error field. If successful, this returns a non-NULL ++ * string. This is valid until something calls nbdkit_error() in the ++ * same thread, so it should be used quickly. Returning NULL is not ++ * necessarily an error. The last_error is informational and may not ++ * be available. ++ */ ++const char * ++threadlocal_get_last_error (void) ++{ ++ struct threadlocal *threadlocal = pthread_getspecific (threadlocal_key); ++ ++ return threadlocal ? threadlocal->last_error : NULL; ++} ++ + /* Return the single pread/pwrite buffer for this thread. The buffer + * size is increased to ‘size’ bytes if required. + * +-- +2.43.0 + diff --git a/0005-server-Take-a-thread-local-copy-of-the-last-call-to-.patch b/0005-server-Take-a-thread-local-copy-of-the-last-call-to-.patch new file mode 100644 index 0000000..485a520 --- /dev/null +++ b/0005-server-Take-a-thread-local-copy-of-the-last-call-to-.patch @@ -0,0 +1,93 @@ +From bfa6d4064cb74f429149d14ab4025b258fc95ec4 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 23 Jul 2024 15:28:06 +0100 +Subject: [PATCH] server: Take a thread-local copy of the last call to + nbdkit_error + +nbdkit_error has traditionally been a "fancy wrapper around fprintf" +(kind of, don't take that literally). It is encouraged that plugins +and filters do something like: + + if (error) { + nbdkit_error ("oops, a bad thing happened"); + return -1; + } + +but we don't enforce this. Plugins might call nbdkit_error more than +once or not at all. + +The point where we get to sending an error back over the wire to the +NBD client is long after the plugin returned above, and after +nbdkit_error was called. + +Therefore in order to send errors back to the NBD client, we must keep +the last error message around. + +This change simply modifies nbdkit_error to make a best-effort attempt +to save the last error message in thread-local storage. + +We also clear the last error when a new request starts, to ensure that +we don't leak errors across different callbacks or connections. +--- + server/log.c | 21 +++++++++++++++++++++ + server/protocol.c | 5 +++++ + 2 files changed, 26 insertions(+) + +diff --git a/server/log.c b/server/log.c +index 9c1f667a..acf14d57 100644 +--- a/server/log.c ++++ b/server/log.c +@@ -40,6 +40,25 @@ + + #include "internal.h" + ++/* Copy the error message to threadlocal. This is sent to callers ++ * which are using structured replies, but is for extra information ++ * only so don't fail if we are unable to copy it. ++ */ ++static void ++copy_error_to_threadlocal (int orig_errno, const char *fs, va_list args) ++{ ++ va_list args_copy; ++ char *msg; ++ int r; ++ ++ va_copy (args_copy, args); ++ errno = orig_errno; /* must restore in case fs contains %m */ ++ r = vasprintf (&msg, fs, args_copy); ++ va_end (args_copy); ++ if (r != -1 && msg) ++ threadlocal_set_last_error (msg); /* ownership passed to threadlocal */ ++} ++ + /* Call the right log_*_verror function depending on log_sink. + * Note: preserves the previous value of errno. + */ +@@ -48,6 +67,8 @@ log_verror (const char *fs, va_list args) + { + int orig_errno = errno; + ++ copy_error_to_threadlocal (orig_errno, fs, args); ++ + switch (log_to) { + case LOG_TO_DEFAULT: + if (forked_into_background) +diff --git a/server/protocol.c b/server/protocol.c +index 677da05c..d428bfc8 100644 +--- a/server/protocol.c ++++ b/server/protocol.c +@@ -239,6 +239,11 @@ handle_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count, + */ + threadlocal_set_errno (0); + ++ /* Also clear the last error in this thread so we will only save ++ * nbdkit_error() from this request. ++ */ ++ threadlocal_clear_last_error (); ++ + switch (cmd) { + case NBD_CMD_READ: + if (backend_pread (c, buf, count, offset, 0, &err) == -1) +-- +2.43.0 + diff --git a/0006-server-Send-the-last-error-to-the-NBD-client.patch b/0006-server-Send-the-last-error-to-the-NBD-client.patch new file mode 100644 index 0000000..18b221d --- /dev/null +++ b/0006-server-Send-the-last-error-to-the-NBD-client.patch @@ -0,0 +1,175 @@ +From 46484ca8e6a35c45fe96b6c972ceba8984d401e8 Mon Sep 17 00:00:00 2001 +From: "Richard W.M. Jones" +Date: Tue, 23 Jul 2024 15:45:04 +0100 +Subject: [PATCH] server: Send the last error to the NBD client + +This sends the last error saved in the connection handle back to the +NBD client. This is informational and best effort. + +qemu reports the error already, for example: + + $ nbdkit --log=null \ + eval open=' echo EPERM Go Away >&2; exit 1 ' get_size=' echo 100 ' \ + --run 'qemu-img info "$uri"' + qemu-img: Could not open 'nbd+unix://?socket=/tmp/nbdkitIDl6iy/socket': Requested export not available + server reported: /tmp/nbdkitRDAfXH/open: Go Away + +This goes back to at least qemu 2.12.0 (RHEL 7) and possibly earlier, +so we can just assume that qemu does this for the test. + +libnbd requires a patch to display this information. +--- + server/protocol-handshake-newstyle.c | 43 ++++++++++++++++------ + tests/Makefile.am | 2 + + tests/test-last-error.sh | 55 ++++++++++++++++++++++++++++ + 3 files changed, 88 insertions(+), 12 deletions(-) + create mode 100755 tests/test-last-error.sh + +diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c +index 6b3bc76f..c18d32e5 100644 +--- a/server/protocol-handshake-newstyle.c ++++ b/server/protocol-handshake-newstyle.c +@@ -57,28 +57,47 @@ send_newstyle_option_reply (uint32_t option, uint32_t reply) + { + GET_CONN; + struct nbd_fixed_new_option_reply fixed_new_option_reply; ++ const char *last_error = NULL; ++ uint32_t replylen = 0; ++ ++ if (NBD_REP_IS_ERR (reply)) { ++ last_error = threadlocal_get_last_error (); ++ /* Note that calling nbdkit_error will invalidate last_error, so ++ * be careful below. ++ */ ++ if (last_error) { ++ size_t len = strlen (last_error); ++ if (len <= NBD_MAX_STRING) ++ replylen = len; ++ } ++ } + + fixed_new_option_reply.magic = htobe64 (NBD_REP_MAGIC); + fixed_new_option_reply.option = htobe32 (option); + fixed_new_option_reply.reply = htobe32 (reply); +- fixed_new_option_reply.replylen = htobe32 (0); ++ fixed_new_option_reply.replylen = htobe32 (replylen); + + debug ("replying to %s with %s", name_of_nbd_opt (option), + name_of_nbd_rep (reply)); + if (conn->send (&fixed_new_option_reply, +- sizeof fixed_new_option_reply, 0) == -1) { +- /* The protocol document says that the client is allowed to simply +- * drop the connection after sending NBD_OPT_ABORT, or may read +- * the reply. +- */ +- if (option == NBD_OPT_ABORT) +- debug ("write: %s: %m", name_of_nbd_opt (option)); +- else +- nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); +- return -1; +- } ++ sizeof fixed_new_option_reply, ++ replylen > 0 ? SEND_MORE : 0) == -1) ++ goto err; ++ if (replylen > 0 && conn->send (last_error, replylen, 0) == -1) ++ goto err; + + return 0; ++ ++err: ++ /* The protocol document says that the client is allowed to simply ++ * drop the connection after sending NBD_OPT_ABORT, or may read ++ * the reply. ++ */ ++ if (option == NBD_OPT_ABORT) ++ debug ("write: %s: %m", name_of_nbd_opt (option)); ++ else ++ nbdkit_error ("write: %s: %m", name_of_nbd_opt (option)); ++ return -1; + } + + /* Reply to NBD_OPT_LIST with the plugin's list of export names. +diff --git a/tests/Makefile.am b/tests/Makefile.am +index 8c7d6b8c..89c5fa9d 100644 +--- a/tests/Makefile.am ++++ b/tests/Makefile.am +@@ -292,6 +292,7 @@ TESTS += \ + test-read-password-interactive.sh \ + test-nbd-client.sh \ + test-nbd-client-tls.sh \ ++ test-last-error.sh \ + $(NULL) + if !IS_WINDOWS + TESTS += \ +@@ -324,6 +325,7 @@ EXTRA_DIST += \ + test-help-plugin.sh \ + test-ipv4-lo.sh \ + test-ipv6-lo.sh \ ++ test-last-error.sh \ + test-long-name.sh \ + test-nbd-client-tls.sh \ + test-nbd-client.sh \ +diff --git a/tests/test-last-error.sh b/tests/test-last-error.sh +new file mode 100755 +index 00000000..fc720606 +--- /dev/null ++++ b/tests/test-last-error.sh +@@ -0,0 +1,55 @@ ++#!/usr/bin/env bash ++# nbdkit ++# Copyright Red Hat ++# ++# 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. ++ ++source ./functions.sh ++set -e ++set -x ++ ++# Test informational error messages sent to the NBD client. ++# qemu-img supports this since at least 2.12.0. ++ ++requires_run ++requires_plugin eval ++requires qemu-img --version ++ ++out=last-error.out ++rm -f $out ++cleanup_fn rm -f $out ++ ++export out ++ ++nbdkit eval \ ++ open=' echo EPERM Go Away >&2; exit 1 ' get_size=' echo 0 ' \ ++ --run ' qemu-img info "$uri" > $out 2>&1 ||: ' ++cat $out ++ ++grep "Go Away" $out +-- +2.43.0 + diff --git a/copy-patches.sh b/copy-patches.sh index 2682a17..cfa8a5b 100755 --- a/copy-patches.sh +++ b/copy-patches.sh @@ -6,7 +6,7 @@ set -e # directory. Use it like this: # ./copy-patches.sh -rhel_version=8.3 +rhel_version=10.0 # Check we're in the right directory. if [ ! -f nbdkit.spec ]; then diff --git a/nbdkit.spec b/nbdkit.spec index 61c9c09..78ed7f3 100644 --- a/nbdkit.spec +++ b/nbdkit.spec @@ -55,7 +55,7 @@ Name: nbdkit Version: 1.40.0 -Release: 1%{?dist} +Release: 2%{?dist} Summary: NBD server License: BSD-3-Clause @@ -76,6 +76,17 @@ Source2: libguestfs.keyring # Maintainer script which helps with handling patches. Source3: copy-patches.sh +# Patches come from the upstream repository: +# https://gitlab.com/nbdkit/nbdkit/-/commits/rhel-10.0/ + +# Patches. +Patch0001: 0001-server-crypto.c-Fix-fallback-functions-used-when-gnu.patch +Patch0002: 0002-server-log-Move-preserve-errno-to-log_verror-functio.patch +Patch0003: 0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch +Patch0004: 0004-server-Introduce-threadlocal_-set-get-_last_error.patch +Patch0005: 0005-server-Take-a-thread-local-copy-of-the-last-call-to-.patch +Patch0006: 0006-server-Send-the-last-error-to-the-NBD-client.patch + # For automatic RPM Provides generation. # See: https://rpm-software-management.github.io/rpm/manual/dependency_generators.html Source4: nbdkit.attr @@ -1492,9 +1503,11 @@ fi %changelog -* Mon Jul 22 2024 Richard W.M. Jones - 1.40.0-1 +* Fri Jul 26 2024 Richard W.M. Jones - 1.40.0-2 - Rebase to nbdkit 1.40.0 related: RHEL-49594 +- Send the last error to the NBD client + resolves: RHEL-50666 * Fri Jul 12 2024 Richard W.M. Jones - 1.38.0-6 - Re-add libxcrypt dependency