On 01/17/2018 02:53 PM, Richard W.M. Jones wrote:
Introduce the concept of a backend. Currently the only type of backend is a plugin, and there can only be one of them. Instead of calling functions like ‘plugin_pwrite’ you call the backend method ‘backend->pwrite (backend, ...)’.
The change is largely mechanical. I was able to remove ‘assert (dl)’ statements throughout since we can now prove they will never be called.
Note this does not lift the restriction of one plugin per server, and it can *never* do that because plugins can use global variables. --- src/connections.c | 40 +++-- src/internal.h | 55 +++--- src/locks.c | 8 +- src/main.c | 31 ++-- src/plugins.c | 529 ++++++++++++++++++++++++++++++------------------------ 5 files changed, 368 insertions(+), 295 deletions(-)
Hmmm. I'm wondering if we want a src/backend.c,...
- if (plugin_thread_model () < NBDKIT_THREAD_MODEL_PARALLEL || nworkers == 1) + if (backend->thread_model (backend) < NBDKIT_THREAD_MODEL_PARALLEL || + nworkers == 1)
...where we'd call backend_thread_model (backend), and backend.c has: int backend_thread_model (struct backend *b) { return b->thread_model (b); } The real reason I ask is because of:
+++ b/src/plugins.c
+static int +plugin_zero (struct backend *b, struct connection *conn, uint32_t count, uint64_t offset, int may_trim) { - assert (dl); - assert (connection_get_handle (conn)); + struct backend_plugin *p = container_of (b, struct backend_plugin, backend); char *buf; uint32_t limit; int result; int err = 0;
+ assert (connection_get_handle (conn)); + debug ("zero count=%" PRIu32 " offset=%" PRIu64 " may_trim=%d", count, offset, may_trim);
if (!count) return 0; - if (plugin.zero) { + if (p->plugin.zero) { errno = 0; - result = plugin.zero (connection_get_handle (conn), count, offset, may_trim); + result = p->plugin.zero (connection_get_handle (conn), + count, offset, may_trim); if (result == -1) { err = threadlocal_get_error (); - if (!err && plugin_errno_is_preserved ()) + if (!err && plugin_errno_is_preserved (b)) err = errno; } if (result == 0 || err != EOPNOTSUPP) return result; }
- assert (plugin.pwrite); + assert (p->plugin.pwrite); threadlocal_set_error (0); limit = count < MAX_REQUEST_SIZE ? count : MAX_REQUEST_SIZE; buf = calloc (limit, 1); @@ -525,7 +446,8 @@ plugin_zero (struct connection *conn, }
while (count) { - result = plugin.pwrite (connection_get_handle (conn), buf, limit, offset); + result = p->plugin.pwrite (connection_get_handle (conn), + buf, limit, offset);
this fallback. If I leave the EOPNOTSUPP check in plugins.c, then every filter has to duplicate the logic. But with a backend.c, I could do something along these lines (psuedocode, not actually tested): int backend_zero (struct backend *b, struct connection *conn, uint32_t count, uint64_t offset, int may_trim) { try b->zero (b, conn, count, offset, may_trim) if EOPNOTSUPP loop on b->pwrite (b, conn, buf, limit, offset) } coupled with: static int plugin_zero (struct backend *b, struct connection *conn, uint32_t count, uint64_t offset, int may_trim) { if (!p->plugin.zero) { errno = EOPNOTSUPP; return -1; } return p->plugin.zero (connection_get_handle (conn), count, offset, may_trim); } basically, splitting the fallback code that needs to be reusable from the plugin code that just has to call into the actual plugin.so function; this way, filters can also implement .zero to fail with EOPNOTSUPP and trigger the fallback code without having to duplicate things. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org