Calling plugin_flush(b, conn, 0, err) instead of the longer
p->plugin.flush(connection_get_handle(conn, 0) has an unfortunate side
effect: if .can_fua is emulate, and the client requested FUA, there is
a possibility that a failed attempt to flush has set neither the
threadlocal error nor errno to a sane value (the most obvious case is
when the plugin lacks .flush, and directly sets *err to EINVAL).
However, when the caller detects that flush failed, it blindly assigns
*err = get_error(p), which will default to EIO if nothing else can be
located in threadlocal or errno.
In practice, this corner case requires a custom plugin to hit: the sh
plugin seemed like the most obvious candidate, but it provides a
.flush callback that reliably sets errno on failure (so the subsequent
call to get_error(p) happens to reset *err to the value it already
had). Other bindings like Python haven't even been converted to
version 2 API yet, which means .can_fua can only be set indirectly
based on .flush existing.
But it's easy enough to prevent: after calling any other plugin_*
function fails, only change *err if it is not already set; in turn
this means we don't have to mess with errno after plugin_pwrite fails
during .zero fallback.
Fixes: 20db811e
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
server/plugins.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/server/plugins.c b/server/plugins.c
index be952504..88a41044 100644
--- a/server/plugins.c
+++ b/server/plugins.c
@@ -599,7 +599,7 @@ plugin_pwrite (struct backend *b, struct connection *conn,
}
if (r != -1 && need_flush)
r = plugin_flush (b, conn, 0, err);
- if (r == -1)
+ if (r == -1 && !*err)
*err = get_error (p);
return r;
}
@@ -633,7 +633,7 @@ plugin_trim (struct backend *b, struct connection *conn,
}
if (r != -1 && need_flush)
r = plugin_flush (b, conn, 0, err);
- if (r == -1)
+ if (r == -1 && !*err)
*err = get_error (p);
return r;
}
@@ -700,13 +700,10 @@ plugin_zero (struct backend *b, struct connection *conn,
count -= limit;
}
- *err = errno;
- errno = *err;
-
done:
if (r != -1 && need_flush)
r = plugin_flush (b, conn, 0, err);
- if (r == -1)
+ if (r == -1 && !*err)
*err = get_error (p);
return r;
}
--
2.20.1