Send the last error to the NBD client
resolves: RHEL-50666
This commit is contained in:
parent
bbd9fe8f63
commit
7a43d26862
@ -0,0 +1,51 @@
|
|||||||
|
From 7563596fdb7587dfb8708b5eeeb4b97d738ba79d Mon Sep 17 00:00:00 2001
|
||||||
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
||||||
|
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
|
||||||
|
|
149
0002-server-log-Move-preserve-errno-to-log_verror-functio.patch
Normal file
149
0002-server-log-Move-preserve-errno-to-log_verror-functio.patch
Normal file
@ -0,0 +1,149 @@
|
|||||||
|
From f2c644d4495d5e75883ff729936102c90489e8d8 Mon Sep 17 00:00:00 2001
|
||||||
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
||||||
|
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
|
||||||
|
|
175
0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch
Normal file
175
0003-server-Rename-threadlocal_-set-get-_error-to-._errno.patch
Normal file
@ -0,0 +1,175 @@
|
|||||||
|
From 1d7f655726ad3483d0e8086741182aada7ae8595 Mon Sep 17 00:00:00 2001
|
||||||
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
||||||
|
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
|
||||||
|
|
93
0004-server-Introduce-threadlocal_-set-get-_last_error.patch
Normal file
93
0004-server-Introduce-threadlocal_-set-get-_last_error.patch
Normal file
@ -0,0 +1,93 @@
|
|||||||
|
From fa5055ae2b9f96af941d697de39198c96ee2580a Mon Sep 17 00:00:00 2001
|
||||||
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
||||||
|
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
|
||||||
|
|
@ -0,0 +1,93 @@
|
|||||||
|
From bfa6d4064cb74f429149d14ab4025b258fc95ec4 Mon Sep 17 00:00:00 2001
|
||||||
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
||||||
|
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
|
||||||
|
|
175
0006-server-Send-the-last-error-to-the-NBD-client.patch
Normal file
175
0006-server-Send-the-last-error-to-the-NBD-client.patch
Normal file
@ -0,0 +1,175 @@
|
|||||||
|
From 46484ca8e6a35c45fe96b6c972ceba8984d401e8 Mon Sep 17 00:00:00 2001
|
||||||
|
From: "Richard W.M. Jones" <rjones@redhat.com>
|
||||||
|
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
|
||||||
|
|
@ -6,7 +6,7 @@ set -e
|
|||||||
# directory. Use it like this:
|
# directory. Use it like this:
|
||||||
# ./copy-patches.sh
|
# ./copy-patches.sh
|
||||||
|
|
||||||
rhel_version=8.3
|
rhel_version=10.0
|
||||||
|
|
||||||
# Check we're in the right directory.
|
# Check we're in the right directory.
|
||||||
if [ ! -f nbdkit.spec ]; then
|
if [ ! -f nbdkit.spec ]; then
|
||||||
|
17
nbdkit.spec
17
nbdkit.spec
@ -55,7 +55,7 @@
|
|||||||
|
|
||||||
Name: nbdkit
|
Name: nbdkit
|
||||||
Version: 1.40.0
|
Version: 1.40.0
|
||||||
Release: 1%{?dist}
|
Release: 2%{?dist}
|
||||||
Summary: NBD server
|
Summary: NBD server
|
||||||
|
|
||||||
License: BSD-3-Clause
|
License: BSD-3-Clause
|
||||||
@ -76,6 +76,17 @@ Source2: libguestfs.keyring
|
|||||||
# Maintainer script which helps with handling patches.
|
# Maintainer script which helps with handling patches.
|
||||||
Source3: copy-patches.sh
|
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.
|
# For automatic RPM Provides generation.
|
||||||
# See: https://rpm-software-management.github.io/rpm/manual/dependency_generators.html
|
# See: https://rpm-software-management.github.io/rpm/manual/dependency_generators.html
|
||||||
Source4: nbdkit.attr
|
Source4: nbdkit.attr
|
||||||
@ -1492,9 +1503,11 @@ fi
|
|||||||
|
|
||||||
|
|
||||||
%changelog
|
%changelog
|
||||||
* Mon Jul 22 2024 Richard W.M. Jones <rjones@redhat.com> - 1.40.0-1
|
* Fri Jul 26 2024 Richard W.M. Jones <rjones@redhat.com> - 1.40.0-2
|
||||||
- Rebase to nbdkit 1.40.0
|
- Rebase to nbdkit 1.40.0
|
||||||
related: RHEL-49594
|
related: RHEL-49594
|
||||||
|
- Send the last error to the NBD client
|
||||||
|
resolves: RHEL-50666
|
||||||
|
|
||||||
* Fri Jul 12 2024 Richard W.M. Jones <rjones@redhat.com> - 1.38.0-6
|
* Fri Jul 12 2024 Richard W.M. Jones <rjones@redhat.com> - 1.38.0-6
|
||||||
- Re-add libxcrypt dependency
|
- Re-add libxcrypt dependency
|
||||||
|
Loading…
Reference in New Issue
Block a user