This partially reverts the refactoring work of 91023f269d4 (server: Remove explicit connection parameter, use TLS instead); but differs in that it only needs struct handle* instead of the larger struct connection*. This patch lays the groundwork for an upcoming patch to add the multi-conn filter, which wants to operate on more than one handle into a plugin from a single connection from the client. While most callers will continue to get handle from TLS, now filters.c will be able to go back to storing the handle per nxdata opaque pointer passed to the filter. --- server/internal.h | 59 ++++++++++++++++++- server/backend.c | 147 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 183 insertions(+), 23 deletions(-) diff --git a/server/internal.h b/server/internal.h index 906f0690..b999b856 100644 --- a/server/internal.h +++ b/server/internal.h @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2021 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -441,8 +441,6 @@ extern int backend_list_exports (struct backend *b, int readonly, __attribute__((__nonnull__ (1, 3))); extern const char *backend_default_export (struct backend *b, int readonly) __attribute__((__nonnull__ (1))); -extern const char *backend_export_description (struct backend *b) - __attribute__((__nonnull__ (1))); /* exportname is only valid for this call and almost certainly will be * freed on return of this function, so backends must save the * exportname if they need to refer to it later. @@ -465,54 +463,109 @@ extern int backend_reopen (struct backend *b, __attribute__((__nonnull__ (1, 3))); extern int64_t backend_get_size (struct backend *b) __attribute__((__nonnull__ (1))); +extern int64_t backend_handle_get_size (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); +extern const char *backend_export_description (struct backend *b) + __attribute__((__nonnull__ (1))); +extern const char *backend_handle_export_description (struct backend *b, + struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_write (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_write (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_flush (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_flush (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_is_rotational (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_is_rotational (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_trim (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_trim (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_zero (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_zero (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_fast_zero (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_fast_zero (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_extents (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_extents (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_fua (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_fua (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_multi_conn (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_multi_conn (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_can_cache (struct backend *b) __attribute__((__nonnull__ (1))); +extern int backend_handle_can_cache (struct backend *b, struct handle *h) + __attribute__((__nonnull__ (1, 2))); extern int backend_pread (struct backend *b, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) __attribute__((__nonnull__ (1, 2, 6))); +extern int backend_handle_pread (struct backend *b, struct handle *h, + void *buf, uint32_t count, uint64_t offset, + uint32_t flags, int *err) + __attribute__((__nonnull__ (1, 2, 3, 7))); extern int backend_pwrite (struct backend *b, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) __attribute__((__nonnull__ (1, 2, 6))); +extern int backend_handle_pwrite (struct backend *b, struct handle *h, + const void *buf, uint32_t count, + uint64_t offset, uint32_t flags, int *err) + __attribute__((__nonnull__ (1, 2, 3, 7))); extern int backend_flush (struct backend *b, uint32_t flags, int *err) __attribute__((__nonnull__ (1, 3))); +extern int backend_handle_flush (struct backend *b, struct handle *h, + uint32_t flags, int *err) + __attribute__((__nonnull__ (1, 2, 4))); extern int backend_trim (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) __attribute__((__nonnull__ (1, 5))); +extern int backend_handle_trim (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, + uint32_t flags, int *err) + __attribute__((__nonnull__ (1, 2, 6))); extern int backend_zero (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) __attribute__((__nonnull__ (1, 5))); +extern int backend_handle_zero (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, + uint32_t flags, int *err) + __attribute__((__nonnull__ (1, 2, 6))); extern int backend_extents (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) __attribute__((__nonnull__ (1, 5, 6))); +extern int backend_handle_extents (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, + uint32_t flags, + struct nbdkit_extents *extents, int *err) + __attribute__((__nonnull__ (1, 2, 6, 7))); extern int backend_cache (struct backend *b, uint32_t count, uint64_t offset, uint32_t flags, int *err) __attribute__((__nonnull__ (1, 5))); +extern int backend_handle_cache (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, + uint32_t flags, int *err) + __attribute__((__nonnull__ (1, 2, 6))); /* plugins.c */ extern struct backend *plugin_register (size_t index, const char *filename, diff --git a/server/backend.c b/server/backend.c index 3630163b..2b42ff9c 100644 --- a/server/backend.c +++ b/server/backend.c @@ -1,5 +1,5 @@ /* nbdkit - * Copyright (C) 2013-2020 Red Hat Inc. + * Copyright (C) 2013-2021 Red Hat Inc. * * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are @@ -364,7 +364,12 @@ const char * backend_export_description (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_export_description (b, get_handle (conn, b->i)); +} + +const char * +backend_handle_export_description (struct backend *b, struct handle *h) +{ const char *s; controlpath_debug ("%s: export_description", b->name); @@ -386,8 +391,12 @@ int64_t backend_get_size (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_get_size (b, get_handle (conn, b->i)); +} +int64_t +backend_handle_get_size (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->exportsize == -1) { controlpath_debug ("%s: get_size", b->name); @@ -400,8 +409,12 @@ int backend_can_write (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_write (b, get_handle (conn, b->i)); +} +int +backend_handle_can_write (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_write == -1) { controlpath_debug ("%s: can_write", b->name); @@ -414,8 +427,12 @@ int backend_can_flush (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_flush (b, get_handle (conn, b->i)); +} +int +backend_handle_can_flush (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_flush == -1) { controlpath_debug ("%s: can_flush", b->name); @@ -428,8 +445,12 @@ int backend_is_rotational (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_is_rotational (b, get_handle (conn, b->i)); +} +int +backend_handle_is_rotational (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->is_rotational == -1) { controlpath_debug ("%s: is_rotational", b->name); @@ -442,7 +463,12 @@ int backend_can_trim (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_trim (b, get_handle (conn, b->i)); +} + +int +backend_handle_can_trim (struct backend *b, struct handle *h) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -462,7 +488,12 @@ int backend_can_zero (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_zero (b, get_handle (conn, b->i)); +} + +int +backend_handle_can_zero (struct backend *b, struct handle *h) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -482,7 +513,12 @@ int backend_can_fast_zero (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_fast_zero (b, get_handle (conn, b->i)); +} + +int +backend_handle_can_fast_zero (struct backend *b, struct handle *h) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -502,8 +538,12 @@ int backend_can_extents (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_extents (b, get_handle (conn, b->i)); +} +int +backend_handle_can_extents (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_extents == -1) { controlpath_debug ("%s: can_extents", b->name); @@ -516,7 +556,12 @@ int backend_can_fua (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_fua (b, get_handle (conn, b->i)); +} + +int +backend_handle_can_fua (struct backend *b, struct handle *h) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -536,8 +581,12 @@ int backend_can_multi_conn (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_multi_conn (b, get_handle (conn, b->i)); +} +int +backend_handle_can_multi_conn (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_multi_conn == -1) { controlpath_debug ("%s: can_multi_conn", b->name); @@ -550,8 +599,12 @@ int backend_can_cache (struct backend *b) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_can_cache (b, get_handle (conn, b->i)); +} +int +backend_handle_can_cache (struct backend *b, struct handle *h) +{ assert (h->handle && (h->state & HANDLE_CONNECTED)); if (h->can_cache == -1) { controlpath_debug ("%s: can_cache", b->name); @@ -566,7 +619,15 @@ backend_pread (struct backend *b, uint32_t flags, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_pread (b, get_handle (conn, b->i), buf, count, offset, + flags, err); +} + +int +backend_handle_pread (struct backend *b, struct handle *h, + void *buf, uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -587,7 +648,15 @@ backend_pwrite (struct backend *b, uint32_t flags, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_pwrite (b, get_handle (conn, b->i), buf, count, + offset, flags, err); +} + +int +backend_handle_pwrite (struct backend *b, struct handle *h, + const void *buf, uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ bool fua = !!(flags & NBDKIT_FLAG_FUA); int r; @@ -611,7 +680,13 @@ backend_flush (struct backend *b, uint32_t flags, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_flush (b, get_handle (conn, b->i), flags, err); +} + +int +backend_handle_flush (struct backend *b, struct handle *h, + uint32_t flags, int *err) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -631,7 +706,15 @@ backend_trim (struct backend *b, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_trim (b, get_handle (conn, b->i), count, offset, + flags, err); +} + +int +backend_handle_trim (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, uint32_t flags, + int *err) +{ bool fua = !!(flags & NBDKIT_FLAG_FUA); int r; @@ -657,7 +740,15 @@ backend_zero (struct backend *b, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_zero (b, get_handle (conn, b->i), count, offset, + flags, err); +} + +int +backend_handle_zero (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, uint32_t flags, + int *err) +{ bool fua = !!(flags & NBDKIT_FLAG_FUA); bool fast = !!(flags & NBDKIT_FLAG_FAST_ZERO); int r; @@ -692,7 +783,15 @@ backend_extents (struct backend *b, struct nbdkit_extents *extents, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_extents (b, get_handle (conn, b->i), count, offset, + flags, extents, err); +} + +int +backend_handle_extents (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, uint32_t flags, + struct nbdkit_extents *extents, int *err) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); @@ -723,7 +822,15 @@ backend_cache (struct backend *b, uint32_t flags, int *err) { GET_CONN; - struct handle *h = get_handle (conn, b->i); + return backend_handle_cache (b, get_handle (conn, b->i), count, offset, + flags, err); +} + +int +backend_handle_cache (struct backend *b, struct handle *h, + uint32_t count, uint64_t offset, + uint32_t flags, int *err) +{ int r; assert (h->handle && (h->state & HANDLE_CONNECTED)); -- 2.30.1

This reverts commit fdbebe389f15bebbda8bf20c81043013396b66f5. This has no semantic impact, but will allow the next patch to expand the contents of b_h with one more field needed to make it possible to implement a multi-conn filter. The reversion is a bit more complex than undoing the original because of new callbacks added in the meantime, but it is still fairly mechanical. --- server/filters.c | 221 +++++++++++++++++++++++++++++++---------------- 1 file changed, 146 insertions(+), 75 deletions(-) diff --git a/server/filters.c b/server/filters.c index f4dbfea7..3e79ef5e 100644 --- a/server/filters.c +++ b/server/filters.c @@ -49,6 +49,16 @@ struct backend_filter { struct nbdkit_filter filter; }; +/* Literally a backend and the filter's handle. + * + * This is the implementation of our handle in .open, and serves as + * a stable ‘void *nxdata’ in the filter API. + */ +struct b_h { + struct backend *b; + void *handle; +}; + /* Note this frees the whole chain. */ static void filter_free (struct backend *b) @@ -288,9 +298,8 @@ filter_default_export (struct backend *b, int readonly, int is_tls) static int next_open (void *nxdata, int readonly, const char *exportname) { - struct backend *b_next = nxdata; - - return backend_open (b_next, readonly, exportname); + struct b_h *b_h = nxdata; + return backend_open (b_h->b, readonly, exportname); } static void * @@ -298,180 +307,200 @@ filter_open (struct backend *b, int readonly, const char *exportname, int is_tls) { struct backend_filter *f = container_of (b, struct backend_filter, backend); - void *handle; + struct b_h *nxdata = calloc (1, sizeof *nxdata); + + if (!nxdata) { + nbdkit_error ("malloc: %m"); + return NULL; + } + + nxdata->b = b->next; /* Most filters will call next_open first, resulting in * inner-to-outer ordering. */ if (f->filter.open) - handle = f->filter.open (next_open, b->next, readonly, exportname, - is_tls); + nxdata->handle = f->filter.open (next_open, nxdata, readonly, exportname, + is_tls); else if (backend_open (b->next, readonly, exportname) == -1) - handle = NULL; + nxdata->handle = NULL; else - handle = NBDKIT_HANDLE_NOT_NEEDED; - return handle; + nxdata->handle = NBDKIT_HANDLE_NOT_NEEDED; + if (nxdata->handle == NULL) { + free (nxdata); + return NULL; + } + + return nxdata; } static void filter_close (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; - if (handle && f->filter.close) - f->filter.close (handle); + if (handle && f->filter.close) { + assert (nxdata->b == b->next); + f->filter.close (nxdata->handle); + } + free (nxdata); } /* The next_functions structure contains pointers to backend - * functions. These are only needed for type safety (nxdata is void - * pointer, backend_* functions expect a struct backend * parameter). - * nxdata is a pointer to the next backend in the linked list. + * functions. However because these functions are all expecting a + * backend and a handle, we cannot call them directly, but must + * write some next_* functions that unpack the two parameters from a + * single ‘void *nxdata’ struct pointer (‘b_h’). */ static int next_reopen (void *nxdata, int readonly, const char *exportname) { - struct backend *b_next = nxdata; - return backend_reopen (b_next, readonly, exportname); + struct b_h *b_h = nxdata; + return backend_reopen (b_h->b, readonly, exportname); } static int64_t next_get_size (void *nxdata) { - struct backend *b_next = nxdata; - return backend_get_size (b_next); + struct b_h *b_h = nxdata; + return backend_get_size (b_h->b); } static const char * next_export_description (void *nxdata) { - struct backend *b_next = nxdata; - return backend_export_description (b_next); + struct b_h *b_h = nxdata; + return backend_export_description (b_h->b); } static int next_can_write (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_write (b_next); + struct b_h *b_h = nxdata; + return backend_can_write (b_h->b); } static int next_can_flush (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_flush (b_next); + struct b_h *b_h = nxdata; + return backend_can_flush (b_h->b); } static int next_is_rotational (void *nxdata) { - struct backend *b_next = nxdata; - return backend_is_rotational (b_next); + struct b_h *b_h = nxdata; + return backend_is_rotational (b_h->b); } static int next_can_trim (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_trim (b_next); + struct b_h *b_h = nxdata; + return backend_can_trim (b_h->b); } static int next_can_zero (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_zero (b_next); + struct b_h *b_h = nxdata; + return backend_can_zero (b_h->b); } static int next_can_fast_zero (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_fast_zero (b_next); + struct b_h *b_h = nxdata; + return backend_can_fast_zero (b_h->b); } static int next_can_extents (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_extents (b_next); + struct b_h *b_h = nxdata; + return backend_can_extents (b_h->b); } static int next_can_fua (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_fua (b_next); + struct b_h *b_h = nxdata; + return backend_can_fua (b_h->b); } static int next_can_multi_conn (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_multi_conn (b_next); + struct b_h *b_h = nxdata; + return backend_can_multi_conn (b_h->b); } static int next_can_cache (void *nxdata) { - struct backend *b_next = nxdata; - return backend_can_cache (b_next); + struct b_h *b_h = nxdata; + return backend_can_cache (b_h->b); } static int next_pread (void *nxdata, void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct backend *b_next = nxdata; - return backend_pread (b_next, buf, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_pread (b_h->b, buf, count, offset, flags, + err); } static int next_pwrite (void *nxdata, const void *buf, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct backend *b_next = nxdata; - return backend_pwrite (b_next, buf, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_pwrite (b_h->b, buf, count, offset, flags, + err); } static int next_flush (void *nxdata, uint32_t flags, int *err) { - struct backend *b_next = nxdata; - return backend_flush (b_next, flags, err); + struct b_h *b_h = nxdata; + return backend_flush (b_h->b, flags, err); } static int next_trim (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct backend *b_next = nxdata; - return backend_trim (b_next, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_trim (b_h->b, count, offset, flags, err); } static int next_zero (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct backend *b_next = nxdata; - return backend_zero (b_next, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_zero (b_h->b, count, offset, flags, err); } static int next_extents (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents *extents, int *err) { - struct backend *b_next = nxdata; - return backend_extents (b_next, count, offset, flags, extents, err); + struct b_h *b_h = nxdata; + return backend_extents (b_h->b, count, offset, flags, + extents, err); } static int next_cache (void *nxdata, uint32_t count, uint64_t offset, uint32_t flags, int *err) { - struct backend *b_next = nxdata; - return backend_cache (b_next, count, offset, flags, err); + struct b_h *b_h = nxdata; + return backend_cache (b_h->b, count, offset, flags, err); } static struct nbdkit_next_ops next_ops = { @@ -501,9 +530,11 @@ static int filter_prepare (struct backend *b, void *handle, int readonly) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.prepare && - f->filter.prepare (&next_ops, b->next, handle, readonly) == -1) + f->filter.prepare (&next_ops, nxdata, nxdata->handle, readonly) == -1) return -1; return 0; @@ -513,9 +544,11 @@ static int filter_finalize (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.finalize && - f->filter.finalize (&next_ops, b->next, handle) == -1) + f->filter.finalize (&next_ops, nxdata, nxdata->handle) == -1) return -1; return 0; } @@ -524,9 +557,11 @@ static const char * filter_export_description (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.export_description) - return f->filter.export_description (&next_ops, b->next, handle); + return f->filter.export_description (&next_ops, nxdata, nxdata->handle); else return backend_export_description (b->next); } @@ -535,9 +570,11 @@ static int64_t filter_get_size (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.get_size) - return f->filter.get_size (&next_ops, b->next, handle); + return f->filter.get_size (&next_ops, nxdata, nxdata->handle); else return backend_get_size (b->next); } @@ -546,9 +583,11 @@ static int filter_can_write (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_write) - return f->filter.can_write (&next_ops, b->next, handle); + return f->filter.can_write (&next_ops, nxdata, nxdata->handle); else return backend_can_write (b->next); } @@ -557,9 +596,11 @@ static int filter_can_flush (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_flush) - return f->filter.can_flush (&next_ops, b->next, handle); + return f->filter.can_flush (&next_ops, nxdata, nxdata->handle); else return backend_can_flush (b->next); } @@ -568,9 +609,11 @@ static int filter_is_rotational (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.is_rotational) - return f->filter.is_rotational (&next_ops, b->next, handle); + return f->filter.is_rotational (&next_ops, nxdata, nxdata->handle); else return backend_is_rotational (b->next); } @@ -579,9 +622,11 @@ static int filter_can_trim (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_trim) - return f->filter.can_trim (&next_ops, b->next, handle); + return f->filter.can_trim (&next_ops, nxdata, nxdata->handle); else return backend_can_trim (b->next); } @@ -590,9 +635,11 @@ static int filter_can_zero (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_zero) - return f->filter.can_zero (&next_ops, b->next, handle); + return f->filter.can_zero (&next_ops, nxdata, nxdata->handle); else return backend_can_zero (b->next); } @@ -601,9 +648,11 @@ static int filter_can_fast_zero (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_fast_zero) - return f->filter.can_fast_zero (&next_ops, b->next, handle); + return f->filter.can_fast_zero (&next_ops, nxdata, nxdata->handle); else return backend_can_fast_zero (b->next); } @@ -612,9 +661,11 @@ static int filter_can_extents (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_extents) - return f->filter.can_extents (&next_ops, b->next, handle); + return f->filter.can_extents (&next_ops, nxdata, nxdata->handle); else return backend_can_extents (b->next); } @@ -623,9 +674,11 @@ static int filter_can_fua (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_fua) - return f->filter.can_fua (&next_ops, b->next, handle); + return f->filter.can_fua (&next_ops, nxdata, nxdata->handle); else return backend_can_fua (b->next); } @@ -634,9 +687,11 @@ static int filter_can_multi_conn (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_multi_conn) - return f->filter.can_multi_conn (&next_ops, b->next, handle); + return f->filter.can_multi_conn (&next_ops, nxdata, nxdata->handle); else return backend_can_multi_conn (b->next); } @@ -645,9 +700,11 @@ static int filter_can_cache (struct backend *b, void *handle) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.can_cache) - return f->filter.can_cache (&next_ops, b->next, handle); + return f->filter.can_cache (&next_ops, nxdata, nxdata->handle); else return backend_can_cache (b->next); } @@ -658,9 +715,11 @@ filter_pread (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.pread) - return f->filter.pread (&next_ops, b->next, handle, + return f->filter.pread (&next_ops, nxdata, nxdata->handle, buf, count, offset, flags, err); else return backend_pread (b->next, buf, count, offset, flags, err); @@ -672,9 +731,11 @@ filter_pwrite (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.pwrite) - return f->filter.pwrite (&next_ops, b->next, handle, + return f->filter.pwrite (&next_ops, nxdata, nxdata->handle, buf, count, offset, flags, err); else return backend_pwrite (b->next, buf, count, offset, flags, err); @@ -685,9 +746,11 @@ filter_flush (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.flush) - return f->filter.flush (&next_ops, b->next, handle, flags, err); + return f->filter.flush (&next_ops, nxdata, nxdata->handle, flags, err); else return backend_flush (b->next, flags, err); } @@ -698,9 +761,11 @@ filter_trim (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.trim) - return f->filter.trim (&next_ops, b->next, handle, count, offset, + return f->filter.trim (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else return backend_trim (b->next, count, offset, flags, err); @@ -711,9 +776,11 @@ filter_zero (struct backend *b, void *handle, uint32_t count, uint64_t offset, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.zero) - return f->filter.zero (&next_ops, b->next, handle, + return f->filter.zero (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else return backend_zero (b->next, count, offset, flags, err); @@ -725,9 +792,11 @@ filter_extents (struct backend *b, void *handle, struct nbdkit_extents *extents, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.extents) - return f->filter.extents (&next_ops, b->next, handle, + return f->filter.extents (&next_ops, nxdata, nxdata->handle, count, offset, flags, extents, err); else @@ -741,9 +810,11 @@ filter_cache (struct backend *b, void *handle, uint32_t flags, int *err) { struct backend_filter *f = container_of (b, struct backend_filter, backend); + struct b_h *nxdata = handle; + assert (nxdata->b == b->next); if (f->filter.cache) - return f->filter.cache (&next_ops, b->next, handle, + return f->filter.cache (&next_ops, nxdata, nxdata->handle, count, offset, flags, err); else return backend_cache (b->next, count, offset, flags, err); -- 2.30.1

Implement a TODO item of emulating multi-connection consistency via multiple plugin flush calls to allow a client to assume that a flush on a single connection is good enough. This also gives us some fine-tuning over whether to advertise the bit, including some setups that are unsafe but may be useful in timing tests. Testing is interesting: I used the sh plugin to implement a server that intentionally keeps a per-connection cache. Note that this filter assumes that multiple connections will still share the same data (other than caching effects); effects are not guaranteed when trying to mix it with more exotic plugins like info that violate that premise. --- filters/cache/nbdkit-cache-filter.pod | 5 +- filters/fua/nbdkit-fua-filter.pod | 7 + .../multi-conn/nbdkit-multi-conn-filter.pod | 178 +++++++ filters/nocache/nbdkit-nocache-filter.pod | 1 + filters/noextents/nbdkit-noextents-filter.pod | 1 + .../noparallel/nbdkit-noparallel-filter.pod | 1 + filters/nozero/nbdkit-nozero-filter.pod | 1 + configure.ac | 4 +- filters/multi-conn/Makefile.am | 68 +++ tests/Makefile.am | 11 +- filters/multi-conn/multi-conn.c | 443 ++++++++++++++++++ tests/test-multi-conn-plugin.sh | 122 +++++ tests/test-multi-conn.sh | 293 ++++++++++++ TODO | 7 - 14 files changed, 1132 insertions(+), 10 deletions(-) create mode 100644 filters/multi-conn/nbdkit-multi-conn-filter.pod create mode 100644 filters/multi-conn/Makefile.am create mode 100644 filters/multi-conn/multi-conn.c create mode 100755 tests/test-multi-conn-plugin.sh create mode 100755 tests/test-multi-conn.sh diff --git a/filters/cache/nbdkit-cache-filter.pod b/filters/cache/nbdkit-cache-filter.pod index 34fd0b29..482a9c55 100644 --- a/filters/cache/nbdkit-cache-filter.pod +++ b/filters/cache/nbdkit-cache-filter.pod @@ -41,7 +41,9 @@ an explicit flush is done by the client. This is the default caching mode, and is safe if your client issues flush requests correctly (which is true for modern Linux and other -well-written NBD clients). +well-written NBD clients). Note that this mode is able to advertise +multi-connection consistency even without the use of +L<nbdkit-multi-conn-filter(1)>. =item B<cache=writethrough> @@ -162,6 +164,7 @@ L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-cacheextents-filter(1)>, L<nbdkit-readahead-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-filter(3)>, L<qemu-img(1)>. diff --git a/filters/fua/nbdkit-fua-filter.pod b/filters/fua/nbdkit-fua-filter.pod index 0f9b9744..9b0d84b5 100644 --- a/filters/fua/nbdkit-fua-filter.pod +++ b/filters/fua/nbdkit-fua-filter.pod @@ -16,6 +16,12 @@ This filter can be used to disable FUA and flush requests for speed server fallbacks, and for evaluating timing differences between proper use of FUA compared to a full flush. +Note that by default, the NBD protocol does not guarantee that the use +of FUA from one connection will be visible from another connection +unless the server advertised NBD_FLAG_MULTI_CONN. You may wish to +combine this filter with L<nbdkit-multi-conn-filter(1)> if you plan on +making multiple connections to the plugin. + =head1 PARAMETERS The C<fuamode> parameter is optional and controls which mode the @@ -137,6 +143,7 @@ L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-blocksize-filter(1)>, L<nbdkit-log-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noextents-filter(1)>, L<nbdkit-noparallel-filter(1)>, diff --git a/filters/multi-conn/nbdkit-multi-conn-filter.pod b/filters/multi-conn/nbdkit-multi-conn-filter.pod new file mode 100644 index 00000000..d624a6fb --- /dev/null +++ b/filters/multi-conn/nbdkit-multi-conn-filter.pod @@ -0,0 +1,178 @@ +=head1 NAME + +nbdkit-multi-conn-filter - nbdkit multi-conn filter + +=head1 SYNOPSIS + + nbdkit --filter=multi-conn plugin \ + [multi-conn-mode=MODE] [multi-conn-track-dirty=LEVEL] [plugin-args...] + +=head1 DESCRIPTION + +C<nbdkit-multi-conn-filter> is a filter that enables alterations to +the server's advertisement of NBD_FLAG_MULTI_CONN. When a server +permits multiple simultaneous clients, and sets this flag, a client +may assume that all connections see a consistent view (after getting +the server reply from a write in one connection, sending a flush +command on a single connection and waiting for that reply then +guarantees that all connections will then see the just-written data). +If the flag is not advertised, a client must presume that separate +connections may have utilized independent caches, and where a flush on +one connection does not affect the cache of a second connection. + +The main use of this filter is to emulate consistent semantics across +multiple connections when not already provided by a plugin, although +it also has additional modes useful for evaluating performance and +correctness of client and plugin multi-conn behaviors. This filter +assumes that multiple connections to a plugin will eventually share +data, other than any caching effects; it is not suitable for use with +a plugin that produces completely independent data per connection. + +Additional control over the behavior of client flush commands is +possible by combining this filter with L<nbdkit-fua-filter(1)>. Note +that L<nbdkit-cache-filter(1)> is also able to provide +multi-connection consistency, but at the expense of an extra layer of +caching not needed with this filter. + +=head1 PARAMETERS + +=over 4 + +=item B<multi-conn-mode=auto> + +This filter defaults to B<auto> mode. If the selected thread model is +SERIALIZE_CONNECTIONS, then this filter behaves the same as B<disable> +mode; if the plugin advertises multi-conn, then this filter behaves +the same as B<plugin> mode; otherwise, this filter behaves the same as +B<emulate> mode. As a result, this mode advertises +NBD_FLAG_MULTI_CONN to the client exactly when the server supports +multiple simultaneous connections. + +=item B<multi-conn-mode=emulate> + +When B<emulate> mode is chosen, then this filter tracks all parallel +connections. When a client issues a flush command over any one +connection (including an implied flush by a write command with the FUA +(force unit access) flag set), the filter then replicates that flush +across each connection to the plugin (although the amount of plugin +calls can be tuned by adjusting B<multi-conn-track-dirty>). This +assumes that flushing each connection is enough to clear any +per-connection cached data, in order to give each connection a +consistent view of the image; therefore, this mode advertises +NBD_FLAG_MULTI_CONN to the client. + +Note that in this mode, a client will be unable to connect if the +plugin lacks support for flush, as there would be no way to emulate +cross-connection consistency. + +=item B<multi-conn-mode=disable> + +When B<disable> mode is chosen, this filter disables advertisement of +NBD_FLAG_MULTI_CONN to the client, even if the plugin supports it, and +does not replicate flush commands across connections. This is useful +for testing whether a client with multiple connections properly sends +multiple flushes in order to overcome per-connection caching. + +=item B<multi-conn-mode=plugin> + +When B<plugin> mode is chosen, the filter does not change whether +NBD_FLAG_MULTI_CONN is advertised by the plugin, and does not +replicate flush commands across connections; but still honors +B<multi-conn-track-dirty> for minimizing the number of flush commands +passed on to the plugin. + +=item B<multi-conn-mode=unsafe> + +When B<unsafe> mode is chosen, this filter blindly advertises +NBD_FLAG_MULTI_CONN to the client even if the plugin lacks support. +This is dangerous, and risks data corruption if the client makes +assumptions about data consistency that were not actually met. + +=item B<multi-conn-track-dirty=fast> + +When dirty tracking is set to B<fast>, the filter tracks whether any +connection has caused the image to be dirty (any write, zero, or trim +commands since the last flush, regardless of connection); if all +connections are clean, a client flush command is ignored rather than +sent on to the plugin. In this mode, a flush action on one connection +marks all other connections as clean, regardless of whether the filter +actually advertised NBD_FLAG_MULTI_CONN, which can result in less +activity when a client sends multiple flushes rather than taking +advantage of multi-conn semantics. This is safe with +B<multi-conn-mode=emulate>, but potentially unsafe with +B<multi-conn-mode=plugin> when the plugin did not advertise +multi-conn, as it does not track whether a read may have cached stale +data prior to a flush. + +=item B<multi-conn-track-dirty=connection> + +Dirty tracking is set to B<connection> by default, where the filter +tracks whether a given connection is dirty (any write, zero, or trim +commands since the last flush on the given connection, and any read +since the last flush on any other connection); if the connection is +clean, a flush command to that connection (whether directly from the +client, or replicated by B<multi-conn-mode=emulate> is ignored rather +than sent on to the plugin. This mode may result in more flush calls +than B<multi-conn-track-dirty=fast>, but in turn is safe to use with +B<multi-conn-mode=plugin>. + +=item B<multi-conn-track-dirty=off> + +When dirty tracking is set to B<off>, all flush commands from the +client are passed on to the plugin, regardless of whether the flush +would be needed for consistency. Note that when combined with +B<multi-conn-mode=emulate>, a client which disregards +NBD_FLAG_MULTI_CONN by flushing on each connection itself results in a +quadratic number of flush operations on the plugin. + +=back + +=head1 EXAMPLES + +Provide consistent cross-connection flush semantics on top of a plugin +that lacks it natively: + + nbdkit --filter=multi-conn vddk /absolute/path/to/file.vmdk + +Minimize the number of expensive flush operations performed when +utilizing a plugin that has multi-conn consistency from a client that +blindly flushes across every connection: + + nbdkit --filter=multi-conn file multi-conn-mode=plugin \ + multi-conn-track-dirty=fast disk.img + +=head1 FILES + +=over 4 + +=item F<$filterdir/nbdkit-multi-conn-filter.so> + +The filter. + +Use C<nbdkit --dump-config> to find the location of C<$filterdir>. + +=back + +=head1 VERSION + +C<nbdkit-multi-conn-filter> first appeared in nbdkit 1.26. + +=head1 SEE ALSO + +L<nbdkit(1)>, +L<nbdkit-file-plugin(1)>, +L<nbdkit-filter(3)>, +L<nbdkit-cache-filter(1)>, +L<nbdkit-fua-filter(1)>, +L<nbdkit-nocache-filter(1)>, +L<nbdkit-noextents-filter(1)>, +L<nbdkit-noparallel-filter(1)>, +L<nbdkit-nozero-filter(1)>. + +=head1 AUTHORS + +Eric Blake + +=head1 COPYRIGHT + +Copyright (C) 2018-2021 Red Hat Inc. diff --git a/filters/nocache/nbdkit-nocache-filter.pod b/filters/nocache/nbdkit-nocache-filter.pod index de970136..3c30533c 100644 --- a/filters/nocache/nbdkit-nocache-filter.pod +++ b/filters/nocache/nbdkit-nocache-filter.pod @@ -77,6 +77,7 @@ L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-cache-filter(1)>, L<nbdkit-fua-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-noextents-filter(1)>, L<nbdkit-noparallel-filter(1)>, L<nbdkit-nozero-filter(1)>. diff --git a/filters/noextents/nbdkit-noextents-filter.pod b/filters/noextents/nbdkit-noextents-filter.pod index 0260a5cf..891b197d 100644 --- a/filters/noextents/nbdkit-noextents-filter.pod +++ b/filters/noextents/nbdkit-noextents-filter.pod @@ -49,6 +49,7 @@ L<nbdkit(1)>, L<nbdkit-filter(3)>, L<nbdkit-extentlist-filter(1)>, L<nbdkit-fua-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noparallel-filter(1)>, L<nbdkit-nozero-filter(1)>, diff --git a/filters/noparallel/nbdkit-noparallel-filter.pod b/filters/noparallel/nbdkit-noparallel-filter.pod index 16861ad9..8ac8d1a7 100644 --- a/filters/noparallel/nbdkit-noparallel-filter.pod +++ b/filters/noparallel/nbdkit-noparallel-filter.pod @@ -76,6 +76,7 @@ L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-fua-filter(1)>, L<nbdkit-limit-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noextents-filter(1)>, L<nbdkit-nozero-filter(1)>. diff --git a/filters/nozero/nbdkit-nozero-filter.pod b/filters/nozero/nbdkit-nozero-filter.pod index 27a030f4..ad6e590d 100644 --- a/filters/nozero/nbdkit-nozero-filter.pod +++ b/filters/nozero/nbdkit-nozero-filter.pod @@ -131,6 +131,7 @@ L<nbdkit(1)>, L<nbdkit-file-plugin(1)>, L<nbdkit-filter(3)>, L<nbdkit-fua-filter(1)>, +L<nbdkit-multi-conn-filter(1)>, L<nbdkit-nocache-filter(1)>, L<nbdkit-noparallel-filter(1)>, L<nbdkit-noextents-filter(1)>. diff --git a/configure.ac b/configure.ac index cb18dd88..2b3e214e 100644 --- a/configure.ac +++ b/configure.ac @@ -1,5 +1,5 @@ # nbdkit -# Copyright (C) 2013-2020 Red Hat Inc. +# Copyright (C) 2013-2021 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -128,6 +128,7 @@ filters="\ ip \ limit \ log \ + multi-conn \ nocache \ noextents \ nofilter \ @@ -1259,6 +1260,7 @@ AC_CONFIG_FILES([Makefile filters/ip/Makefile filters/limit/Makefile filters/log/Makefile + filters/multi-conn/Makefile filters/nocache/Makefile filters/noextents/Makefile filters/nofilter/Makefile diff --git a/filters/multi-conn/Makefile.am b/filters/multi-conn/Makefile.am new file mode 100644 index 00000000..778b8947 --- /dev/null +++ b/filters/multi-conn/Makefile.am @@ -0,0 +1,68 @@ +# nbdkit +# Copyright (C) 2021 Red Hat Inc. +# +# 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. + +include $(top_srcdir)/common-rules.mk + +EXTRA_DIST = nbdkit-multi-conn-filter.pod + +filter_LTLIBRARIES = nbdkit-multi-conn-filter.la + +nbdkit_multi_conn_filter_la_SOURCES = \ + multi-conn.c \ + $(top_srcdir)/include/nbdkit-filter.h \ + $(NULL) + +nbdkit_multi_conn_filter_la_CPPFLAGS = \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/common/include \ + -I$(top_srcdir)/common/utils \ + $(NULL) +nbdkit_multi_conn_filter_la_CFLAGS = $(WARNINGS_CFLAGS) +nbdkit_multi_conn_filter_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(IMPORT_LIBRARY_ON_WINDOWS) \ + $(NULL) +nbdkit_multi_conn_filter_la_LDFLAGS = \ + -module -avoid-version -shared $(NO_UNDEFINED_ON_WINDOWS) \ + -Wl,--version-script=$(top_srcdir)/filters/filters.syms \ + $(NULL) + +if HAVE_POD + +man_MANS = nbdkit-multi-conn-filter.1 +CLEANFILES += $(man_MANS) + +nbdkit-multi-conn-filter.1: nbdkit-multi-conn-filter.pod + $(PODWRAPPER) --section=1 --man $@ \ + --html $(top_builddir)/html/$@.html \ + $< + +endif HAVE_POD diff --git a/tests/Makefile.am b/tests/Makefile.am index 70898f20..4b3ee65c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,5 +1,5 @@ # nbdkit -# Copyright (C) 2013-2020 Red Hat Inc. +# Copyright (C) 2013-2021 Red Hat Inc. # # Redistribution and use in source and binary forms, with or without # modification, are permitted provided that the following conditions are @@ -1538,6 +1538,15 @@ EXTRA_DIST += \ test-log-script-info.sh \ $(NULL) +# multi-conn filter test. +TESTS += \ + test-multi-conn.sh \ + $(NULL) +EXTRA_DIST += \ + test-multi-conn-plugin.sh \ + test-multi-conn.sh \ + $(NULL) + # nofilter test. TESTS += test-nofilter.sh EXTRA_DIST += test-nofilter.sh diff --git a/filters/multi-conn/multi-conn.c b/filters/multi-conn/multi-conn.c new file mode 100644 index 00000000..ce01b488 --- /dev/null +++ b/filters/multi-conn/multi-conn.c @@ -0,0 +1,443 @@ +/* nbdkit + * Copyright (C) 2021 Red Hat Inc. + * + * 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. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <stdint.h> +#include <string.h> +#include <stdbool.h> +#include <assert.h> +#include <pthread.h> + +#include <nbdkit-filter.h> + +#include "cleanup.h" +#include "vector.h" + +/* Track results of .config */ +static enum MultiConnMode { + AUTO, + EMULATE, + PLUGIN, + UNSAFE, + DISABLE, +} mode; + +static enum TrackDirtyMode { + CONN, + FAST, + OFF, +} track; + +enum dirty { + WRITE = 1, /* A write may have populated a cache */ + READ = 2, /* A read may have populated a cache */ +}; + +/* Coordination between connections. */ +static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER; + +/* The list of handles to active connections. */ +struct handle { + struct nbdkit_next_ops *next_ops; + void *nxdata; + enum MultiConnMode mode; /* Runtime resolution of mode==AUTO */ + enum dirty dirty; /* What aspects of this connection are dirty */ +}; +DEFINE_VECTOR_TYPE(conns_vector, struct handle *); +static conns_vector conns = empty_vector; +static bool dirty; /* True if any connection is dirty */ + +/* Accept 'multi-conn-mode=mode' and 'multi-conn-track-dirty=level' */ +static int +multi_conn_config (nbdkit_next_config *next, void *nxdata, + const char *key, const char *value) +{ + if (strcmp (key, "multi-conn-mode") == 0) { + if (strcmp (value, "auto") == 0) + mode = AUTO; + else if (strcmp (value, "emulate") == 0) + mode = EMULATE; + else if (strcmp (value, "plugin") == 0) + mode = PLUGIN; + else if (strcmp (value, "disable") == 0) + mode = DISABLE; + else if (strcmp (value, "unsafe") == 0) + mode = UNSAFE; + else { + nbdkit_error ("unknown multi-conn mode '%s'", value); + return -1; + } + return 0; + } + else if (strcmp (key, "multi-conn-track-dirty") == 0) { + if (strcmp (value, "connection") == 0 || + strcmp (value, "conn") == 0) + track = CONN; + else if (strcmp (value, "fast") == 0) + track = FAST; + else if (strcmp (value, "off") == 0) + track = OFF; + else { + nbdkit_error ("unknown multi-conn track-dirty setting '%s'", value); + return -1; + } + return 0; + } + return next (nxdata, key, value); +} + +#define multi_conn_config_help \ + "multi-conn-mode=<MODE> 'auto' (default), 'emulate', 'plugin',\n" \ + " 'disable', or 'unsafe'.\n" \ + "multi-conn-track-dirty=<LEVEL> 'conn' (default), 'fast', or 'off'.\n" + +static int +multi_conn_get_ready (nbdkit_next_get_ready *next, void *nxdata, + int thread_model) +{ + if (thread_model == NBDKIT_THREAD_MODEL_SERIALIZE_CONNECTIONS && + mode == AUTO) + mode = DISABLE; + return next (nxdata); +} + +static void * +multi_conn_open (nbdkit_next_open *next, void *nxdata, + int readonly, const char *exportname, int is_tls) +{ + struct handle *h; + + if (next (nxdata, readonly, exportname) == -1) + return NULL; + + /* Allocate here, but populate and insert into list in .prepare */ + h = calloc (1, sizeof *h); + if (h == NULL) { + nbdkit_error ("calloc: %m"); + return NULL; + } + return h; +} + +static int +multi_conn_prepare (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, int readonly) +{ + struct handle *h = handle; + int r; + + h->next_ops = next_ops; + h->nxdata = nxdata; + if (mode == AUTO) { /* See also .get_ready turning AUTO into DISABLE */ + r = next_ops->can_multi_conn (nxdata); + if (r == -1) + return -1; + if (r == 0) + h->mode = EMULATE; + else + h->mode = PLUGIN; + } + else + h->mode = mode; + if (h->mode == EMULATE && next_ops->can_flush (nxdata) != 1) { + nbdkit_error ("emulating multi-conn requires working flush"); + return -1; + } + + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + conns_vector_append (&conns, h); + return 0; +} + +static int +multi_conn_finalize (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + struct handle *h = handle; + + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + assert (h->next_ops == next_ops); + assert (h->nxdata == nxdata); + + /* XXX should we add a config param to flush if the client forgot? */ + for (size_t i = 0; i < conns.size; i++) { + if (conns.ptr[i] == h) { + conns_vector_remove (&conns, i); + break; + } + } + return 0; +} + +static void +multi_conn_close (void *handle) +{ + free (handle); +} + +static int +multi_conn_can_fua (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + /* If the backend has native FUA support but is not multi-conn + * consistent, and we have to flush on every connection, then we are + * better off advertising emulated fua rather than native. + */ + struct handle *h = handle; + int fua = next_ops->can_fua (nxdata); + + assert (h->mode != AUTO); + if (fua == NBDKIT_FUA_NATIVE && h->mode == EMULATE) + return NBDKIT_FUA_EMULATE; + return fua; +} + +static int +multi_conn_can_multi_conn (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle) +{ + struct handle *h = handle; + + switch (h->mode) { + case EMULATE: + return 1; + case PLUGIN: + return next_ops->can_multi_conn (nxdata); + case DISABLE: + return 0; + case UNSAFE: + return 1; + case AUTO: /* Not possible, see .prepare */ + default: + abort (); + } +} + +static void +mark_dirty (struct handle *h, bool is_write) +{ + /* No need to grab lock here: the NBD spec is clear that a client + * must wait for the response to a flush before sending the next + * command that expects to see the result of that flush, so any race + * in accessing dirty can be traced back to the client improperly + * sending a flush in parallel with other live commands. + */ + switch (track) { + case CONN: + h->dirty |= is_write ? WRITE : READ; + /* fallthrough */ + case FAST: + if (is_write) + dirty = true; + break; + case OFF: + break; + default: + abort (); + } +} + +static int +multi_conn_flush (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t flags, int *err); + +static int +multi_conn_pread (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, void *buf, uint32_t count, uint64_t offs, + uint32_t flags, int *err) +{ + struct handle *h = handle; + + mark_dirty (h, false); + return next_ops->pread (nxdata, buf, count, offs, flags, err); +} + +static int +multi_conn_pwrite (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, const void *buf, uint32_t count, + uint64_t offs, uint32_t flags, int *err) +{ + struct handle *h = handle; + bool need_flush = false; + + if (flags & NBDKIT_FLAG_FUA) { + if (h->mode == EMULATE) { + mark_dirty (h, true); + need_flush = true; + flags &= ~NBDKIT_FLAG_FUA; + } + } + else + mark_dirty (h, true); + + if (next_ops->pwrite (nxdata, buf, count, offs, flags, err) == -1) + return -1; + if (need_flush) + return multi_conn_flush (next_ops, nxdata, h, 0, err); + return 0; +} + +static int +multi_conn_zero (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + bool need_flush = false; + + if (flags & NBDKIT_FLAG_FUA) { + if (h->mode == EMULATE) { + mark_dirty (h, true); + need_flush = true; + flags &= ~NBDKIT_FLAG_FUA; + } + } + else + mark_dirty (h, true); + + if (next_ops->zero (nxdata, count, offs, flags, err) == -1) + return -1; + if (need_flush) + return multi_conn_flush (next_ops, nxdata, h, 0, err); + return 0; +} + +static int +multi_conn_trim (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + bool need_flush = false; + + if (flags & NBDKIT_FLAG_FUA) { + if (h->mode == EMULATE) { + mark_dirty (h, true); + need_flush = true; + flags &= ~NBDKIT_FLAG_FUA; + } + } + else + mark_dirty (h, true); + + if (next_ops->trim (nxdata, count, offs, flags, err) == -1) + return -1; + if (need_flush) + return multi_conn_flush (next_ops, nxdata, h, 0, err); + return 0; +} + +static int +multi_conn_cache (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t count, uint64_t offs, uint32_t flags, + int *err) +{ + struct handle *h = handle; + + mark_dirty (h, false); + return next_ops->cache (nxdata, count, offs, flags, err); +} + +static int +multi_conn_flush (struct nbdkit_next_ops *next_ops, void *nxdata, + void *handle, uint32_t flags, int *err) +{ + struct handle *h = handle, *h2; + size_t i; + + if (h->mode == EMULATE) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + for (i = 0; i < conns.size; i++) { + h2 = conns.ptr[i]; + if (track == OFF || (dirty && (track == FAST || h2->dirty & READ)) || + h2->dirty & WRITE) { + if (h2->next_ops->flush (h2->nxdata, flags, err) == -1) + return -1; + h2->dirty = 0; + } + } + dirty = 0; + } + else { + /* !EMULATE: Check if the image is clean, allowing us to skip a flush. */ + if (track != OFF && !dirty) + return 0; + /* Perform the flush, then update dirty tracking. */ + if (next_ops->flush (nxdata, flags, err) == -1) + return -1; + switch (track) { + case CONN: + if (next_ops->can_multi_conn (nxdata) == 1) { + ACQUIRE_LOCK_FOR_CURRENT_SCOPE (&lock); + for (i = 0; i < conns.size; i++) + conns.ptr[i]->dirty = 0; + dirty = 0; + } + else + h->dirty = 0; + break; + case FAST: + dirty = false; + break; + case OFF: + break; + default: + abort (); + } + } + return 0; +} + +static struct nbdkit_filter filter = { + .name = "multi-conn", + .longname = "nbdkit multi-conn filter", + .config = multi_conn_config, + .config_help = multi_conn_config_help, + .get_ready = multi_conn_get_ready, + .open = multi_conn_open, + .prepare = multi_conn_prepare, + .finalize = multi_conn_finalize, + .close = multi_conn_close, + .can_fua = multi_conn_can_fua, + .can_multi_conn = multi_conn_can_multi_conn, + .pread = multi_conn_pread, + .pwrite = multi_conn_pwrite, + .trim = multi_conn_trim, + .zero = multi_conn_zero, + .cache = multi_conn_cache, + .flush = multi_conn_flush, +}; + +NBDKIT_REGISTER_FILTER(filter) diff --git a/tests/test-multi-conn-plugin.sh b/tests/test-multi-conn-plugin.sh new file mode 100755 index 00000000..7262a348 --- /dev/null +++ b/tests/test-multi-conn-plugin.sh @@ -0,0 +1,122 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018-2021 Red Hat Inc. +# +# 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. + +# Test plugin used by test-multi-conn.sh. +# This plugin purposefully maintains a per-connection cache. +# An optional parameter tightfua=true controls whether FUA acts on +# just the given region, or on all pending ops in the current connection. +# Note that an earlier cached write on one connection can overwrite a later +# FUA write on another connection - this is okay (the client is buggy if +# it ever sends overlapping writes without coordinating flushes and still +# expects any particular write to occur last). + +fill_cache() { + if test ! -f "$tmpdir/$1"; then + cp "$tmpdir/0" "$tmpdir/$1" + fi +} +do_fua() { + case ,$4, in + *,fua,*) + if test -f "$tmpdir/strictfua"; then + dd of="$tmpdir/0" if="$tmpdir/$1" skip=$3 seek=$3 count=$2 \ + conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes + else + do_flush $1 + fi ;; + esac +} +do_flush() { + if test -f "$tmpdir/$1-replay"; then + while read cnt off; do + dd of="$tmpdir/0" if="$tmpdir/$1" skip=$off seek=$off count=$cnt \ + conv=notrunc iflag=count_bytes,skip_bytes oflag=seek_bytes + done < "$tmpdir/$1-replay" + fi + rm -f "$tmpdir/$1" "$tmpdir/$1-replay" +} +case "$1" in + config) + case $2 in + strictfua) + case $3 in + true | on | 1) touch "$tmpdir/strictfua" ;; + false | off | 0) ;; + *) echo "unknown value for strictfua $3" >&2; exit 1 ;; + esac ;; + *) echo "unknown config key $2" >&2; exit 1 ;; + esac + ;; + get_ready) + printf "%-32s" 'Initial contents' > "$tmpdir/0" + echo 0 > "$tmpdir/counter" + ;; + get_size) + echo 32 + ;; + can_write | can_zero | can_trim | can_flush) + exit 0 + ;; + can_fua | can_cache) + echo native + ;; + open) + read i < "$tmpdir/counter" + echo $((i+1)) | tee "$tmpdir/counter" + ;; + pread) + fill_cache $2 + dd if="$tmpdir/$2" skip=$4 count=$3 iflag=count_bytes,skip_bytes + ;; + cache) + fill_cache $2 + ;; + pwrite) + fill_cache $2 + dd of="$tmpdir/$2" seek=$4 conv=notrunc oflag=seek_bytes + echo $3 $4 >> "$tmpdir/$2-replay" + do_fua $2 $3 $4 $5 + ;; + zero | trim) + fill_cache $2 + dd of="$tmpdir/$2" if="/dev/zero" count=$3 seek=$4 conv=notrunc\ + oflag=seek_bytes iflag=count_bytes + echo $3 $4 >> "$tmpdir/$2-replay" + do_fua $2 $3 $4 $5 + ;; + flush) + do_flush $2 + ;; + *) + exit 2 + ;; +esac diff --git a/tests/test-multi-conn.sh b/tests/test-multi-conn.sh new file mode 100755 index 00000000..8580608e --- /dev/null +++ b/tests/test-multi-conn.sh @@ -0,0 +1,293 @@ +#!/usr/bin/env bash +# nbdkit +# Copyright (C) 2018-2021 Red Hat Inc. +# +# 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. + +# Demonstrate various multi-conn filter behaviors. + +source ./functions.sh +set -e +set -x + +requires_plugin sh +requires_nbdsh_uri +requires dd iflag=count_bytes </dev/null + +files="test-multi-conn.out test-multi-conn.stat" +rm -f $files +cleanup_fn rm -f $files + +fail=0 +export handles preamble uri +uri= # will be set by --run later +handles=3 +preamble=' +import os + +uri = os.environ["uri"] +handles = int(os.environ["handles"]) +h = [] +for i in range(handles): + h.append(nbd.NBD()) + h[i].connect_uri(uri) +print(h[0].can_multi_conn()) +' + +# Demonstrate the caching present without use of filter +for filter in '' '--filter=multi-conn multi-conn-mode=plugin'; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh $filter \ + --run 'handles=4 nbdsh -c "$preamble" -c " +# Without flush, reads cache, and writes do not affect persistent data +print(h[0].pread(4, 0)) +h[1].pwrite(b'\''next '\'', 0) +print(h[0].pread(4, 0)) +print(h[1].pread(4, 0)) +print(h[2].pread(4, 0)) +# Flushing an unrelated connection does not make writes persistent +h[2].flush() +print(h[0].pread(4, 0)) +print(h[1].pread(4, 0)) +print(h[2].pread(4, 0)) +# After write is flushed, only connections without cache see new data +h[1].flush() +print(h[0].pread(4, 0)) +print(h[1].pread(4, 0)) +print(h[2].pread(4, 0)) +print(h[3].pread(4, 0)) +# Flushing before reads clears the cache +h[0].flush() +h[2].flush() +print(h[0].pread(4, 0)) +print(h[2].pread(4, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +False +b'Init' +b'Init' +b'next' +b'Init' +b'Init' +b'next' +b'Init' +b'Init' +b'next' +b'Init' +b'next' +b'next' +b'next' +EOF + ) test-multi-conn.out || fail=1 +done + +# Demonstrate specifics of FUA flag +for filter in '' '--filter=multi-conn multi-conn-mode=plugin'; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh $filter \ + --run 'nbdsh -c "$preamble" -c " +# Some servers let FUA flush all outstanding requests +h[0].pwrite(b'\''hello '\'', 0) +h[0].pwrite(b'\''world.'\'', 6, nbd.CMD_FLAG_FUA) +print(h[1].pread(12, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +False +b'hello world.' +EOF + ) test-multi-conn.out || fail=1 +done +for filter in '' '--filter=multi-conn multi-conn-mode=plugin'; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh strictfua=1 $filter \ + --run 'nbdsh -c "$preamble" -c " +# But it is also compliant for a server that only flushes the exact request +h[0].pwrite(b'\''hello '\'', 0) +h[0].pwrite(b'\''world.'\'', 6, nbd.CMD_FLAG_FUA) +print(h[1].pread(12, 0)) +# Without multi-conn, data flushed in one connection can later be reverted +# by a flush of earlier data in another connection +h[1].pwrite(b'\''H'\'', 0, nbd.CMD_FLAG_FUA) +h[2].flush() +print(h[2].pread(12, 0)) +h[0].flush() +h[2].flush() +print(h[2].pread(12, 0)) +h[1].flush() +h[2].flush() +print(h[2].pread(12, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +False +b'Initiaworld.' +b'Hnitiaworld.' +b'hello world.' +b'Hello world.' +EOF + ) test-multi-conn.out || fail=1 +done + +# Demonstrate multi-conn effects. The cache filter in writeback +# mode is also able to supply multi-conn by a different technique. +for filter in '--filter=multi-conn' 'strictfua=1 --filter=multi-conn' \ + '--filter=multi-conn multi-conn-mode=plugin --filter=cache' ; do + nbdkit -vf -U - sh test-multi-conn-plugin.sh $filter \ + --run 'nbdsh -c "$preamble" -c " +# FUA writes are immediately visible on all connections +h[0].cache(12, 0) +h[1].pwrite(b'\''Hello '\'', 0, nbd.CMD_FLAG_FUA) +print(h[0].pread(12, 0)) +# A flush on an unrelated connection makes all other connections consistent +h[1].pwrite(b'\''world.'\'', 6) +h[2].flush() +print(h[0].pread(12, 0)) +"' > test-multi-conn.out || fail=1 + diff -u <(cat <<\EOF +True +b'Hello l cont' +b'Hello world.' +EOF + ) test-multi-conn.out || fail=1 +done + +# unsafe mode intentionally lacks consistency, use at your own risk +nbdkit -vf -U - sh test-multi-conn-plugin.sh \ + --filter=multi-conn multi-conn-mode=unsafe \ + --run 'nbdsh -c "$preamble" -c " +h[0].cache(12, 0) +h[1].pwrite(b'\''Hello '\'', 0, nbd.CMD_FLAG_FUA) +print(h[0].pread(12, 0)) +h[1].pwrite(b'\''world.'\'', 6) +h[2].flush() +print(h[0].pread(12, 0)) +"' > test-multi-conn.out || fail=1 +diff -u <(cat <<\EOF +True +b'Initial cont' +b'Initial cont' +EOF + ) test-multi-conn.out || fail=1 + +# auto mode devolves to multi-conn disable when connections are serialized +nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=noparallel \ + serialize=connections --filter=multi-conn --filter=cache \ + --run 'handles=1 nbdsh -c "$preamble" +' > test-multi-conn.out || fail=1 +diff -u <(cat <<\EOF +False +EOF + ) test-multi-conn.out || fail=1 + +# Use --filter=stats to show track-dirty effects +for level in off connection fast; do + for mode in emulate 'emulate --filter=cache' \ + plugin 'plugin --filter=cache'; do + echo "setup: $level $mode" >> test-multi-conn.stat + # Flush with no activity + nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --filter=stats statsfile=test-multi-conn.stat statsappend=true \ + multi-conn-track-dirty=$level multi-conn-mode=$mode \ + --run 'nbdsh -c "$preamble" -c " +h[0].flush() +h[0].pread(1, 0) +h[0].flush() +"' > test-multi-conn.out || fail=1 + # Client that flushes assuming multi-conn semantics + nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --filter=stats statsfile=test-multi-conn.stat statsappend=true \ + multi-conn-track-dirty=$level multi-conn-mode=$mode \ + --run 'handles=4 nbdsh -c "$preamble" -c " +h[0].pread(1, 0) +h[1].zero(1, 0) +h[3].flush() +h[2].zero(1, 1) +h[0].pread(1, 0) +h[3].flush() +h[3].flush() +"' > test-multi-conn.out || fail=1 + # Client that flushes assuming inconsistent semantics + nbdkit -vf -U - sh test-multi-conn-plugin.sh --filter=multi-conn \ + --filter=stats statsfile=test-multi-conn.stat statsappend=true \ + multi-conn-track-dirty=$level multi-conn-mode=$mode \ + --run 'nbdsh -c "$preamble" -c " +h[0].pread(1, 0) +h[1].trim(1, 0) +h[0].flush() +h[1].flush() +h[0].pread(1, 0) +h[2].trim(1, 1) +h[0].flush() +h[2].flush() +"' > test-multi-conn.out || fail=1 + done +done +cat test-multi-conn.stat +diff -u <(cat <<\EOF +setup: off emulate +flush: 6 ops +flush: 12 ops +flush: 12 ops +setup: off emulate --filter=cache +flush: 6 ops +flush: 12 ops +flush: 12 ops +setup: off plugin +flush: 2 ops +flush: 3 ops +flush: 4 ops +setup: off plugin --filter=cache +flush: 2 ops +flush: 3 ops +flush: 4 ops +setup: connection emulate +flush: 4 ops +flush: 4 ops +setup: connection emulate --filter=cache +flush: 4 ops +flush: 4 ops +setup: connection plugin +flush: 3 ops +flush: 4 ops +setup: connection plugin --filter=cache +flush: 2 ops +flush: 2 ops +setup: fast emulate +flush: 8 ops +flush: 6 ops +setup: fast emulate --filter=cache +flush: 8 ops +flush: 6 ops +setup: fast plugin +flush: 2 ops +flush: 2 ops +setup: fast plugin --filter=cache +flush: 2 ops +flush: 2 ops +EOF + ) <(sed -n 's/\(flush:.*ops\).*/\1/p; /^setup:/p' \ + test-multi-conn.stat) || fail=1 + +exit $fail diff --git a/TODO b/TODO index d8dd7ef2..e41e38e8 100644 --- a/TODO +++ b/TODO @@ -206,13 +206,6 @@ Suggestions for filters * masking plugin features for testing clients (see 'nozero' and 'fua' filters for examples) -* multi-conn filter to adjust advertisement of multi-conn bit. In - particular, if the plugin lacks .can_multi_conn, then .open/.close - track all open connections, and .flush and FUA flag will call - next_ops->flush() on all of them. Conversely, if plugin supports - multi-conn, we can cache whether the image is dirty, and avoid - expense of next_ops->flush when it is clean. - * "bandwidth quota" filter which would close a connection after it exceeded a certain amount of bandwidth up or down. -- 2.30.1