[Libguestfs] [PATCH 1/2] Define .errno_is_preserved constant instead of a .errno_is_reliable callback.
Richard W.M. Jones
rjones at redhat.com
Mon Feb 6 14:57:36 UTC 2017
The callback doesn't make much sense: Could the value change
per-connection? Unlikely. This is a property of the plugin as a
whole.
I changed the name to "errno_is_preserved", because it's not about the
reliability of errno, but about whether errno is preserved across
calls.
---
docs/nbdkit-plugin.pod | 37 +++++++++------------------------
include/nbdkit-plugin.h | 2 +-
plugins/ocaml/nbdkit-ocaml-plugin.pod | 11 ----------
plugins/ocaml/ocaml.c | 11 ----------
plugins/perl/nbdkit-perl-plugin.pod | 6 ------
plugins/perl/perl.c | 12 -----------
plugins/python/nbdkit-python-plugin.pod | 6 ------
plugins/python/python.c | 12 -----------
plugins/ruby/nbdkit-ruby-plugin.pod | 6 ------
plugins/ruby/ruby.c | 12 -----------
src/connections.c | 7 ++-----
src/internal.h | 3 +--
src/plugins.c | 26 +++++++++--------------
13 files changed, 24 insertions(+), 127 deletions(-)
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 3611244..4b364f3 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -147,10 +147,16 @@ C<nbdkit_error> to report an error message; additionally, if the
callback is involved in serving data, the plugin should call
C<nbdkit_set_error> to influence the error code that will be sent to
the client. These two functions can be called in either order. Then,
-the callback should return the appropriate error indication, eg. NULL
-or -1. If the call to C<nbdkit_set_error> is omitted while serving
-data, then the C<.errno_is_reliable> callback controls whether to fall
-back to the value of C<errno> or a hard-coded C<EIO>.
+the callback should return the appropriate error indication,
+eg. C<NULL> or C<-1>.
+
+If the call to C<nbdkit_set_error> is omitted while serving data, then
+the global variable C<errno> may be used. For plugins which have
+C<.errno_is_preserved == 1> the core code will use C<errno>. In
+plugins written in non-C languages, we usually cannot trust that
+C<errno> will not be overwritten when returning from that language to
+C. In that case, either the plugin must call C<nbdkit_set_error> or
+hard-coded C<EIO> is used.
C<nbdkit_error> has the following prototype and works like
L<printf(3)>:
@@ -386,29 +392,6 @@ error message and return C<-1>.
This callback is not required. If omitted, then we return true iff a
C<.trim> callback has been defined.
-=head2 C<.errno_is_reliable>
-
- int errno_is_reliable (void *handle);
-
-This is called during the option negotiation phase to find out what
-errors to reflect to the client. An error explicitly reported to
-C<nbdkit_set_error> takes precedence, but plugins written in C can
-often get by with implicitly using the value stored in C<errno>, when
-this callback returns true. On the other hand, a plugin written in any
-other language generally cannot control the value of C<errno> after
-the glue code of the language binding wrappers; returning false forces
-the use of C<EIO> as the fallback error rather than using an
-unreliable value in C<errno>. This callback is not needed in the
-bindings of other languages. The results of this callback do not
-matter if all data serving callbacks correctly call
-C<nbdkit_set_error>.
-
-If there is an error, C<.errno_is_reliable> should call C<nbdkit_error>
-with an error message and return C<-1>.
-
-This callback is not required. If omitted, then we return true for
-a plugin written in C, and false for plugins in other languages.
-
=head2 C<.pread>
int pread (void *handle, void *buf, uint32_t count, uint64_t offset);
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index 568aaf5..95cba8d 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -92,7 +92,7 @@ struct nbdkit_plugin {
int (*trim) (void *handle, uint32_t count, uint64_t offset);
int (*zero) (void *handle, uint32_t count, uint64_t offset, int may_trim);
- int (*errno_is_reliable) (void *handle);
+ int errno_is_preserved;
/* int (*set_exportname) (void *handle, const char *exportname); */
};
diff --git a/plugins/ocaml/nbdkit-ocaml-plugin.pod b/plugins/ocaml/nbdkit-ocaml-plugin.pod
index 250c1da..8f67d9b 100644
--- a/plugins/ocaml/nbdkit-ocaml-plugin.pod
+++ b/plugins/ocaml/nbdkit-ocaml-plugin.pod
@@ -89,17 +89,6 @@ handle struct in your plugin:
printf "handle ID = %d\n" handle.h_id;
(* ... *)
-=head2 MISSING CALLBACKS
-
-=over 4
-
-=item Missing: C<errno_is_reliable>
-
-This is not needed because the process of gluing OCaml code into C cannot
-reliably use C<errno>.
-
-=back
-
=head2 THREADS
The first parameter of C<NBDKit.register_plugin> is the thread model,
diff --git a/plugins/ocaml/ocaml.c b/plugins/ocaml/ocaml.c
index 5d7aeeb..b38789a 100644
--- a/plugins/ocaml/ocaml.c
+++ b/plugins/ocaml/ocaml.c
@@ -499,16 +499,6 @@ SET(pwrite)
SET(flush)
SET(trim)
-/* We can't guarantee that errno is stable across language binding
- * glue code, so this callback is implemented in C only, and not
- * exposed in OCaml.
- */
-static int
-plugin_ocaml_errno_is_reliable (void *handle)
-{
- return 0;
-}
-
/* We can't directly use NBDKIT_REGISTER_PLUGIN(). */
struct nbdkit_plugin *
plugin_init (void)
@@ -517,6 +507,5 @@ plugin_init (void)
fprintf (stderr, "error: OCaml code did not call NBDKit.register_plugin\n");
exit (EXIT_FAILURE);
}
- plugin.errno_is_reliable = plugin_ocaml_errno_is_reliable;
return &plugin;
}
diff --git a/plugins/perl/nbdkit-perl-plugin.pod b/plugins/perl/nbdkit-perl-plugin.pod
index 920bdef..2ecdcf1 100644
--- a/plugins/perl/nbdkit-perl-plugin.pod
+++ b/plugins/perl/nbdkit-perl-plugin.pod
@@ -327,12 +327,6 @@ C<Nbdkit::set_error(POSIX::EOPNOTSUPP)>.
These are not needed because you can just use regular Perl C<BEGIN>
and C<END> constructs.
-=item Missing: C<errno_is_reliable>
-
-This is not needed because the process of gluing Perl code into C cannot
-reliably use C<errno>. Instead, call C<Nbdkit::set_error> when reporting
-a failure.
-
=item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help>
These are not yet supported.
diff --git a/plugins/perl/perl.c b/plugins/perl/perl.c
index 317a775..d9ad842 100644
--- a/plugins/perl/perl.c
+++ b/plugins/perl/perl.c
@@ -647,16 +647,6 @@ perl_trim (void *handle, uint32_t count, uint64_t offset)
return 0;
}
-/* We can't guarantee that errno is stable across language binding
- * glue code, so this callback is implemented in C only, and not
- * exposed in Perl.
- */
-static int
-perl_errno_is_reliable (void *handle)
-{
- return 0;
-}
-
#define perl_config_help \
"script=<FILENAME> (required) The Perl plugin to run.\n" \
"[other arguments may be used by the plugin that you load]"
@@ -688,8 +678,6 @@ static struct nbdkit_plugin plugin = {
.flush = perl_flush,
.trim = perl_trim,
.zero = perl_zero,
-
- .errno_is_reliable = perl_errno_is_reliable,
};
NBDKIT_REGISTER_PLUGIN(plugin)
diff --git a/plugins/python/nbdkit-python-plugin.pod b/plugins/python/nbdkit-python-plugin.pod
index 1c57e15..90395b9 100644
--- a/plugins/python/nbdkit-python-plugin.pod
+++ b/plugins/python/nbdkit-python-plugin.pod
@@ -246,12 +246,6 @@ use C<nbdkit.set_error(errno.EOPNOTSUPP)>.
These are not needed because you can just use ordinary Python
constructs.
-=item Missing: C<errno_is_reliable>
-
-This is not needed because the process of gluing Python code into C cannot
-reliably use C<errno>. Instead, call C<nbdkit.set_error> when reporting
-a failure.
-
=item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help>
These are not yet supported.
diff --git a/plugins/python/python.c b/plugins/python/python.c
index 895a361..c4ac92d 100644
--- a/plugins/python/python.c
+++ b/plugins/python/python.c
@@ -612,16 +612,6 @@ py_can_trim (void *handle)
return 0;
}
-/* We can't guarantee that errno is stable across language binding
- * glue code, so this callback is implemented in C only, and not
- * exposed in Python.
- */
-static int
-py_errno_is_reliable (void *handle)
-{
- return 0;
-}
-
#define py_config_help \
"script=<FILENAME> (required) The Python plugin to run.\n" \
"[other arguments may be used by the plugin that you load]"
@@ -653,8 +643,6 @@ static struct nbdkit_plugin plugin = {
.flush = py_flush,
.trim = py_trim,
.zero = py_zero,
-
- .errno_is_reliable = py_errno_is_reliable,
};
NBDKIT_REGISTER_PLUGIN(plugin)
diff --git a/plugins/ruby/nbdkit-ruby-plugin.pod b/plugins/ruby/nbdkit-ruby-plugin.pod
index 6cf8e97..980e009 100644
--- a/plugins/ruby/nbdkit-ruby-plugin.pod
+++ b/plugins/ruby/nbdkit-ruby-plugin.pod
@@ -251,12 +251,6 @@ use C<Nbdkit.set_error(Errno::EOPNOTSUPP)>.
These are not needed because you can just use ordinary Ruby
constructs.
-=item Missing: C<errno_is_reliable>
-
-This is not needed because the process of gluing Ruby code into C cannot
-reliably use C<errno>. Instead, call C<Nbdkit.set_error> when reporting
-a failure.
-
=item Missing: C<name>, C<version>, C<longname>, C<description>, C<config_help>
These are not yet supported.
diff --git a/plugins/ruby/ruby.c b/plugins/ruby/ruby.c
index 33d7968..023c1e8 100644
--- a/plugins/ruby/ruby.c
+++ b/plugins/ruby/ruby.c
@@ -467,16 +467,6 @@ plugin_rb_can_trim (void *handle)
return RTEST (rv);
}
-/* We can't guarantee that errno is stable across language binding
- * glue code, so this callback is implemented in C only, and not
- * exposed in Ruby.
- */
-static int
-plugin_rb_errno_is_reliable (void *handle)
-{
- return 0;
-}
-
#define plugin_rb_config_help \
"script=<FILENAME> (required) The Ruby plugin to run.\n" \
"[other arguments may be used by the plugin that you load]"
@@ -511,8 +501,6 @@ static struct nbdkit_plugin plugin = {
.flush = plugin_rb_flush,
.trim = plugin_rb_trim,
.zero = plugin_rb_zero,
-
- .errno_is_reliable = plugin_rb_errno_is_reliable,
};
NBDKIT_REGISTER_PLUGIN(plugin)
diff --git a/src/connections.c b/src/connections.c
index cf06ac5..a0d689a 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -498,10 +498,7 @@ negotiate_handshake (struct connection *conn)
int r;
plugin_lock_request (conn);
- conn->errno_is_reliable = plugin_errno_is_reliable (conn);
- if (conn->errno_is_reliable < 0)
- r = -1;
- else if (!newstyle)
+ if (!newstyle)
r = _negotiate_handshake_oldstyle (conn);
else
r = _negotiate_handshake_newstyle (conn);
@@ -612,7 +609,7 @@ get_error (struct connection *conn)
{
int ret = tls_get_error ();
- if (!ret && conn->errno_is_reliable)
+ if (!ret && plugin_errno_is_preserved ())
ret = errno;
return ret ? ret : EIO;
}
diff --git a/src/internal.h b/src/internal.h
index aa7dbd3..e73edf1 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -120,7 +120,6 @@ struct connection {
int can_flush;
int is_rotational;
int can_trim;
- int errno_is_reliable;
};
extern int handle_single_connection (int sockin, int sockout);
@@ -141,10 +140,10 @@ extern void plugin_lock_connection (void);
extern void plugin_unlock_connection (void);
extern void plugin_lock_request (struct connection *conn);
extern void plugin_unlock_request (struct connection *conn);
+extern int plugin_errno_is_preserved (void);
extern int plugin_open (struct connection *conn, int readonly);
extern void plugin_close (struct connection *conn);
extern int64_t plugin_get_size (struct connection *conn);
-extern int plugin_errno_is_reliable (struct connection *conn);
extern int plugin_can_write (struct connection *conn);
extern int plugin_can_flush (struct connection *conn);
extern int plugin_is_rotational (struct connection *conn);
diff --git a/src/plugins.c b/src/plugins.c
index 837a54f..eeed8a9 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -236,6 +236,7 @@ plugin_dump_fields (void)
break;
}
printf ("\n");
+ printf ("errno_is_preserved=%d\n", plugin.errno_is_preserved);
#define HAS(field) if (plugin.field) printf ("has_%s=1\n", #field)
HAS (longname);
@@ -350,6 +351,14 @@ plugin_unlock_request (struct connection *conn)
}
int
+plugin_errno_is_preserved (void)
+{
+ assert (dl);
+
+ return plugin.errno_is_preserved;
+}
+
+int
plugin_open (struct connection *conn, int readonly)
{
void *handle;
@@ -395,21 +404,6 @@ plugin_get_size (struct connection *conn)
}
int
-plugin_errno_is_reliable (struct connection *conn)
-{
- assert (dl);
- assert (conn->handle);
-
- debug ("errno_is_reliable");
-
- if (plugin.errno_is_reliable)
- return plugin.errno_is_reliable (conn->handle);
-
- /* Default to 1, for backwards compatibility (correct for C plugins) */
- return 1;
-}
-
-int
plugin_can_write (struct connection *conn)
{
assert (dl);
@@ -548,7 +542,7 @@ plugin_zero (struct connection *conn,
result = plugin.zero (conn->handle, count, offset, may_trim);
if (result == -1) {
err = tls_get_error ();
- if (!err && conn->errno_is_reliable)
+ if (!err && plugin_errno_is_preserved ())
err = errno;
}
if (result == 0 || err != EOPNOTSUPP)
--
2.10.2
More information about the Libguestfs
mailing list